Dan Kenigsberg has posted comments on this change.
Change subject: Invalidate filters on HSMs before rescanning extended VG ......................................................................
Patch Set 16: Code-Review-1
(6 comments)
.................................................... Commit Message Line 5: CommitDate: 2013-10-30 20:01:34 +0100 Line 6: Line 7: Invalidate filters on HSMs before rescanning extended VG Line 8: Line 9: domainMonitor thread might use stale filters while running vgs command filters in plural, or the global filter? Line 10: after extending of the VG. As a result LvmCache doesn't invalidate Line 11: stale filters, volume group is marked as partial and selftest fails Line 12: (host goes to non-operational status). By design filter should be Line 13: invalidated if cmd returns nonzero code but vgs returns zero even if
Line 6: Line 7: Invalidate filters on HSMs before rescanning extended VG Line 8: Line 9: domainMonitor thread might use stale filters while running vgs command Line 10: after extending of the VG. As a result LvmCache doesn't invalidate English sentence is not clear: As a result LvmCache...?? Did you mean "Since LvmCache doesn't invalidate the stale filter," Line 11: stale filters, volume group is marked as partial and selftest fails Line 12: (host goes to non-operational status). By design filter should be Line 13: invalidated if cmd returns nonzero code but vgs returns zero even if Line 14: devices are filtered . The only way to invalidate filter now is flush LvmCache.
Line 8: Line 9: domainMonitor thread might use stale filters while running vgs command Line 10: after extending of the VG. As a result LvmCache doesn't invalidate Line 11: stale filters, volume group is marked as partial and selftest fails Line 12: (host goes to non-operational status). By design filter should be maybe: "currently, the filter is invalidated if cmd returns nonzero code but vgs returns zero even if devices are filtered out." Line 13: invalidated if cmd returns nonzero code but vgs returns zero even if Line 14: devices are filtered . The only way to invalidate filter now is flush LvmCache. Line 15: To avoid perfomance issues the patch introduce public method for filter invalidation Line 16: and calls it from the getDevicesVisibility instead of _reloadvgs.
Line 11: stale filters, volume group is marked as partial and selftest fails Line 12: (host goes to non-operational status). By design filter should be Line 13: invalidated if cmd returns nonzero code but vgs returns zero even if Line 14: devices are filtered . The only way to invalidate filter now is flush LvmCache. Line 15: To avoid perfomance issues the patch introduce public method for filter invalidation keep lines shorter, introduce->introduces Line 16: and calls it from the getDevicesVisibility instead of _reloadvgs. Line 17: Line 18: Change-Id: If1eeed1c203f2c8c73370987048565d665932299 Line 19: Bugzilla-Url: https://bugzilla.redhat.com/1022976
Line 12: (host goes to non-operational status). By design filter should be Line 13: invalidated if cmd returns nonzero code but vgs returns zero even if Line 14: devices are filtered . The only way to invalidate filter now is flush LvmCache. Line 15: To avoid perfomance issues the patch introduce public method for filter invalidation Line 16: and calls it from the getDevicesVisibility instead of _reloadvgs. The commit message lacks an explanation on why it makes sense to invalidate the filter on getDevicesVisibility. Line 17: Line 18: Change-Id: If1eeed1c203f2c8c73370987048565d665932299 Line 19: 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() That's a good explanation, that should appear in the commit message. 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)