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 <[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: No _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
