Change in vdsm[master]: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API

ishaby at redhat.com ishaby at redhat.com
Tue Aug 11 14:29:24 UTC 2015


Idan Shaby has posted comments on this change.

Change subject: lvm: Update hsm.__processVGInfos() with lvm.getPV() new API
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.ovirt.org/#/c/38421/1/vdsm/storage/hsm.py
File vdsm/storage/hsm.py:

Line 3003:                 try:
Line 3004:                     pvInfo = lvm.getPV(pv)
Line 3005:                 except se.InaccessiblePhysDev:
Line 3006:                     self.log.error("PV %s no longer exists", pv)
Line 3007:                     continue
> What if you catch InaccessiblePhysDev and you take the entire vg out of the
I ran a few tests. Here are the results when trying to add an iscsi domain:

1. A test that makes getPV in lvm.py always fail (raise an InaccessiblePhysDev), which is the scenario that can most probably happen today - fails with the error "Error while executing action New SAN Storage Domain: Multipath cannot access physical device".
There was no db pollution.

2. A test that makes getPV in lvm.py fail only for a specific pv - same error is shown, and no db pollution.

3. A test that removes the vg out of the vg list (what Fede suggested) - the storage domain is created, a new record is added to the storage_domain_static table but the lun (the only one) is not added to the luns table.
That means that we saved a partial information in the db (db pollution).
If you create another storage domain, you will see the luns that you used earlier as free luns (not greyed out), and if you try to edit the storage domain that we created, you will get a bunch of java.lang.NullPointerException-s.

I am abandoning the patch.
Line 3008:                 vgInfo['pvlist'].append(self.__fillPVDict(dev, pvInfo, vgType))
Line 3009: 
Line 3010:             if vgType == multipath.DEV_FCP:
Line 3011:                 vgType = sd.FCP_DOMAIN


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I583c0493093d2c9c8bca8713df8ee123c415de7f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Idan Shaby <ishaby 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: Candace Sheremeta <cshereme at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Freddy Rolland <frolland at redhat.com>
Gerrit-Reviewer: Idan Shaby <ishaby at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Tal Nisan <tnisan at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list