Skip to content

Commit

Permalink
XSI-915: Improve performance of LVHDoHBA operations by
Browse files Browse the repository at this point in the history 
	1) Reducing expensive calls to _pathfresh
	2) Use multipath with option "-l" and then falling back to using the more expensive "-ll"
	   only if first operation it fails.
	3) Add missing vdi_epoch operations to vdi operation list

Signed-off-by: ben sims <ben.sims@citrix.com>
  • Loading branch information
@BenSimsCitrix
BenSimsCitrix committed Jan 28, 2021
1 parent d1cdd8d commit bbca7a1
Show file tree
Hide file tree
Showing 9 changed files with 29 additions and 33 deletions.
1 change: 0 additions & 1 deletion drivers/LVHDoFCoESR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ def load(self, sr_uuid):
raise xs_errors.XenError('ConfigSCSIid')

self.SCSIid = self.dconf['SCSIid']
self._pathrefresh(LVHDoFCoESR, load=False)
LVHDSR.LVHDSR.load(self, sr_uuid)

def vdi(self, uuid):
Expand Down
13 changes: 6 additions & 7 deletions drivers/LVHDoHBASR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,13 @@ def load(self, sr_uuid):
raise xs_errors.XenError('ConfigSCSIid')

self.SCSIid = self.dconf['SCSIid']
self._pathrefresh(LVHDoHBASR, load=False)
LVHDSR.LVHDSR.load(self, sr_uuid)

def create(self, sr_uuid, size):
self.hbasr.attach(sr_uuid)
if self.mpath == "true":
self.mpathmodule.refresh(self.SCSIid, 0)
self._pathrefresh(LVHDoHBASR)
self.mpathmodule.refresh(self.SCSIid,0)
self._pathrefresh()
try:
LVHDSR.LVHDSR.create(self, sr_uuid, size)
finally:
Expand All @@ -132,7 +131,7 @@ def attach(self, sr_uuid):
for file in os.listdir(path):
self.block_setscheduler('%s/%s' % (path, file))

self._pathrefresh(LVHDoHBASR)
self._pathrefresh()
if not os.path.exists(self.dconf['device']):
# Force a rescan on the bus
self.hbasr._init_hbadict()
Expand All @@ -149,8 +148,8 @@ def scan(self, sr_uuid):
if self.mpath == "true":
if not os.path.exists(self.dconf['device']):
util.SMlog("@@@@@ path does not exists")
self.mpathmodule.refresh(self.SCSIid, 0)
self._pathrefresh(LVHDoHBASR)
self.mpathmodule.refresh(self.SCSIid,0)
self._pathrefresh()
self._setMultipathableFlag(SCSIid=self.SCSIid)
LVHDSR.LVHDSR.scan(self, sr_uuid)

Expand All @@ -171,7 +170,7 @@ def probe(self):
self.mpathmodule.refresh(self.SCSIid, 0)

try:
self._pathrefresh(LVHDoHBASR)
self._pathrefresh()
result = LVHDSR.LVHDSR.probe(self)
if self.mpath == "true":
self.mpathmodule.reset(self.SCSIid, explicit_unmap=True)
Expand Down
7 changes: 3 additions & 4 deletions drivers/LVHDoISCSISR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ def load(self, sr_uuid):
except:
raise xs_errors.XenError('ISCSILogout')

self._pathrefresh(LVHDoISCSISR, load=False)

LVHDSR.LVHDSR.load(self, sr_uuid)

Expand Down Expand Up @@ -478,7 +477,7 @@ def create(self, sr_uuid, size):
continue
if not upgraded:
raise xs_errors.XenError('InvalidDev')
self._pathrefresh(LVHDoISCSISR)
self._pathrefresh()
LVHDSR.LVHDSR.create(self, sr_uuid, size)
except Exception as inst:
self.iscsi.detach(sr_uuid)
Expand Down Expand Up @@ -519,7 +518,7 @@ def attach(self, sr_uuid):
for a in self.iscsi.adapter:
scsiutil.rescan([self.iscsi.adapter[a]])

self._pathrefresh(LVHDoISCSISR)
self._pathrefresh()
LVHDSR.LVHDSR.attach(self, sr_uuid)
except Exception as inst:
for i in self.iscsiSRs:
Expand Down Expand Up @@ -561,7 +560,7 @@ def probe(self):
if not self.iscsi._attach_LUN_bySCSIid(self.SCSIid):
util.SMlog("Unable to detect LUN")
raise xs_errors.XenError('InvalidDev')
self._pathrefresh(LVHDoISCSISR)
self._pathrefresh()
out = LVHDSR.LVHDSR.probe(self)
self.iscsi.detach(self.uuid)
return out
Expand Down
9 changes: 4 additions & 5 deletions drivers/OCFSoHBASR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,14 +77,13 @@ def load(self, sr_uuid):
raise xs_errors.XenError('ConfigSCSIid')

self.SCSIid = self.dconf['SCSIid']
self._pathrefresh(OCFSoHBASR, load=False)
super(OCFSoHBASR, self).load(sr_uuid)

def create(self, sr_uuid, size):
self.hbasr.attach(sr_uuid)
if self.mpath == "true":
self.mpathmodule.refresh(self.SCSIid, 0)
self._pathrefresh(OCFSoHBASR)
self._pathrefresh()
try:
super(OCFSoHBASR, self).create(sr_uuid, size)
finally:
Expand All @@ -102,7 +101,7 @@ def attach(self, sr_uuid):
for file in os.listdir(path):
self.block_setscheduler('%s/%s' % (path, file))

self._pathrefresh(OCFSoHBASR)
self._pathrefresh()
if not os.path.exists(self.dconf['device']):
# Force a rescan on the bus
self.hbasr._init_hbadict()
Expand All @@ -121,7 +120,7 @@ def scan(self, sr_uuid):
if not os.path.exists(self.dconf['device']):
util.SMlog("%s path does not exists" % self.dconf['device'])
self.mpathmodule.refresh(self.SCSIid, 0)
self._pathrefresh(OCFSoHBASR)
self._pathrefresh()
self._setMultipathableFlag(SCSIid=self.SCSIid)
super(OCFSoHBASR, self).scan(sr_uuid)

Expand All @@ -141,7 +140,7 @@ def probe(self):
raise xs_errors.XenError('SRInUse')
self.mpathmodule.refresh(self.SCSIid, 0)
try:
self._pathrefresh(OCFSoHBASR)
self._pathrefresh()
result = super(OCFSoHBASR, self).probe()
if self.mpath == "true":
self.mpathmodule.reset(self.SCSIid, True)
Expand Down
7 changes: 3 additions & 4 deletions drivers/OCFSoISCSISR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,6 @@ def load(self, sr_uuid):
except:
raise xs_errors.XenError('ISCSILogout')

self._pathrefresh(OCFSoISCSISR, load=False)
OCFSSR.OCFSSR.load(self, sr_uuid)

def print_LUNs_XML(self):
Expand Down Expand Up @@ -461,7 +460,7 @@ def create(self, sr_uuid, size):
continue
if not upgraded:
raise xs_errors.XenError('InvalidDev')
self._pathrefresh(OCFSoISCSISR)
self._pathrefresh()
OCFSSR.OCFSSR.create(self, sr_uuid, size)
except Exception as inst:
self.iscsi.detach(sr_uuid)
Expand Down Expand Up @@ -499,7 +498,7 @@ def attach(self, sr_uuid):
# Force a manual bus refresh
for a in self.iscsi.adapter:
scsiutil.rescan([self.iscsi.adapter[a]])
self._pathrefresh(OCFSoISCSISR)
self._pathrefresh()
OCFSSR.OCFSSR.attach(self, sr_uuid)
except Exception as inst:
for i in self.iscsiSRs:
Expand Down Expand Up @@ -533,7 +532,7 @@ def probe(self):
if not self.iscsi._attach_LUN_bySCSIid(self.SCSIid):
util.SMlog("Unable to detect LUN")
raise xs_errors.XenError('InvalidDev')
self._pathrefresh(OCFSoISCSISR)
self._pathrefresh()
out = OCFSSR.OCFSSR.probe(self)
self.iscsi.detach(self.uuid)
return out
Expand Down
4 changes: 1 addition & 3 deletions drivers/SR.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -489,11 +489,9 @@ def _mpathHandle(self):
else:
self.mpathmodule.deactivate()

def _pathrefresh(self, obj, load=True):
def _pathrefresh(self):
SCSIid = getattr(self, 'SCSIid')
self.dconf['device'] = self.mpathmodule.path(SCSIid)
if load:
super(obj, self).load(self.uuid)

def _setMultipathableFlag(self, SCSIid=''):
try:
Expand Down
4 changes: 3 additions & 1 deletion drivers/mpath_dmp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,9 @@ def refresh(sid, npaths):
def _is_valid_multipath_device(sid):

# Check if device is already multipathed
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-ll', sid])
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-l', sid])
if not stdout + stderr:
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-ll', sid])
if not stdout + stderr:
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-a', sid])
if ret < 0:
Expand Down
3 changes: 2 additions & 1 deletion drivers/util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1196,7 +1196,8 @@ def _containsVDIinuse(srobj):

def isVDICommand(cmd):
if cmd == None or cmd in ["vdi_attach", "vdi_detach",
"vdi_activate", "vdi_deactivate"]:
"vdi_activate", "vdi_deactivate",
"vdi_epoch_begin", "vdi_epoch_end"]:
return True
else:
return False
Expand Down
14 changes: 7 additions & 7 deletions tests/test_mpath_dmp.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,42 +51,42 @@ def test_is_valid_multipath_device(self, context, mock_os, util_mod):
self.assertTrue(mpath_dmp._is_valid_multipath_device("fake_dev"))

# Test 'multipath -a' failure
util_mod.doexec.side_effect = [(0, "", ""), (1, "out", "err")]
util_mod.doexec.side_effect = [(0, "", ""), (0, "", ""), (1, "out", "err")]
self.assertFalse(mpath_dmp._is_valid_multipath_device("fake_dev"))

# Test failure when device is not available
mock_os.path.exists.return_value = False
util_mod.doexec.side_effect = [(0, "", ""), (0, "out", "err")]
util_mod.doexec.side_effect = [(0, "", ""), (0, "", ""), (0, "out", "err")]
self.assertFalse(mpath_dmp._is_valid_multipath_device("fake_dev"))

# Test 'multipath -c' with error and empty output
mock_os.path.exists.return_value = True
mock_os.listdir.side_effect = [['sdc']]
util_mod.doexec.side_effect = [(0, "", ""), (0, "out", "err"),
util_mod.doexec.side_effect = [(0, "", ""), (0, "", ""), (0, "out", "err"),
(1, "", ""), OSError()]
with self.assertRaises(SR.SROSError) as exc:
mpath_dmp._is_valid_multipath_device("xx")
self.assertEqual(exc.exception.errno, 431)

# Test 'multipath -c' with error but some output
mock_os.listdir.side_effect = [['sdc']]
util_mod.doexec.side_effect = [(0, "", ""), (0, "out", "err"),
util_mod.doexec.side_effect = [(0, "", ""), (0, "", ""), (0, "out", "err"),
(1, "xx", "")]
self.assertFalse(mpath_dmp._is_valid_multipath_device("fake_dev"))

mock_os.listdir.side_effect = [['sdc']]
util_mod.doexec.side_effect = [(0, "", ""), (0, "out", "err"),
util_mod.doexec.side_effect = [(0, "", ""), (0, "", ""), (0, "out", "err"),
(1, "xx", "yy")]
self.assertFalse(mpath_dmp._is_valid_multipath_device("fake_dev"))

mock_os.listdir.side_effect = [['sdc']]
util_mod.doexec.side_effect = [(0, "", ""), (0, "out", "err"),
util_mod.doexec.side_effect = [(0, "", ""), (0, "", ""), (0, "out", "err"),
(1, "", "yy")]
self.assertFalse(mpath_dmp._is_valid_multipath_device("fake_dev"))

# Test when everything is fine
mock_os.listdir.side_effect = [['sdc']]
util_mod.doexec.side_effect = [(0, "", ""), (0, "out", "err"),
util_mod.doexec.side_effect = [(0, "", ""), (0, "", ""), (0, "out", "err"),
(0, "out", "err")]
self.assertTrue(mpath_dmp._is_valid_multipath_device("fake_dev"))

Expand Down

0 comments on commit bbca7a1

Please sign in to comment.