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 11:29:53 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
This is the bug that commit 15c7f74365cb5 fixes. Maybe replace with http://gerrit.ovirt.org/#/c/13454?

The famous bug (1022976) is reported with the vdsm-4.10.2-23.0, containing commit 15c7f74365cb5, but the logs attached to that patch show that all devices were visible when getDeviesVisibility was called, therefore the code path where multipath rescan is invoked is not related.
Line 24: Change-Id: I00d5be2f73b644c194625e08654784e0aad64aee


....................................................
File tests/hsmTests.py
Line 52:         self.calls = ''
Line 53: 
Line 54:     def rescan(self):
Line 55:         self.calls += 'rescan;'
Line 56:         os.path.exists.pathes = self.pathes
Good point - this will leave new "pathes" attribute on the real os.path.exists if multipath rescan is called when os.path.exists is not monkeypatched. I'll fix it in the next patch.
Line 57: 
Line 58: 
Line 59: @monkeypatch.MonkeyClass(hsm, 'multipath', FakeMultipath())
Line 60: class ScanDevicesVisibilityTests(TestCaseBase):


....................................................
File vdsm/storage/hsm.py
Line 1981: 
Line 1982:         return devices
Line 1983: 
Line 1984:     @public
Line 1985:     def scanDevicesVisibility(self, guids):
Like you I can read code and the comments in gerrit, but this patch is not trying to change the behavior suggested in that patch, but to fix the wrong implementation.

I don't know why scanDevicesVisibility has to be public - this is out of the scope of this patch.
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