Change in vdsm[master]: lvm: Raise an exception if missing physical volume

nsoffer at redhat.com nsoffer at redhat.com
Thu Mar 5 10:48:29 UTC 2015


Nir Soffer has posted comments on this change.

Change subject: lvm: Raise an exception if missing physical volume
......................................................................


Patch Set 9:

(2 comments)

https://gerrit.ovirt.org/#/c/27442/9//COMMIT_MSG
Commit Message:

Line 8: 
Line 9: Raise InaccessiblePhysDev when getting a pv which is not in lvm cache.
Line 10: 
Line 11: When lvm.getPV() tries to get a cached PV object from its cache, there
Line 12: is a possibility that the physical volume is not accessible anymore. In
is not accessible -> is not in the cache

What why a missing device may happen:

Basically we do:

   for pvName in lvm.listPVNames(vgName):
       pv = lvm.getPV(pvName)

While we iterate, lvm cache may change (another thread may invalidate the cache).

Typically we don't expect pvs to be removed, and if
this happens we want to warn about it.
Line 13: such a case, we should raise a relevant storage exception, instead of
Line 14: letting the code fail at some random point later when trying to access
Line 15: an attribute of None object.
Line 16: 


Line 18:  * It updates hsm._getDeviceList() to work with the new API.
Line 19:  * hsm.__processVGInfos() and blockSD.getMetadataMapping() did not
Line 20:  handle the case of a missing pv which caused the flow to fail with
Line 21:  AttributeError. Now they do not fail and log a warning about missing
Line 22:  pvs.
We do not need these details, we can see this in the code.

What we want to tell is:

- lvm.getPV() raises now InaccessiblePhysDev instead of returning None
- Callers updated to log a warning when a device is not the cache, which is similar to the existing behavior of hsm._getDeviceList()

Lets separate error handling in hsm.__processVGInfos() and blockSD.getMetadataMapping() to another patch. Letting them fail with a relevant error is way better then the current behavior, and we may need to check if this failure is expected in these flows.
Line 23: 
Line 24: Change-Id: I9932b044c8b439dc8b1f09191a5d89f4bc44c38a
Line 25: Bug-Url: https://bugzilla.redhat.com/1048696
Line 26: Signed-off-by: Xavi Francisco <xfrancis at redhat.com>


-- 
To view, visit https://gerrit.ovirt.org/27442
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9932b044c8b439dc8b1f09191a5d89f4bc44c38a
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Xavi Francisco <xfrancis at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Ala Hino <ahino at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: Xavi Francisco <xfrancis at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list