Skip to content

Commit

Permalink
CA-387861 Wrap dmsetup and multipath with 'multipath' exclusive context
Browse files Browse the repository at this point in the history
Signed-off-by: Tim Smith <[email protected]>
  • Loading branch information
Tim Smith authored and TimSmithCtx committed May 7, 2024
1 parent 5e182cc commit 2eef1f6
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 30 deletions.
18 changes: 12 additions & 6 deletions drivers/lvutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -653,7 +653,8 @@ def activateNoRefcount(path, refresh):
text = cmd_lvm([CMD_LVCHANGE, "--refresh", path])
mapperDevice = path[5:].replace("-", "--").replace("/", "-")
cmd = [CMD_DMSETUP, "table", mapperDevice]
ret = util.pread(cmd)
with Fairlock("multipath"):
ret = util.pread(cmd)
util.SMlog("DM table for %s: %s" % (path, ret.strip()))
# Restore slave mode lvm.conf
os.environ['LVM_SYSTEM_DIR'] = DEF_LVM_CONF
Expand Down Expand Up @@ -695,7 +696,8 @@ def _checkActive(path):
mapperDevice = path[5:].replace("-", "--").replace("/", "-")
cmd = [CMD_DMSETUP, "status", mapperDevice]
try:
ret = util.pread2(cmd)
with Fairlock("multipath"):
ret = util.pread2(cmd)
mapperDeviceExists = True
util.SMlog("_checkActive: %s: %s" % (mapperDevice, ret))
except util.CommandException:
Expand Down Expand Up @@ -734,7 +736,8 @@ def _lvmBugCleanup(path):
cmd_rf = [CMD_DMSETUP, "remove", mapperDevice, "--force"]

try:
util.pread(cmd_st, expect_rc=1)
with Fairlock("multipath"):
util.pread(cmd_st, expect_rc=1)
except util.CommandException as e:
if e.code == 0:
nodeExists = True
Expand All @@ -749,13 +752,15 @@ def _lvmBugCleanup(path):
util.SMlog("_lvmBugCleanup: removing dm device %s" % mapperDevice)
for i in range(LVM_FAIL_RETRIES):
try:
util.pread2(cmd_rm)
with Fairlock("multipath"):
util.pread2(cmd_rm)
break
except util.CommandException as e:
if i < LVM_FAIL_RETRIES - 1:
util.SMlog("Failed on try %d, retrying" % i)
try:
util.pread(cmd_st, expect_rc=1)
with Fairlock("multipath"):
util.pread(cmd_st, expect_rc=1)
util.SMlog("_lvmBugCleanup: dm device {}"
" removed".format(mapperDevice)
)
Expand Down Expand Up @@ -799,7 +804,8 @@ def removeDevMapperEntry(path, strict=True):
if not strict:
cmd = [CMD_DMSETUP, "status", path]
try:
util.pread(cmd, expect_rc=1)
with Fairlock("multipath"):
util.pread(cmd, expect_rc=1)
return True
except:
pass # Continuining will fail and log the right way
Expand Down
10 changes: 7 additions & 3 deletions drivers/mpath_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import util
import re
import time
from fairlock import Fairlock


class MPathCLIFail(Exception):
Expand All @@ -32,7 +33,8 @@ def __str__(self):

def mpexec(cmd):
util.SMlog("mpath cmd: %s" % cmd)
(rc, stdout, stderr) = util.doexec(mpathcmd, cmd)
with Fairlock("multipath"):
(rc, stdout, stderr) = util.doexec(mpathcmd, cmd)
if stdout != "multipathd> ok\nmultipathd> " \
and stdout != "multipathd> " + cmd + "\nok\nmultipathd> ":
raise MPathCLIFail
Expand Down Expand Up @@ -77,7 +79,8 @@ def is_working():

def do_get_topology(cmd):
util.SMlog("mpath cmd: %s" % cmd)
(rc, stdout, stderr) = util.doexec(mpathcmd, cmd)
with Fairlock("multipath"):
(rc, stdout, stderr) = util.doexec(mpathcmd, cmd)
util.SMlog("mpath output: %s" % stdout)
lines = stdout.split('\n')[:-1]
if len(lines):
Expand Down Expand Up @@ -109,7 +112,8 @@ def list_paths(scsi_id):
def list_maps():
cmd = "list maps"
util.SMlog("mpath cmd: %s" % cmd)
(rc, stdout, stderr) = util.doexec(mpathcmd, cmd)
with Fairlock("multipath"):
(rc, stdout, stderr) = util.doexec(mpathcmd, cmd)
util.SMlog("mpath output: %s" % stdout)
return [x.split(' ')[0] for x in stdout.split('\n')[2:-1]]

Expand Down
44 changes: 23 additions & 21 deletions drivers/mpath_dmp.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import scsiutil
import wwid_conf
import errno
from fairlock import Fairlock

DMPBIN = "/sbin/multipath"
DEVMAPPERPATH = "/dev/mapper"
Expand Down Expand Up @@ -77,10 +78,11 @@ def _resetDMP(sid, explicit_unmap=False):
# tables. In that case, list_paths will return [], but remove_map might
# throw an exception. Catch it and ignore it.
if explicit_unmap:
util.retry(lambda: util.pread2(['/usr/sbin/multipath', '-f', sid]),
maxretry=3, period=4)
util.retry(lambda: util.pread2(['/usr/sbin/multipath', '-W']), maxretry=3,
period=4)
with Fairlock("multipath"):
util.retry(lambda: util.pread2(['/usr/sbin/multipath', '-f', sid]),
maxretry=3, period=4)
util.retry(lambda: util.pread2(['/usr/sbin/multipath', '-W']), maxretry=3,
period=4)
else:
mpath_cli.ensure_map_gone(sid)

Expand Down Expand Up @@ -109,33 +111,35 @@ def refresh(sid, npaths):
def _is_valid_multipath_device(sid):

# Check if device is already multipathed
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-l', sid])
with Fairlock("multipath"):
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-l', sid])
if not stdout + stderr:
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-ll', sid])
with Fairlock("multipath"):
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-ll', sid])
if not stdout + stderr:
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-a', sid])
with Fairlock("multipath"):
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-a', sid])
if ret < 0:
util.SMlog("Failed to add {}: wwid could be explicitly "
"blacklisted\n Continue with multipath disabled for "
"this SR".format(sid))
"blacklisted\n Continue with multipath disabled for "
"this SR".format(sid))
return False

by_scsid_path = "/dev/disk/by-scsid/" + sid
if os.path.exists(by_scsid_path):
devs = os.listdir(by_scsid_path)
else:
util.SMlog("Device {} is not ready yet, skipping multipath check"
.format(by_scsid_path))
util.SMlog("Device {} is not ready yet, skipping multipath check".format(by_scsid_path))
return False
ret = 1
# Some paths might be down, check all associated devices
for dev in devs:
devpath = os.path.join(by_scsid_path, dev)
real_path = util.get_real_path(devpath)
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-c',
real_path])
if ret == 0:
break
with Fairlock("multipath"):
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-c', real_path])
if ret == 0:
break

if ret == 1:
# This is very fragile but it is not a good sign to fail without
Expand All @@ -146,8 +150,8 @@ def _is_valid_multipath_device(sid):
if not stdout + stderr:
# Attempt to cleanup wwids file before raising
try:
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath',
'-w', sid])
with Fairlock("multipath"):
(ret, stdout, stderr) = util.doexec(['/usr/sbin/multipath', '-w', sid])
except OSError:
util.SMlog("Error removing {} from wwids file".format(sid))
raise xs_errors.XenError('MultipathGenericFailure',
Expand All @@ -166,10 +170,8 @@ def _refresh_DMP(sid, npaths):
path = os.path.join(DEVMAPPERPATH, sid)
# If the mapper path doesn't exist force a reload in multipath
if not os.path.exists(path):
util.retry(lambda: util.pread2(
['/usr/sbin/multipath', '-r', sid]),
maxretry=3,
period=4)
with Fairlock("multipath"):
util.retry(lambda: util.pread2(['/usr/sbin/multipath', '-r', sid]), maxretry=3, period=4)
util.wait_for_path(path, 30)
if not os.path.exists(path):
raise xs_errors.XenError('MultipathMapperPathMissing',
Expand Down
3 changes: 3 additions & 0 deletions tests/test_mpath_dmp.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ def setUp(self):
mpath_cli_patcher = mock.patch('mpath_dmp.mpath_cli', autospec=True)
self.mock_mpath_cli = mpath_cli_patcher.start()

lock_patcher = mock.patch('mpath_dmp.Fairlock', autospec=True)
self.mock_lock = lock_patcher.start()

self.addCleanup(mock.patch.stopall)

@testlib.with_context
Expand Down

0 comments on commit 2eef1f6

Please sign in to comment.