Xavi Francisco has posted comments on this change. Change subject: blockSD: raise an exception if missing physical volume ......................................................................
Patch Set 4: (2 comments) There are two other methods that use getPV: __processVGInfos() and _getDeviceList() both in hsm.py. The second one already expects a StorageException when getPV should fail so this patch corrects this. In the first method there is no check whether the getPV returned None or not so in this case the added exception is needed as well. http://gerrit.ovirt.org/#/c/27442/4/vdsm/storage/blockSD.py File vdsm/storage/blockSD.py: Line 669: pv = lvm.getPV(dev) Line 670: except se.InaccessiblePhysDev: Line 671: cls.log.error("INTERNAL: Unable to get metadata from PV: %s", Line 672: dev) Line 673: raise > Please remove this log and let this error bubble up the engine - we will ha OK, I just tried to give some insight in the VDSM logs about what happened and where. I can remove it but I added it for clarity purposes. Line 674: Line 675: pvInfo = {} Line 676: pvInfo["guid"] = os.path.basename(pv.name) Line 677: pvInfo["uuid"] = pv.uuid http://gerrit.ovirt.org/#/c/27442/4/vdsm/storage/lvm.py File vdsm/storage/lvm.py: Line 824: def getPV(pv): Line 825: pv = _lvminfo.getPv(_fqpvname(pv)) Line 826: if not pv: Line 827: raise se.InaccessiblePhysDev(pv) Line 828: else: > +1 I do not like it either, but I tried to be consistent with the same behaviour of getVG(). I'll change it but another patch should be added to change the style of the getVG call (to be consistent at least) Line 829: return pv Line 830: Line 831: Line 832: def getAllPVs(): -- To view, visit http://gerrit.ovirt.org/27442 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I9932b044c8b439dc8b1f09191a5d89f4bc44c38a Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Xavi Francisco <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Xavi Francisco <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
