Change in vdsm[master]: scsi: Scan only the required domain type

[Nir Soffer]

Nir Soffer has uploaded a new change for review.

Change subject: scsi: Scan only the required domain type
......................................................................

scsi: Scan only the required domain type

We used to perform both iSCSI and FCP rescan when creating or editing a
storage domain, connecting to storage server, getting vg and storage
domain list and more.

The unneeded rescan is typically fast, but if a storage server or device
is not accessible, a SCSI rescan may block for couple of minutes,
leading to unwanted blocking of unrelated storage threads. This is
particularly bad when you are interested only in one domain type, but
the host get stuck scanning the other type.

To improve storage domain isolation, we use the specified storage type
to perform a rescan only of the relevant type. If storage type was not
specified, we scan both ISCSI and FCP keeping the old behavior.

Change-Id: Ic32cd683020e94df016dd77b19ae3eb7317c5554
Signed-off-by: Nir Soffer 
---
M vdsm/storage/hsm.py
M vdsm/storage/multipath.py
M vdsm/storage/sdc.py
3 files changed, 25 insertions(+), 17 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/24/45824/1

diff --git a/vdsm/storage/hsm.py b/vdsm/storage/hsm.py
index 1b8c064..541b699 100644
--- a/vdsm/storage/hsm.py
+++ b/vdsm/storage/hsm.py
@@ -1984,15 +1984,16 @@
         return dict(devList=devices)

def _getDeviceList(self, storageType=None, guids=(), checkStatus=True):
-        sdCache.refreshStorage()
-        typeFilter = lambda dev: True
-        if storageType:
-            if sd.storageType(storageType) == sd.type2name(sd.ISCSI_DOMAIN):
-                typeFilter = \
-                    lambda dev: multipath.devIsiSCSI(dev.get("devtype"))
-            elif sd.storageType(storageType) == sd.type2name(sd.FCP_DOMAIN):
-                typeFilter = \
-                    lambda dev: multipath.devIsFCP(dev.get("devtype"))
+        domType = sd.storageType(storageType) if storageType else None
+
+        sdCache.refreshStorage(domType)
+
+        if domType == sd.ISCSI_DOMAIN:
+            typeFilter = lambda dev: multipath.devIsiSCSI(dev.get("devtype"))
+        elif domType == sd.FCP_DOMAIN:
+            typeFilter = lambda dev: multipath.devIsFCP(dev.get("devtype"))
+        else:
+            typeFilter = lambda dev: True

devices = []
         pvs = {}
@@ -2470,7 +2471,7 @@
                 # while the VDSM was not connected, we need to
                 # call refreshStorage.
                 if domType in (sd.FCP_DOMAIN, sd.ISCSI_DOMAIN):
-                    sdCache.refreshStorage()
+                    sdCache.refreshStorage(domType)
                 try:
                     doms = self.__prefetchDomains(domType, conObj)
                 except:
@@ -2864,7 +2865,8 @@
         """
         vars.task.setDefaultException(
             se.StorageDomainActionError("spUUID: %s" % spUUID))
-        sdCache.refreshStorage()
+        domType = sd.storageType(storageType) if storageType else None
+        sdCache.refreshStorage(domType)
         if spUUID and spUUID != volume.BLANK_UUID:
             domList = self.getPool(spUUID).getDomains()
             domains = domList.keys()
@@ -2925,7 +2927,8 @@
         :rtype: dict
         """
         vars.task.setDefaultException(se.VolumeGroupActionError())
-        sdCache.refreshStorage()
+        domType = sd.storageType(storageType) if storageType else None
+        sdCache.refreshStorage(domType)
         # getSharedLock(connectionsResource...)
         vglist = []
         vgs = self.__getVGsInfo()
diff --git a/vdsm/storage/multipath.py b/vdsm/storage/multipath.py
index ad81d2d..32deb98 100644
--- a/vdsm/storage/multipath.py
+++ b/vdsm/storage/multipath.py
@@ -39,6 +39,7 @@
 import misc
 import iscsi
 import devicemapper
+import sd

DEV_ISCSI = "iSCSI"
 DEV_FCP = "FCP"
@@ -61,7 +62,7 @@
     """ multipath operation failed """

-def rescan():
+def rescan(domType=None):
     """
     Forces multipath daemon to rescan the list of available devices and
     refresh the mapping table. New devices can be found under /dev/mapper
@@ -70,8 +71,12 @@
     """

# First rescan iSCSI and FCP connections
-    iscsi.rescan()
-    hba.rescan()
+
+    if domType in (None, sd.ISCSI_DOMAIN):
+        iscsi.rescan()
+
+    if domType in (None, sd.FCP_DOMAIN):
+        hba.rescan()

# Now let multipath daemon pick up new devices
     misc.execCmd([constants.EXT_MULTIPATH], sudo=True)
diff --git a/vdsm/storage/sdc.py b/vdsm/storage/sdc.py
index ecb9708..273c5c0 100644
--- a/vdsm/storage/sdc.py
+++ b/vdsm/storage/sdc.py
@@ -77,10 +77,10 @@
             self.__staleStatus = self.STORAGE_STALE

@misc.samplingmethod
-    def refreshStorage(self):
+    def refreshStorage(self, domType=None):
         self.__staleStatus = self.STORAGE_REFRESHING

-        multipath.rescan()
+        multipath.rescan(domType)
         multipath.resize_devices()
         lvm.invalidateCache()

[e-mail] has posted comments on this change.

Change subject: scsi: Scan only the required domain type
......................................................................

Patch Set 1:

* Update tracker::IGNORE, no Bug-Url found
* Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url.
* Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3'])

Freddy Rolland has posted comments on this change.

Change subject: scsi: Scan only the required domain type
......................................................................

Patch Set 1: Code-Review+1

Adam Litke has posted comments on this change.

Change subject: scsi: Scan only the required domain type
......................................................................

Patch Set 1:

This one worries me.  The sdCache has a state machine that assumes that we are not doing partial refreshes.  I'm concerned about the potential for race conditions where one thread refreshes some storage while another thread is blocked on refresh of the other type of storage.  Although I cannot see anything clearly wrong in this patch, I'm still not convinced it's correct.

[Nir Soffer]

Nir Soffer has posted comments on this change.

Change subject: scsi: Scan only the required domain type
......................................................................

Patch Set 1: Code-Review-1

Need to check if partial refresh works with current logic when refreshing domain during realProduce.

[Jenkins CI RO]

Jenkins CI RO has abandoned this change.

Change subject: scsi: Scan only the required domain type
......................................................................

Abandoned

Abandoned due to no activity - please restore if still relevant

gerrit-hooks has posted comments on this change.

Change subject: scsi: Scan only the required domain type
......................................................................

Patch Set 1:

* Update tracker: IGNORE, no Bug-Url found

[Nir Soffer]

Nir Soffer has restored this change.

Change subject: scsi: Scan only the required domain type
......................................................................

Restored