Federico Simoncelli has posted comments on this change. Change subject: hsm: [wip] protect deleteImage with the spm lock ......................................................................
Patch Set 1: (3 comments) .................................................... File vdsm/storage/hsm.py Line 1540: else: Line 1541: if fakeTUUID: Line 1542: tParams = dom.produceVolume(imgUUID, fakeTUUID).\ Line 1543: getVolumeParams() Line 1544: pool.deleteImage(dom, imgUUID, volsByImg) Even though this is the bare minimum, we can try to investigate better the comment here below and move this block (lines 1534-1556) to sp.py. Line 1545: # This is a hack to keep the interface consistent Line 1546: # We currently have race conditions in delete image, to quickly fix Line 1547: # this we delete images in the "synchronous" state. This only works Line 1548: # because Engine does not send two requests at a time. This hack is .................................................... File vdsm/storage/sp.py Line 2019: for volUUID in volumes: Line 2020: sdCache.produce(sdUUID).produceVolume(imgUUID, volUUID).delete( Line 2021: postZero=postZero, force=force) Line 2022: Line 2023: def deleteImage(self, domain, imgUUID, volsByImg): We can use the domain object here because we don't schedule it as a task. Might worth commenting. I'm also fine with using sdUUID and produce the domain here again. Line 2024: """ Line 2025: TODO: describe method Line 2026: """ Line 2027: domain.deleteImage(domain.sdUUID, imgUUID, volsByImg) Line 2023: def deleteImage(self, domain, imgUUID, volsByImg): Line 2024: """ Line 2025: TODO: describe method Line 2026: """ Line 2027: domain.deleteImage(domain.sdUUID, imgUUID, volsByImg) Why do we need to pass the sdUUID? The domain object says it all. Also volsByImg are redundant but I understand that getting rid of them would require a larger refactoring (e.g lvm commands can be applied by to multiple lvs using tags, eg: lvremove @IU_imgUUID, provided that we're able to filter also by vg). Line 2028: Line 2029: def setMaxHostID(self, spUUID, maxID): Line 2030: """ Line 2031: Set maximum host ID -- To view, visit http://gerrit.ovirt.org/19795 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iac5b4b0f71de6a12de34513d4fafb295b701306c Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: Ayal Baron <aba...@redhat.com> Gerrit-Reviewer: Federico Simoncelli <fsimo...@redhat.com> Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
