Ala Hino has posted comments on this change. Change subject: Live Merge: Refresh base volume before live merge ......................................................................
Patch Set 5: (5 comments) https://gerrit.ovirt.org/#/c/63454/5/vdsm/virt/vm.py File vdsm/virt/vm.py: Line 4436: # If the base volume format is RAW and its size is smaller than its Line 4437: # capacity (this could happen because the engine extended the base Line 4438: # volume), we have to refresh the volume to cause lvm to get current lv Line 4439: # size from storage, and update the kernel so the lv reflects the real Line 4440: # size on storage. Not refreshing the volume may fail live merre > merre -> merge Done Line 4441: # because the top volume size may be larger then the base volume size. Line 4442: # This could happen if disk extended after taking a snapshot but before Line 4443: # performing the live merge. See https://bugzilla.redhat.com/1367281 Line 4444: baseSize = int(baseInfo['apparentsize']) Line 4439: # size from storage, and update the kernel so the lv reflects the real Line 4440: # size on storage. Not refreshing the volume may fail live merre Line 4441: # because the top volume size may be larger then the base volume size. Line 4442: # This could happen if disk extended after taking a snapshot but before Line 4443: # performing the live merge. See https://bugzilla.redhat.com/1367281 > This comment became too long, the first part explain the issue very clearly Done Line 4444: baseSize = int(baseInfo['apparentsize']) Line 4445: baseCapacity = int(baseInfo['capacity']) Line 4446: if (drive.chunked Line 4447: and baseInfo['format'] == 'RAW' Line 4441: # because the top volume size may be larger then the base volume size. Line 4442: # This could happen if disk extended after taking a snapshot but before Line 4443: # performing the live merge. See https://bugzilla.redhat.com/1367281 Line 4444: baseSize = int(baseInfo['apparentsize']) Line 4445: baseCapacity = int(baseInfo['capacity']) > Please avoid these variables, in particular using "size" for "apparentsize" Done Line 4446: if (drive.chunked Line 4447: and baseInfo['format'] == 'RAW' Line 4448: and baseSize < baseCapacity): Line 4449: self.log.info("Base volume is RAW and its size is smaller " Line 4443: # performing the live merge. See https://bugzilla.redhat.com/1367281 Line 4444: baseSize = int(baseInfo['apparentsize']) Line 4445: baseCapacity = int(baseInfo['capacity']) Line 4446: if (drive.chunked Line 4447: and baseInfo['format'] == 'RAW' > Always put "and" at the end of the previous line: Done Line 4448: and baseSize < baseCapacity): Line 4449: self.log.info("Base volume is RAW and its size is smaller " Line 4450: "than its capacity hence, explicitly " Line 4451: "refreshing the base volume is required") Line 4447: and baseInfo['format'] == 'RAW' Line 4448: and baseSize < baseCapacity): Line 4449: self.log.info("Base volume is RAW and its size is smaller " Line 4450: "than its capacity hence, explicitly " Line 4451: "refreshing the base volume is required") > This is too verbose and not detailed enough - use something like: Done Line 4452: self.__refreshDriveVolume({ Line 4453: 'domainID': drive.domainID, 'poolID': drive.poolID, Line 4454: 'imageID': drive.imageID, 'volumeID': baseVolUUID, Line 4455: }) -- To view, visit https://gerrit.ovirt.org/63454 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I787d6854e780035b09e4f09d71ca776342dff5be Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Ala Hino <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Ala Hino <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/admin/lists/[email protected]
