Change in vdsm[master]: hsm: Rescan multipath if device is missing when scanning dev...

nsoffer at redhat.com nsoffer at redhat.com
Fri Nov 8 17:31:16 UTC 2013


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 <nsoffer at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Sergey Gotliv <sgotliv at redhat.com>
Gerrit-Reviewer: Yeela Kaplan <ykaplan at redhat.com>
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list