Change in vdsm[ovirt-3.3]: Revert "clientIF: rescan devices on failed hotplugDisk"

danken at redhat.com danken at redhat.com
Thu Apr 3 17:42:51 UTC 2014


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/1066413
Signed-off-by: Nir Soffer <nsoffer at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/21173
Reviewed-by: Ayal Baron <abaron at redhat.com>
Reviewed-on: http://gerrit.ovirt.org/25240
Reviewed-by: Allon Mureinik <amureini at redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce at redhat.com>
---
M vdsm/clientIF.py
M vdsm/storage/hsm.py
2 files changed, 24 insertions(+), 21 deletions(-)

Approvals:
  Nir Soffer: Verified
  Federico Simoncelli: Looks good to me, approved
  Allon Mureinik: Looks good to me, but someone else must approve



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

Gerrit-MessageType: merged
Gerrit-Change-Id: Ib978fada8ee82e3c6da9de9b79a66edafba4eb70
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini 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: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org


More information about the vdsm-patches mailing list