Change in vdsm[master]: when boostraping lvm refresh/activate also LVs with special ...

nsoffer at redhat.com nsoffer at redhat.com
Tue May 20 11:08:53 UTC 2014


Nir Soffer has posted comments on this change.

Change subject: when boostraping lvm refresh/activate also LVs with special tags
......................................................................


Patch Set 2:

(3 comments)

http://gerrit.ovirt.org/#/c/27880/2/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 660: 
Line 661:         if activate:
Line 662:             log.info("Activating lvs: vg=%s lvs=%s", vg.name, activate)
Line 663:             try:
Line 664:                 activateLVs(vg.name, activate)
Please use the lower level _setLVAvailability with the "y" parameter. At this point we already know that the lvs are not active, and there is no need to check that again as done inside the public activateLVs() function.
Line 665:             except se.CannotActivateLogicalVolumes:
Line 666:                 log.error("Error activating lvs: vg=%s lvs=%s", vg.name,
Line 667:                           activate)
Line 668: 


Line 663:             try:
Line 664:                 activateLVs(vg.name, activate)
Line 665:             except se.CannotActivateLogicalVolumes:
Line 666:                 log.error("Error activating lvs: vg=%s lvs=%s", vg.name,
Line 667:                           activate)
At this point our lvm metadata cache may be out of sync with the actual lvs, and must be invalidated - like we do when deactivating lvs.

    _lvminfo._invalidatelvs(vg.name, deactivate)

There is duplication here, that can be fix with some refactoring in later patch.
Line 668: 
Line 669:         if deactivate:
Line 670:             log.info("Deactivating lvs: vg=%s lvs=%s", vg.name, deactivate)
Line 671:             try:


Line 668: 
Line 669:         if deactivate:
Line 670:             log.info("Deactivating lvs: vg=%s lvs=%s", vg.name, deactivate)
Line 671:             try:
Line 672:                 deactivateLVs(vg.name, deactivate)
This change is not related to your change, and incorrect (see above why we use _setLVAvailability).
Line 673:             except se.CannotDeactivateLogicalVolumes:
Line 674:                 log.error("Error deactivating lvs: vg=%s lvs=%s", vg.name,
Line 675:                           deactivate)
Line 676:             # Some lvs are inactive now


-- 
To view, visit http://gerrit.ovirt.org/27880
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I850bf500e8eabfe414a6d6920155ac0697fe5604
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Jiří Moskovčák <jmoskovc at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Martin Sivák <msivak at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer 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