Nir Soffer has posted comments on this change.

Change subject: storage: ignore missing pv metadata
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.ovirt.org/#/c/27442/1/vdsm/storage/blockSD.py
File vdsm/storage/blockSD.py:

Line 670:             pv = lvm.getPV(dev)
Line 671:             if not pv:
Line 672:                 cls.log.warning("Could not get metadata from the 
device: %s",
Line 673:                                 dev)
Line 674:                 continue
Why do we get into this corner, when a pv does not have meta data. can you 
reproduce this?

The solution, returning None from a function is not a good idea in general - 
this is something that we do in C when we have no other choice, but if this is 
not a normal condition, the right thing would be to raise an exception here.

I'm not sure that this is the right place to fix this, and I suspect that this 
should be handled in the point where we pvs fail and we get no metadata. Maybe 
we should remove this pv from our metadata cache that this point.

Since I don't have time to research this now, I cannot do a complete review of 
this change at this time.
Line 675:             pvInfo = {}
Line 676:             pvInfo["guid"] = os.path.basename(pv.name)
Line 677:             pvInfo["uuid"] = pv.uuid
Line 678:             # this is another trick, it's not the


-- 
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: 1
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: [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

Reply via email to