Shahar Havivi has posted comments on this change.

Change subject: v2v: add volume size to disk info
......................................................................


Patch Set 8:

(4 comments)

http://gerrit.ovirt.org/#/c/36263/8/tests/v2vTests.py
File tests/v2vTests.py:

Line 78:     def storageVolLookupByPath(self, name):
Line 79:         return LibvirtMock.Volume()
Line 80: 
Line 81:     class Volume(object):
Line 82:         def info(item):
> doesn't it return a three-tuple?
No it returns a list
Line 83:             return [0, 0]
Line 84: 
Line 85: 
Line 86: def hypervisorConnect(uri, username, passwd):


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

Line 3635: # @dev:      #optional guest device name (sda, sdb, etc)
Line 3636: #
Line 3637: # @alias:    #optional device path (file path)
Line 3638: #
Line 3639: # @truesize: #optional device size in bytes
> We have very confusing truesize/apparentsize/virtual size elsewhere. Do we 
Please suggest name
Line 3640: #
Line 3641: # Since: 4.17.0
Line 3642: ##
Line 3643: {'type': 'ExternalDiskInfo',


Line 3640: #
Line 3641: # Since: 4.17.0
Line 3642: ##
Line 3643: {'type': 'ExternalDiskInfo',
Line 3644:  'data': {'*dev': 'str', '*alias': 'str', '*truesize': 'str'}}
> I hope truesize is a uint.
Done
Line 3645: 
Line 3646: ##
Line 3647: # @ExternalNetworkInfo:
Line 3648: #


http://gerrit.ovirt.org/#/c/36263/8/vdsm/v2v.py
File vdsm/v2v.py:

Line 117:         try:
Line 118:             vol = conn.storageVolLookupByPath(disk['alias'])
Line 119:             disk['truesize'] = str(vol.info()[1])
Line 120:         except libvirt.libvirtError as e:
Line 121:             logging.error("Error getting disk size: %s" % e)
> Since we catch the exception right after calling libvirt, I don't think the
assuming:
logging.exception("Error getting disk size: %s" % e)
Line 122: 
Line 123: 
Line 124: def _add_disks(root, params):
Line 125:     params['disks'] = []


-- 
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: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shav...@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: 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

Reply via email to