Nir Soffer has posted comments on this change. Change subject: v2v: add volume size to disk info ......................................................................
Patch Set 6: (6 comments) http://gerrit.ovirt.org/#/c/36263/6/vdsm/v2v.py File vdsm/v2v.py: Line 22 Line 23 Line 24 Line 25 Line 26 This is vdsm lib import, should be after stdlib import but before vdsm application imports. Line 22: from vdsm.define import errCode Line 23: import caps Line 24: import libvirt Line 25: from vdsm import libvirtconnection Line 26: from contextlib import closing This is stdlib import, should be on top. Line 27: Line 28: Line 29: class InvalidVMConfiguration(ValueError): Line 30: ''' Unexpected error while parsing libvirt domain xml ''' Line 83: params['vmName'] = vm.name() Line 84: if vm.state()[0] == libvirt.VIR_DOMAIN_SHUTOFF: Line 85: params['status'] = "Down" Line 86: else: Line 87: params['status'] = "Up" Nice, but it not related to this change (adding disk size) - please separate to another patch. Line 88: Line 89: Line 90: def _add_general_info(root, params): Line 91: e = root.find('./uuid') Line 113: if e is not None: Line 114: params['arch'] = e.get('arch') Line 115: Line 116: Line 117: def _add_disk_info(disk, conn): To be consistent with other functions, conn should be the first arguement (get info from conn) and disk last (add info to disk). Line 118: if 'alias' in disk.keys(): Line 119: try: Line 120: vol = conn.storageVolLookupByPath(disk['alias']) Line 121: disk['truesize'] = str(vol.info()[1]) Line 119: try: Line 120: vol = conn.storageVolLookupByPath(disk['alias']) Line 121: disk['truesize'] = str(vol.info()[1]) Line 122: except libvirt.libvirtError as e: Line 123: logging.error("error parsing volume size: %s" % e) This error message is incorrect - should be "Error getting disk size", I don't see any parsing here. Line 124: Line 125: Line 126: def _add_disks(root, params, conn): Line 127: params['disks'] = [] Line 122: except libvirt.libvirtError as e: Line 123: logging.error("error parsing volume size: %s" % e) Line 124: Line 125: Line 126: def _add_disks(root, params, conn): conn is not needed here now. Line 127: params['disks'] = [] Line 128: disks = root.findall('.//disk[@type="file"]') Line 129: for disk in disks: Line 130: d = {} -- To view, visit http://gerrit.ovirt.org/36263 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic7e9ba73514292cc2bb4a025d286e2c732e88a75 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Shahar Havivi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
