Change in vdsm[master]: Revert "clientIF: rescan devices on failed hotplugDisk"

danken at redhat.com danken at redhat.com
Thu Dec 26 10:28:08 UTC 2013


Dan Kenigsberg has submitted this change and it was merged.

Change subject: Revert "clientIF: rescan devices on failed hotplugDisk"
......................................................................


Revert "clientIF: rescan devices on failed hotplugDisk"

Commit 15c7f74365cb5d2c9258bc333c441941c6e42bdb tried fix a case where
device is not visible after hotplug. The solution included a call to
multipath.rescan() if the device does not exists, and checking again if
the device exists. However, due to sneaky bug in the fix, the code was
never invoking multipath.rescan(), and was actually a Placebo.

The original commit added an undocumented public method for scanning
devices visibility. The method did not perform readability check, like
getDevicesVisiblity; there is no documentation or other evidence why
this solution was chosen.

The original commit tried to improve the way devices are checked for
visibility, checking if all devices exists after multipath.rescan().
However this improvement was canceled because multipath.resacn() was
never running.

The original code was calling os.stat() once per device.  After the
commit, we call os.path.exists() for all devices, and os.stat() for
existing devices.  This additional calls are pointless and introduces a
race, when a devices exist in the first call, and disappear in the
second.  Since the commit remove the exception handling while calling
os.stat(), getDevicesVisibility may fail now with OSError in this case.

One successful change was improving logging in clientIF, but this should
have never be committed with the rest of the changes.

This is the first patch in series of patches fixing the regressions
introduced by the original commit and adding the good changes.

This reverts commit 15c7f74365cb5d2c9258bc333c441941c6e42bdb.

Change-Id: Ib978fada8ee82e3c6da9de9b79a66edafba4eb70
Relates-To: https://bugzilla.redhat.com/923773
Bug-Url: https://bugzilla.redhat.com/1044068
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/21173
Reviewed-by: Ayal Baron <abaron at redhat.com>
---
M vdsm/clientIF.py
M vdsm/storage/hsm.py
2 files changed, 24 insertions(+), 21 deletions(-)

Approvals:
  Ayal Baron: Looks good to me, approved
  Nir Soffer: Verified



-- 
To view, visit http://gerrit.ovirt.org/21173
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib978fada8ee82e3c6da9de9b79a66edafba4eb70
Gerrit-PatchSet: 6
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: Federico Simoncelli <fsimonce 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-Reviewer: oVirt Jenkins CI Server


More information about the vdsm-patches mailing list