Adam Litke has posted comments on this change.

Change subject: blockSD: Avoid stale lvs
......................................................................


Patch Set 3:

(4 comments)

https://gerrit.ovirt.org/#/c/56876/3/vdsm/storage/lvm.py
File vdsm/storage/lvm.py:

Line 653:     for vg in _lvminfo.getAllVgs():
Line 654:         deactivateUnusedLVs(vg.name, refreshlvs=refreshlvs)
Line 655: 
Line 656: 
Line 657: def deactivateUnusedLVs(vgname, refreshlvs=()):
refreshlvs is a confusing parameter name.  To me it suggests that these lvs 
should be refreshed and deactivated.  Perhaps 'excludelvs' or 'keeplvs' would 
be clearer.
Line 658:     deactivate = []
Line 659:     refresh = []
Line 660: 
Line 661:     for lv in _lvminfo.getLv(vgname):


Line 672:         try:
Line 673:             _setLVAvailability(vgname, deactivate, "n")
Line 674:         except se.CannotDeactivateLogicalVolume:
Line 675:             log.error("Error deactivating lvs: vg=%s lvs=%s", vgname,
Line 676:                       deactivate)
Since stale lvs could lead to data corruption, should this cause a failure in 
the setup lifecycle event?  Perhaps we should not permit this host to use this 
domain if it cannot be managed properly here.
Line 677:         # Some lvs are inactive now
Line 678:         _lvminfo._invalidatelvs(vgname, deactivate)
Line 679: 
Line 680:     if refresh:


Line 681:         log.info("Refreshing lvs: vg=%s lvs=%s", vgname, refresh)
Line 682:         try:
Line 683:             refreshLVs(vgname, refresh)
Line 684:         except se.LogicalVolumeRefreshError:
Line 685:             log.error("Error refreshing lvs: vg=%s lvs=%s", vgname, 
refresh)
Same here?
Line 686: 
Line 687: 
Line 688: def invalidateCache():
Line 689:     _lvminfo.invalidateCache()


https://gerrit.ovirt.org/#/c/56876/3/vdsm/storage/sd.py
File vdsm/storage/sd.py:

Line 503:     # Life cycle
Line 504: 
Line 505:     def setup(self):
Line 506:         """
Line 507:         Called when after storage domain is produced in the storage 
domain
Called after
Line 508:         monitor.
Line 509:         """
Line 510: 
Line 511:     def teardown(self):


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7227bb43c2e1ee67a6239956aae48173a27f566e
Gerrit-PatchSet: 3
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Adam Litke <ali...@redhat.com>
Gerrit-Reviewer: Ala Hino <ah...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Freddy Rolland <froll...@redhat.com>
Gerrit-Reviewer: Idan Shaby <ish...@redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stira...@redhat.com>
Gerrit-Reviewer: gerrit-hooks <automat...@ovirt.org>
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to