Nir Soffer has posted comments on this change.

Change subject: vm: Update LUN size when starting a vm
......................................................................


Patch Set 2: Code-Review+1

(3 comments)

Looks good with few text issues.

The check for GUID looks wrong but is "protected" by other wrong code few lines 
before. Fixing this require fixing the entire method and is out of the scope of 
this patch.

http://gerrit.ovirt.org/#/c/24235/2/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3106:         utils.retry(partial(fileUtils.validateQemuReadable, devPath),
Line 3107:                     expectedException=OSError,
Line 3108:                     timeout=QEMU_READABLE_TIMEOUT)
Line 3109: 
Line 3110:         # Get the size of the logical unit volume.
This line still adds no value.
Line 3111:         # Casting to string for keeping consistency with public 
methods
Line 3112:         # that use it to overcome xmlrpc integer size limitation 
issues.
Line 3113:         size = 
str(multipath.getDeviceSize(devicemapper.getDmId(guid)))
Line 3114: 


Line 3107:                     expectedException=OSError,
Line 3108:                     timeout=QEMU_READABLE_TIMEOUT)
Line 3109: 
Line 3110:         # Get the size of the logical unit volume.
Line 3111:         # Casting to string for keeping consistency with public 
methods
Converting to string
Line 3112:         # that use it to overcome xmlrpc integer size limitation 
issues.
Line 3113:         size = 
str(multipath.getDeviceSize(devicemapper.getDmId(guid)))
Line 3114: 
Line 3115:         return dict(truesize=size, apparentsize=size)


http://gerrit.ovirt.org/#/c/24235/2/vdsm/vm.py
File vdsm/vm.py:

Line 631:                           'apparentsize': str(vmDrive.apparentsize)}
Line 632:                 if isVdsmImage(vmDrive):
Line 633:                     dStats['imageID'] = vmDrive.imageID
Line 634:                 else:
Line 635:                     dStats['lunGUID'] = vmDrive.GUID
This check is wrong, but since drives without imageID or GUID also do not have 
truesize attribute, they will fail earlier with AttributeError on line 630.
Line 636:                 dStats['readRate'] = ((eInfo[dName][1] - 
sInfo[dName][1]) /
Line 637:                                       sampleInterval)
Line 638:                 dStats['writeRate'] = ((eInfo[dName][3] - 
sInfo[dName][3]) /
Line 639:                                        sampleInterval)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4988212f7f96078e774d2a4e7b5cd1681383cb0
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <[email protected]>
Gerrit-Reviewer: Ayal Baron <[email protected]>
Gerrit-Reviewer: Daniel Erez <[email protected]>
Gerrit-Reviewer: Federico Simoncelli <[email protected]>
Gerrit-Reviewer: Nir Soffer <[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

Reply via email to