Nir Soffer has posted comments on this change. Change subject: hsm: Rescan multipath if device is missing when scanning devices ......................................................................
Patch Set 3: (3 comments) .................................................... Commit Message Line 19: Line 20: This patch fixes the regression and adds the missing unitests, hopefully Line 21: preventing the next regression in this hard to verify code. Line 22: Line 23: Relates-To: https://bugzilla.redhat.com/923773 > No its not! Do you think that this patch is related to 1022976? how? > The major difference between them is that Eduardo acked 923773 so its in and > nacked 1022976 so that one became to be "famous". > Now seriously. 923773 suspects that there is a race in the multipath so it > suggests to scan device visibility before hotplug, not bad idea at all. Maybe > calling to getDeviceVisibility() is even better idea. The only difference between getDevicesVisibility and scanDevicesVisibility is that getDevicesVisibility also check if a device is readable and scanDevicesVisibility check only that the devices exists. I don't see how all this discussion is related to this patch and I don't think it help us to make progress. Line 24: Change-Id: I00d5be2f73b644c194625e08654784e0aad64aee .................................................... File vdsm/storage/hsm.py Line 1981: Line 1982: return devices Line 1983: Line 1984: @public Line 1985: def scanDevicesVisibility(self, guids): > there is no customer between getDeviceList() and getDeviceVisibility() What? Line 1986: def visible(guid): Line 1987: path = os.path.join("/dev/mapper", guid) Line 1988: return os.path.exists(path) Line 1989: visibility = map(visible, guids) Line 1981: Line 1982: return devices Line 1983: Line 1984: @public Line 1985: def scanDevicesVisibility(self, guids): I don't know why you think that commit 15c7f74365cb5 did not solve bz 923773, but it does not matter for this patch. scanDevicesVisibility is used by getDevicesVisibility and it should be fixed. This fix is also needed for 3.2. You don't want to revert 15c7f74365cb5 because the old code it replaced had it own problems, like hiding os.stat errors with empty except: and not checking all devices again after a multipath rescan. Line 1986: def visible(guid): Line 1987: path = os.path.join("/dev/mapper", guid) Line 1988: return os.path.exists(path) Line 1989: visibility = map(visible, guids) -- To view, visit http://gerrit.ovirt.org/20942 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I00d5be2f73b644c194625e08654784e0aad64aee Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Ayal Baron <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Sergey Gotliv <[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
