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 <shav...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches