Nir Soffer has posted comments on this change.
Change subject: Invalidate filters on HSMs before rescanning extended VG ......................................................................
Patch Set 17:
(3 comments)
I think that this fix is correct. The only thing holding this now is more clear commit message. If you use the suggested commit message I'm sure Dan will be happy to merge this soon.
.................................................... Commit Message Line 3: AuthorDate: 2013-10-25 11:10:39 +0200 Line 4: Commit: Pavel Zhukov pzhukov@redhat.com Line 5: CommitDate: 2013-11-04 09:39:47 +0100 Line 6: Line 7: Invalidate filters on HSMs before rescanning extended VG Lets use:
hsm: Invalidate lvm filter before extending a volume group Line 8: Line 9: domainMonitor thread might use stale lvm filters while running vgs command Line 10: after extending of the VG (added PV is missed in the filter set). Line 11: Since LvmCache doesn't invalidate filters, the volume
Line 16: To avoid perfomance issues the patch introduces public method for filter invalidation Line 17: and calls it from the getDevicesVisibility. As far as getDevicesVisibility Line 18: is called to all added devices this allows to reload filters every time Line 19: after new devices has been mapped but before vgscan. Line 20: Lets use something like:
lvm module maintains a cache of the global filter, used to speed up lvm operations. The cache becomes stale after we extend a volume group with a new physical volume, leading to failure in the domain monitor thread, where a volume group is found partial.
The current code try to invalidate the filter when lvm command fails, but this logic is not good enough for commands such as vgs do not return nonzero exit code when a vg is found partial.
This patch invalidates the lvm filter cache before extending a volume group, ensuring that the next lvm command will build up a new filter, and would not report a volume group as partial when it is actually not.
The change is made in getDevicesVisibility(), since it is called before extending a volume, to make sure that all hosts are seeing the new physical volume. Line 21: Change-Id: If1eeed1c203f2c8c73370987048565d665932299 Line 22: Bugzilla-Url: https://bugzilla.redhat.com/1022976
.................................................... File vdsm/storage/hsm.py Line 2004: boolean Line 2005: :rtype: dict Line 2006: """ Line 2007: visibility = self.scanDevicesVisibility(guids) Line 2008: lvm.invalidateFilter() This is not critical, but I think the lvm.invalidateFilter() call should be before the visibility scan.
There are few reasons: - This is important side effect, and we don't want to hide in the middle of the visibility processing - You don't want to break the visibility logic anyway. - It will merge more cleanly into 3.2 Line 2009: for guid in guids: Line 2010: if visibility[guid]: Line 2011: visibility[guid] = (os.stat('/dev/mapper/' + guid).st_mode & Line 2012: stat.S_IRUSR != 0)