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) -- To view, visit http://gerrit.ovirt.org/20552 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If1eeed1c203f2c8c73370987048565d665932299 Gerrit-PatchSet: 16 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Pavel Zhukov <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Eduardo <[email protected]> Gerrit-Reviewer: Federico Simoncelli <[email protected]> Gerrit-Reviewer: Lee Yarwood <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Pavel Zhukov <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[email protected]> Gerrit-Reviewer: Tomáš Došek <[email protected]> Gerrit-Reviewer: Yeela Kaplan <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
