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 18:57:04 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:

> Please, consider instead of completely revert the original patch (not yours Nir) just call getDeviceVisibility() instead of ScanDeviceVisibility() from clientIF the only place which is using Scan. API.. directly. Then Scan... can become a private method of hsm.

I think the reason the scanDevicesVisibility is called instead of getDeviesVisibility, is the former check only if the device exists, and the later checks also readability. In clientIF.py, the code first check if the device exists, and then try to change ownership of the device. I guess that the author wanted to keep that behavior.

Practically, commit 15c7f74365cb5 did not fix anything, since before and after this commit the code check if device exists, and never invoke multipath rescan. It however introduced regression in getDevicesVisibility, because multipath rescan is also never invoked there. Finally it introduced a race by calling os.stat twice per device, see http://gerrit.ovirt.org/#/c/21053/.

So what I plan to do is to create a new patch that:

1. Remove the changes in clientIF.py, as they did not fix anything and the bug was verified, so multipath rescan is probably not needed there. This was a Placebo, not a bug fix :-)

2. Merge fixed, scanDevicesVisibility and getDevicesVisibility, fixing both multipath rescan issue and double stat race. If someone prove that we need public function for checking devices existence, we will add it later in a separate patch.

What do you think?

-- 
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: No


More information about the vdsm-patches mailing list