Change in vdsm[master]: External hypervisor VMs integration

smizrahi at redhat.com smizrahi at redhat.com
Mon Nov 17 14:55:20 UTC 2014


Saggi Mizrahi has posted comments on this change.

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


Patch Set 19:

(5 comments)

http://gerrit.ovirt.org/#/c/33309/19/vdsm/rpc/vdsmapi-schema.json
File vdsm/rpc/vdsmapi-schema.json:

Line 3598: # @ExternalDiskInfo:
Line 3599: #
Line 3600: # Disk information from external source
Line 3601: #
Line 3602: # @dev: device name (sda, sdb, etc)
It's not clear if it's the device name as the guest sees it or as the host sees it.
Line 3603: #
Line 3604: # @alias: device path (file path)
Line 3605: #
Line 3606: # Since: 4.17.0


Line 3632: # map of VM properties
Line 3633: #
Line 3634: # @vmName:  The name of the VM
Line 3635: #
Line 3636: # @status:  The VM status (up, down, etc)
Don't use str, there is a VmStatus type.
Line 3637: #
Line 3638: # @vmId:  The VM uuid
Line 3639: #
Line 3640: # @smp:  number of cpu


http://gerrit.ovirt.org/#/c/33309/19/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(Exception):
Please extend ValueError[1] and not Exception.

Also, you can make the constructor expect:
field_name and bad_value and set the message to

"Invalid %s value: '%r'" % (field_name, bad_value)

Otherwise you might as well use the built in ValueError


[1]
exception ValueError
 
    Raised when a built-in operation or function receives an argument that has the right type but an inappropriate value, and the situation is not described by a more precise exception such as IndexError.
Line 27:     ''' Unexpected value while parsing libvirt domain xml '''
Line 28: 
Line 29: 
Line 30: def getExternalVMs(uri, username, password):


Line 48:             ret.append(params)
Line 49:         return ret
Line 50: 
Line 51: 
Line 52: def _memToMb(size, unit):
do:
  unit = unit.lower()
 And compare values in a case insensitive manner
Line 53:     if unit in ('bytes', 'b'):
Line 54:         return size / 1024 / 1024
Line 55:     elif unit in ('KiB', 'k'):
Line 56:         return size / 1024


Line 60:         return size * 1024
Line 61:     elif unit in ('TiB', 'T'):
Line 62:         return size * 1024 * 1024
Line 63:     else:
Line 64:         raise InvalidVMConfiguration("Invalid memory unit: '%r'" % unit)
raise ValueError as this method doesn't directly access the VMConfiguration and is agnostic to where the parameter arrived from.
Line 65: 
Line 66: 
Line 67: def _addGeneralInfo(root, params):
Line 68:     e = root.find('./uuid')


-- 
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: 19
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: 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