Eduardo has posted comments on this change. Change subject: [WIP] destroy storage pool using command type 1 ......................................................................
Patch Set 1: Code-Review-1 (3 comments) http://gerrit.ovirt.org/#/c/24398/1/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 1366: self.log.error( Line 1367: "Can't remove pool %s from domain %s pool list %s, " Line 1368: "it does not exist", Line 1369: spUUID, self.sdUUID, str(pools)) Line 1370: return Use an else: block please instead this ugly return. Line 1371: vgUUID = self.getInfo()['vguuid'] Line 1372: vg = lvm.getVGbyUUID(vgUUID) Line 1373: vgName = vg.name Line 1374: toAdd = encodeVgTags({sd.DMDK_POOLS: '', Line 1369: spUUID, self.sdUUID, str(pools)) Line 1370: return Line 1371: vgUUID = self.getInfo()['vguuid'] Line 1372: vg = lvm.getVGbyUUID(vgUUID) Line 1373: vgName = vg.name This block (3 lines) seems to me to be an echo from code a long time gone. vgName is self.sdUUID. Please remove this redundant calls. Line 1374: toAdd = encodeVgTags({sd.DMDK_POOLS: '', Line 1375: sd.DMDK_ROLE: sd.REGULAR_DOMAIN}) Line 1376: toDel = encodeVgTags({sd.DMDK_POOLS: spUUID, Line 1377: sd.DMDK_ROLE: sd.MASTER_DOMAIN}) Line 1374: toAdd = encodeVgTags({sd.DMDK_POOLS: '', Line 1375: sd.DMDK_ROLE: sd.REGULAR_DOMAIN}) Line 1376: toDel = encodeVgTags({sd.DMDK_POOLS: spUUID, Line 1377: sd.DMDK_ROLE: sd.MASTER_DOMAIN}) Line 1378: lvm.changeVGTags(vgName, delTags=toDel, addTags=toAdd, safe=False) Since detachMaster is called only from hsm.destroyStoragePool() and the domain was already removed from the pool MD, you can avoid here the use of changeVGtags replacing it by addVGTags and remVGTags as was discussed in a previous patch. The operations will be logically atomic and no one should try ever to access the domain between the removal and the adiditon of the role tag. You can lock the master domain at hsm.destroyStoragePool() to be sure and this covers the previous forceDetach() call. Line 1379: Line 1380: def refresh(self): Line 1381: self.refreshDirTree() Line 1382: lvm.invalidateVG(self.sdUUID) -- To view, visit http://gerrit.ovirt.org/24398 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I67cda9abd0bbc01d7d0642d5d3327f8687d7f728 Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Yeela Kaplan <ykap...@redhat.com> Gerrit-Reviewer: Eduardo <ewars...@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