Change in vdsm[master]: External hypervisor VMs integration

shavivi at redhat.com shavivi at redhat.com
Wed Nov 26 09:28:43 UTC 2014


Shahar Havivi has posted comments on this change.

Change subject: External hypervisor VMs integration
......................................................................


Patch Set 27:

(7 comments)

http://gerrit.ovirt.org/#/c/33309/27/vdsm/v2v.py
File vdsm/v2v.py:

Line 22: from virt import vmstatus
Line 23: from vdsm import libvirtconnection
Line 24: 
Line 25: 
Line 26: class InvalidVMConfiguration(ValueError):
> Subclassing ValueError make it possible that code looking for ValueError (s
Saggy?
Line 27:     ''' Unexpected value while parsing libvirt domain xml '''
Line 28:     def __init__(self, field_name, bad_value):
Line 29:         ValueError.__init__(self, "Invalid %s value: '%r'" % (field_name,
Line 30:                                                               bad_value))


Line 25: 
Line 26: class InvalidVMConfiguration(ValueError):
Line 27:     ''' Unexpected value while parsing libvirt domain xml '''
Line 28:     def __init__(self, field_name, bad_value):
Line 29:         ValueError.__init__(self, "Invalid %s value: '%r'" % (field_name,
> %r is used to show a Python representation of the value - you don't need to
Done
Line 30:                                                               bad_value))
Line 31: 
Line 32: 
Line 33: def getExternalVMs(uri, username, password):


Line 43:             params['status'] = vmstatus.statuses[vm.state()[0]]
Line 44:             try:
Line 45:                 _addGeneralInfo(root, params)
Line 46:             except InvalidVMConfiguration:
Line 47:                 logging.error('v2v::getExternalVMs: error parsing domain xml,'
> We don't need to log the module and function name, since vdsm log is config
Done
Line 48:                               'msg: %s  xml: %s', vm.XMLDesc(0))
Line 49:             _addDisks(root, params)
Line 50:             _addNetworks(root, params)
Line 51:             ret.append(params)


Line 44:             try:
Line 45:                 _addGeneralInfo(root, params)
Line 46:             except InvalidVMConfiguration:
Line 47:                 logging.error('v2v::getExternalVMs: error parsing domain xml,'
Line 48:                               'msg: %s  xml: %s', vm.XMLDesc(0))
> A space before "msg" is missing - this will log:
Done
Line 49:             _addDisks(root, params)
Line 50:             _addNetworks(root, params)
Line 51:             ret.append(params)
Line 52:         return ret


Line 64:         return size * 1024
Line 65:     elif lunit in ('tib', 't'):
Line 66:         return size * 1024 * 1024
Line 67:     else:
Line 68:         raise ValueError("Invalid memory unit: '%r'", unit)
> Did you mean InvalidVMConfiguration(...)?
Done
Line 69: 
Line 70: 
Line 71: def _addGeneralInfo(root, params):
Line 72:     e = root.find('./uuid')


Line 77:     if e is not None:
Line 78:         try:
Line 79:             size = int(e.text)
Line 80:         except ValueError:
Line 81:             raise InvalidVMConfiguration("memory value", e.text)
> This will create the error message (assuming that e.text is 3.14):
Done
Line 82:         unit = e.get('unit', 'KiB')
Line 83:         params['memSize'] = _memToMb(size, unit)
Line 84: 
Line 85:     e = root.find('./vcpu')


Line 86:     if e is not None:
Line 87:         try:
Line 88:             params['smp'] = int(e.text)
Line 89:         except ValueError:
Line 90:             raise InvalidVMConfiguration("vcpu value", e.text)
> Same
Done
Line 91: 
Line 92: 
Line 93: def _addDisks(root, params):
Line 94:     params['disks'] = []


-- 
To view, visit http://gerrit.ovirt.org/33309
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I7dcfb860626a844d1d08590274b508519a33f4a3
Gerrit-PatchSet: 27
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list