Change in vdsm[master]: One shot prepare.

abaron at redhat.com abaron at redhat.com
Mon Aug 5 11:59:40 UTC 2013


Ayal Baron has posted comments on this change.

Change subject: One shot prepare.
......................................................................


Patch Set 10: Code-Review-2

(9 comments)

First, need to fix comments
I'm giving -2 since without fixing selinux issue this must not be introduced (as it will break run vm).
once selinux changes are introduced then we can introduce this patch (after fixes)

....................................................
File vdsm/clientIF.py
Line 257: 
Line 258:                 volPath = res['path']
Line 259:                 # drive['volsInfo'] is unsorted!
Line 260:                 drive['volumeInfo'] = res['imgVolumesInfo']
Line 261:                 drive['volumeInfo'] = res['info']
this looks like a bug to me.
Line 262: 
Line 263:             # GUID drive format
Line 264:             elif "GUID" in drive:
Line 265:                 visible = self.irs.scanDevicesVisibility([drive["GUID"]])


....................................................
File vdsm/storage/blockSD.py
Line 1027:     def getAllRemnants(self):
Line 1028:         vols, rems = self.getAllVolumesImages()
Line 1029:         return rems
Line 1030: 
Line 1031:     def linkRunImage(self, srcImgPath, imgUUID, volUUIDs):
I don't understand the name of this method
Line 1032:         """
Line 1033:         Creates a compatiblity layer for vdsm and qemu-img use.
Line 1034: 
Line 1035:         srcImgPath: Dir where the image volumes are.


Line 1029:         return rems
Line 1030: 
Line 1031:     def linkRunImage(self, srcImgPath, imgUUID, volUUIDs):
Line 1032:         """
Line 1033:         Creates a compatiblity layer for vdsm and qemu-img use.
?
Line 1034: 
Line 1035:         srcImgPath: Dir where the image volumes are.
Line 1036:         """
Line 1037:         sdRunDir = os.path.join("/var/run/vdsm/storage", self.sdUUID)


Line 1059:         allVols: getAllVolumes result.
Line 1060: 
Line 1061:         If the image is based on a template image it will be activated.
Line 1062:         """
Line 1063:         volUUIDs = sd.getVolsOfImage(allVols, imgUUID).keys()
you're already passing imgVols, not allVols, no need to call getVolsOfImage
Line 1064:         lvm.activateLVs(self.sdUUID, volUUIDs)
Line 1065:         vgDir = os.path.join("/dev", self.sdUUID)
Line 1066:         return self.linkRunImage(vgDir, imgUUID, volUUIDs)
Line 1067: 


....................................................
File vdsm/storage/fileSD.py
Line 417:                 volumes[volUUID] = {'imgs': [imgUUID], 'parent': None}
Line 418:         return dict((k, sd.ImgsPar(tuple(v['imgs']), v['parent']))
Line 419:                     for k, v in volumes.iteritems())
Line 420: 
Line 421:     def linkRunImage(self, srcImgPath, imgUUID):
same
Line 422:         """
Line 423:         Creates a compatiblity layer for vdsm and qemu-img use.
Line 424: 
Line 425:         srcImgPath: Dir where the image volumes are.


Line 419:                     for k, v in volumes.iteritems())
Line 420: 
Line 421:     def linkRunImage(self, srcImgPath, imgUUID):
Line 422:         """
Line 423:         Creates a compatiblity layer for vdsm and qemu-img use.
ditto
Line 424: 
Line 425:         srcImgPath: Dir where the image volumes are.
Line 426:         """
Line 427:         sdRunDir = os.path.join("/var/run/vdsm/storage", self.sdUUID)


Line 437:                 raise
Line 438: 
Line 439:         return imgRunDir
Line 440: 
Line 441:     def activateVolumes(self, imgUUID, allVols):
you're only ever passing imgVolumes, not allVols
Line 442:         """
Line 443:         Activate all the volumes listed in volUUIDs
Line 444:         """
Line 445:         # Volumes leaves created in 2.2 did not have group writeable bit


Line 448:         # of the leaf only but to not introduce an additional requirement
Line 449:         # (ordered volUUIDs) we fix them all.
Line 450:         imgDir = os.path.join(self.mountpoint, self.sdUUID, sd.DOMAIN_IMAGES,
Line 451:                               imgUUID)
Line 452:         volUUIDs = sd.getVolsOfImage(allVols, imgUUID).keys()
you're already passing image volumes, why call this again?
Line 453:         volPaths = tuple(os.path.join(imgDir, v) for v in volUUIDs)
Line 454:         for volPath in volPaths:
Line 455:             self.oop.fileUtils.copyUserModeToGroup(volPath)
Line 456: 


....................................................
File vdsm/storage/imageRepository/formatConverter.py
Line 272:                 except (OSError, se.CannotActivateLogicalVolumes):
Line 273:                     log.error("Image %s can't be activated.",
Line 274:                               imgUUID, exc_info=True)
Line 275: 
Line 276:                 for vol in sd.getVolsOfImage(allVolumes, imgUUID):
you're already calling getVolsOfImage here, do it separately before activateVolumes and pass that
Line 277:                     try:
Line 278:                         v3ResetMetaVolSize(vol)  # BZ#811880
Line 279:                     except qemuImg.QImgError:
Line 280:                         log.error("It is not possible to read the volume %s "


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Id47db9c53199385dd5d08586e4782ea23885eb72
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Elad Ben Aharon <eladba1990 at gmail.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Haim Ateya <hateya at redhat.com>
Gerrit-Reviewer: Igor Lvovsky <ilvovsky at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list