Change in vdsm[master]: image: copying a template is always allowed

abaron at redhat.com abaron at redhat.com
Sat Oct 13 20:26:32 UTC 2012


Ayal Baron has posted comments on this change.

Change subject: image: copying a template is always allowed
......................................................................


Patch Set 2: (3 inline comments)

....................................................
File vdsm/storage/hsm.py
Line 1298: 
Line 1299:         Copying an image based on a template to a data domain is allowed only
Line 1300:         if the template already exists on the target domain.
Line 1301:         """
Line 1302:         dstAllVols = dstDom.getAllVolumes()
it seems odd that you'd pass srcAllVols but not dstAllVols?
Line 1303: 
Line 1304:         for volName, volImages in self.__imageVolumes(srcAllVols, imgUUID):
Line 1305:             if (len(volImages) > 1 and imgUUID != volImages[0]
Line 1306:                     and volName not in dstAllVols.keys()):


Line 1301:         """
Line 1302:         dstAllVols = dstDom.getAllVolumes()
Line 1303: 
Line 1304:         for volName, volImages in self.__imageVolumes(srcAllVols, imgUUID):
Line 1305:             if (len(volImages) > 1 and imgUUID != volImages[0]
I'm not sure that this is readable enough without at least a comment stating that if volImages is greater then one then first element of volImages is the template image.
And that this test is to make sure that target domain has the template image to make sure image would be complete on destination
Line 1306:                     and volName not in dstAllVols.keys()):
Line 1307:                  self.log.error("Cannot move image %s to domain %s because "
Line 1308:                             "the template %s is missing on the destination",
Line 1309:                             imgUUID, dstDom.sdUUID, volName)


Line 1339: 
Line 1340:         # Validates that the pool is connected. WHY?
Line 1341:         pool = self.getPool(spUUID)
Line 1342: 
Line 1343:         if op != image.COPY_OP:
why negative test and not op == image.MOVE_OP ?
Line 1344:             self.validateImageDelete(srcDom, srcAllVols, imgUUID)
Line 1345: 
Line 1346:         self.validateImageCopy(srcAllVols, dstDom, imgUUID)
Line 1347: 


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I9e07d569eec02ac4dcd386a7c576342f640ec242
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Eduardo <ewarszaw at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Shu Ming <shuming at linux.vnet.ibm.com>


More information about the vdsm-patches mailing list