Change in vdsm[ovirt-3.3]: clientIF: Rescan devices if device not found after vmHotplug...

nsoffer at redhat.com nsoffer at redhat.com
Sun Mar 2 13:39:20 UTC 2014


Hello Ayal Baron, Federico Simoncelli, Allon Mureinik,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/25243

to review the following change.

Change subject: clientIF: Rescan devices if device not found after vmHotplugDisk
......................................................................

clientIF: Rescan devices if device not found after vmHotplugDisk

According to the analysis in the bug, invoking vmHotplugDisk right after
connectStorageServer may not leave enough time for multipathd to create
the device, leading to unnecessary failure.

As I understand, multipathd is receiving events from udev and creating
devices. The race is probably related to the asynchronous nature of this
mechanism. Invoking multipath.rescan() force synchronous update of the
found devices.

The original fix in commit 15c7f74365cb5d2c9258bc333c441941c6e42bdb
tried to to invoke multipath.rescan() if the device was not found. Due
to a unfortunate error, multipath.rescan() was never invoked.  Although
the bug was never fixed, QE verified it and customers are using this
code for the last 6 month.  We believe that the problem is a tiny race
that is unlikely to happen, but it is possible and should be fixed.

This patch implements the original fix in a different way, calling
getDevicesVisibility(), which invokes multipath.rescan() if a device is
missing.

Change-Id: I3d75ac53ca809b7a25d7fb237ec661dd865e31fc
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/21181
Reviewed-by: Ayal Baron <abaron at redhat.com>
Reviewed-by: Allon Mureinik <amureini at redhat.com>
Reviewed-by: Federico Simoncelli <fsimonce at redhat.com>
---
M vdsm/clientIF.py
1 file changed, 4 insertions(+), 3 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/43/25243/1

diff --git a/vdsm/clientIF.py b/vdsm/clientIF.py
index 48c88a8..d08ef9d 100644
--- a/vdsm/clientIF.py
+++ b/vdsm/clientIF.py
@@ -286,15 +286,16 @@
 
             # GUID drive format
             elif "GUID" in drive:
-                volPath = os.path.join("/dev/mapper", drive["GUID"])
-
-                if not os.path.exists(volPath):
+                res = self.irs.getDevicesVisibility([drive["GUID"]])
+                if not res["visible"][drive["GUID"]]:
                     raise vm.VolumeError(drive)
 
                 res = self.irs.appropriateDevice(drive["GUID"], vmId)
                 if res['status']['code']:
                     raise vm.VolumeError(drive)
 
+                volPath = os.path.join("/dev/mapper", drive["GUID"])
+
             # UUID drive format
             elif "UUID" in drive:
                 volPath = self._getUUIDSpecPath(drive["UUID"])


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I3d75ac53ca809b7a25d7fb237ec661dd865e31fc
Gerrit-PatchSet: 1
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: Ayal Baron <abaron at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>


More information about the vdsm-patches mailing list