Change in vdsm[master]: vm: Update LUN size when starting a vm

ewarszaw at redhat.com ewarszaw at redhat.com
Sun Jan 12 07:20:23 UTC 2014


Eduardo has posted comments on this change.

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


Patch Set 5:

(4 comments)

....................................................
File vdsm/clientIF.py
Line 263:                 res = self.irs.appropriateDevice(drive["GUID"], vmId)
Line 264:                 if res['status']['code']:
Line 265:                     raise vm.VolumeError(drive)
Line 266: 
Line 267:                 # Update size for LUN volume
Please add a comment here indicating that this should be removed when the issue is solved with the appropiate refresh of the storage target, and this is a compromise "solution".
Line 268:                 volSize = self.irs.getVolumeSize(drive["GUID"])
Line 269:                 drive["truesize"] = volSize['truesize']
Line 270:                 drive["apparentsize"] = volSize['apparentsize']
Line 271: 


....................................................
File vdsm/storage/hsm.py
Line 2991:             sdUUID=sdUUID).produceVolume(imgUUID=imgUUID,
Line 2992:                                          volUUID=volUUID).refreshVolume()
Line 2993: 
Line 2994:     @public
Line 2995:     def getVolumeSize(self, *args):
Daniel you are right with a general get size interface for a volume being independent of the identifier passed.

Please consider passing a tuple for the PDIV. This way all the signatures will have the same number of arguments.
Line 2996:         if len(args) == 1:
Line 2997:             return self._getLUVolumeSize(args[0])
Line 2998:         else:
Line 2999:             return self._getPDIVSize(args[0], args[1], args[2], args[3])


Line 2997:             return self._getLUVolumeSize(args[0])
Line 2998:         else:
Line 2999:             return self._getPDIVSize(args[0], args[1], args[2], args[3])
Line 3000: 
Line 3001:     def _getLUVolumeSize(self, devGUID, options=None):
The name should be getLUSize IMHO. Logical Unit is descriptive enough for anybody with basic storage knowledge.
Line 3002:         """
Line 3003:         Gets the size of a LUN device volume.
Line 3004: 
Line 3005:         :param devGUID: The LUN device GUID.


....................................................
File vdsm/vm.py
Line 632:                 dStats = {'truesize': str(vmDrive.truesize),
Line 633:                           'apparentsize': str(vmDrive.apparentsize)}
Line 634:                 if isVdsmImage(vmDrive):
Line 635:                     dStats['imageID'] = vmDrive.imageID
Line 636:                 if vmDrive.GUID:
elif should be used here since up to now the PDIVs are not identified by GUIDs.

The usage criteria of PDIV or Direct LU is the 'GUID' key being present in the drive specification dictionary. Please change your condition.

You need to be very careful and check that the all the dStats can be filled for any Direct LU usage and dStats info is enough for all the consumers.
Line 637:                     dStats['lunGUID'] = vmDrive.GUID
Line 638:                 dStats['readRate'] = ((eInfo[dName][1] - sInfo[dName][1]) /
Line 639:                                       sampleInterval)
Line 640:                 dStats['writeRate'] = ((eInfo[dName][3] - sInfo[dName][3]) /


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I48b4343c6519fed366beec415c47226d4e3c8fef
Gerrit-PatchSet: 5
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Daniel Erez <derez at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list