Skip to content

Commit

Permalink
Merge 1561a43 into 78946da
Browse files Browse the repository at this point in the history
  • Loading branch information
@BenSimsCitrix
BenSimsCitrix committed Mar 4, 2020
2 parents 78946da + 1561a43 commit 5aae9a1
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 7 deletions.
59 changes: 52 additions & 7 deletions drivers/blktap2.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,17 @@ def mkdirs(path, mode=0777):
if e.errno != errno.EEXIST:
raise

def rmdirs(path):
# lexist returns true even if link is broken
if os.path.lexists(path):
parent, subdir = os.path.split(path)
assert parent != path
if subdir:
os.unlink(path)
if parent and len(os.listdir(parent)) == 0:
os.rmdir(parent)


class KObject(object):

SYSFS_CLASSTYPE = None
Expand Down Expand Up @@ -1009,6 +1020,7 @@ def __init__(self, uuid, target, driver_info):
self.__o_direct = None
self.__o_direct_reason = None
self.lock = Lock("vdi", uuid)
self.tap = None

def get_o_direct_capability(self, options):
"""Returns True/False based on licensing and caching_params"""
Expand Down Expand Up @@ -1208,6 +1220,9 @@ def mklink(self, target):
if e.errno != errno.EEXIST: raise
assert self._equals(target), "'%s' not equal to '%s'" % (path, target)

def rmdirs(self):
rmdirs(self.path())

def unlink(self):
try:
os.unlink(self.path())
Expand Down Expand Up @@ -1298,7 +1313,12 @@ def _equals(self, target):
class PhyLink(SymLink): BASEDIR = "/dev/sm/phy"
# NB. Cannot use DeviceNodes, e.g. FileVDIs aren't bdevs.

class NBDLink(SymLink):

BASEDIR = "/run/blktap-control/nbd"

class BackendLink(Hybrid): BASEDIR = "/dev/sm/backend"

# NB. Could be SymLinks as well, but saving major,minor pairs in
# Links enables neat state capturing when managing Tapdisks. Note
# that we essentially have a tap-ctl list replacement here. For
Expand Down Expand Up @@ -1329,7 +1349,7 @@ def _tap_activate(phy_path, vdi_type, sr_uuid, options, pool_size = None):
else:
util.SMlog("tap.activate: Found %s" % tapdisk)

return tapdisk.get_devpath()
return tapdisk.get_devpath(), tapdisk

@staticmethod
def _tap_deactivate(minor):
Expand Down Expand Up @@ -1518,6 +1538,12 @@ def _get_pool_config(self, pool_name):
session.xenapi.session.logout()
return pool_info

def linkNBD(self, sr_uuid, vdi_uuid):
if self.tap:
nbd_path = '/run/blktap-control/nbd%d.%d' % (int(self.tap.pid),
int(self.tap.minor))
VDI.NBDLink.from_uuid(sr_uuid, vdi_uuid).mklink(nbd_path)

def attach(self, sr_uuid, vdi_uuid, writable, activate = False, caching_params = {}):
"""Return/dev/sm/backend symlink path"""
self.xenstore_data.update(self._get_pool_config(sr_uuid))
Expand All @@ -1527,16 +1553,25 @@ def attach(self, sr_uuid, vdi_uuid, writable, activate = False, caching_params =
dev_path = self._activate(sr_uuid, vdi_uuid,
{"rdonly": not writable})
self.BackendLink.from_uuid(sr_uuid, vdi_uuid).mklink(dev_path)
self.linkNBD(sr_uuid, vdi_uuid)

# Return backend/ link
back_path = self.BackendLink.from_uuid(sr_uuid, vdi_uuid).path()
if self.tap_wanted():
# Only have NBD if we also have a tap
nbd_path =\
"nbd:unix:" + VDI.NBDLink.from_uuid(sr_uuid, vdi_uuid).path()
else:
nbd_path = ""

options = {"rdonly": not writable}
options.update(caching_params)
o_direct, o_direct_reason = self.get_o_direct_capability(options)
struct = { 'params': back_path,
'o_direct': o_direct,
'o_direct_reason': o_direct_reason,
'xenstore_data': self.xenstore_data}
struct = {'params': back_path,
'params_nbd': nbd_path,
'o_direct': o_direct,
'o_direct_reason': o_direct_reason,
'xenstore_data': self.xenstore_data}
util.SMlog('result: %s' % struct)

try:
Expand Down Expand Up @@ -1652,6 +1687,7 @@ def _activate_locked(self, sr_uuid, vdi_uuid, options):

# Link result to backend/
self.BackendLink.from_uuid(sr_uuid, vdi_uuid).mklink(dev_path)
self.linkNBD(sr_uuid, vdi_uuid)
return True

def _activate(self, sr_uuid, vdi_uuid, options):
Expand All @@ -1666,8 +1702,8 @@ def _activate(self, sr_uuid, vdi_uuid, options):
options["o_direct"] = self.get_o_direct_capability(options)[0]
if vdi_options:
options.update(vdi_options)
dev_path = self._tap_activate(phy_path, vdi_type, sr_uuid,
options,
dev_path, self.tap = self._tap_activate(phy_path, vdi_type,
sr_uuid, options,
self._get_pool_config(sr_uuid).get("mem-pool-size"))
else:
dev_path = phy_path # Just reuse phy
Expand Down Expand Up @@ -1726,10 +1762,16 @@ def _deactivate(self, sr_uuid, vdi_uuid, caching_params):

# Shutdown tapdisk
back_link = self.BackendLink.from_uuid(sr_uuid, vdi_uuid)

if not util.pathexists(back_link.path()):
util.SMlog("Backend path %s does not exist" % back_link.path())
return

nbd_link = VDI.NBDLink.from_uuid(sr_uuid, vdi_uuid)
if (not util.pathexists(nbd_link.path())) and self.tap_wanted():
util.SMlog("Backend path %s does not exist" % nbd_link.path())
return

try:
attach_info_path = "%s.attach_info" % (back_link.path())
os.unlink(attach_info_path)
Expand All @@ -1747,6 +1789,8 @@ def _deactivate(self, sr_uuid, vdi_uuid, caching_params):

# Remove the backend link
back_link.unlink()
util.SMlog("UNLINKING NBD")
nbd_link.rmdirs()

# Deactivate & detach the physical node
if self.tap_wanted() and self.target.vdi.session is not None:
Expand Down Expand Up @@ -1960,6 +2004,7 @@ def _setup_cache(self, session, sr_uuid, vdi_uuid, local_sr_uuid,
util.SMlog("Local read cache: %s, local leaf: %s" % \
(read_cache_path, local_leaf_path))

self.tap = leaf_tapdisk
return leaf_tapdisk.get_devpath()

def remove_cache(self, sr_uuid, vdi_uuid, params):
Expand Down
91 changes: 91 additions & 0 deletions tests/test_blktap2.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import blktap2
import mock
import testlib
import os


class TestVDI(unittest.TestCase):
Expand Down Expand Up @@ -29,3 +30,93 @@ def test_get_tap_type_returns_aio_for_udev_device(self):
result = self.vdi.get_tap_type()

self.assertEquals('aio', result)

class NBDLinkForTest(blktap2.VDI.NBDLink):
__name__ = "bob"

@mock.patch('blktap2.VDI.NBDLink', autospec=NBDLinkForTest)
@mock.patch('blktap2.VDI.NBDLink', autospec=NBDLinkForTest)
def test_linknbd_not_called_for_no_tap(self, nbd_link2, nbd_link):
self.vdi.linkNBD("blahblah", "yadayada")
self.assertEquals(nbd_link.from_uuid.call_count, 0)

@mock.patch('blktap2.VDI.NBDLink', autospec=NBDLinkForTest)
@mock.patch('blktap2.VDI.NBDLink', autospec=NBDLinkForTest)
def test_linknbd(self, nbd_link2, nbd_link):
self.vdi.tap = blktap2.Tapdisk(123, 456, "blah", "blah", "blah")
nbd_link.from_uuid.return_value = nbd_link2
self.vdi.linkNBD("blahblah", "yadayada")
expected_path = '/run/blktap-control/nbd%d.%d' % (123, 456)
nbd_link.from_uuid.assert_called_with("blahblah", "yadayada")
nbd_link2.mklink.assert_called_with(expected_path)

def reset_removedirmocks(self, mock_rmdir, mock_unlink, mock_listdir,
mock_split, mock_lexists):
mock_rmdir.reset_mock()
mock_unlink.reset_mock()
mock_listdir.reset_mock()
mock_split.reset_mock()
mock_lexists.reset_mock()

@mock.patch("blktap2.os.path.lexists", autospec=True)
@mock.patch("blktap2.os.path.split", autospec=True)
@mock.patch("blktap2.os.listdir", autospec=True)
@mock.patch("blktap2.os.unlink", autospec=True)
@mock.patch("blktap2.os.rmdir", autospec=True)
def test_removedirs(self, mock_rmdir, mock_unlink, mock_listdir,
mock_split, mock_lexists):
# Path does not exist
mock_lexists.return_value = False
blktap2.rmdirs("blah/blah1/blah2.txt")
mock_lexists.assert_called_with("blah/blah1/blah2.txt")
self.assertEquals(mock_split.call_count, 0)

self.reset_removedirmocks(mock_rmdir, mock_unlink,
mock_listdir, mock_split, mock_lexists)
# Split returns nothing.
mock_lexists.return_value = True
mock_split.return_value = [None, None]
blktap2.rmdirs("blah/blah1/blah2.txt")
mock_lexists.assert_called_with("blah/blah1/blah2.txt")
mock_split.assert_called_with("blah/blah1/blah2.txt")
self.assertEquals(mock_rmdir.call_count, 0)
self.assertEquals(mock_unlink.call_count, 0)
self.assertEquals(mock_listdir.call_count, 0)

self.reset_removedirmocks(mock_rmdir, mock_unlink,
mock_listdir, mock_split, mock_lexists)
# Split returns single file.
mock_lexists.return_value = True
mock_split.return_value = [None, "blah.txt"]
blktap2.rmdirs("blah/blah1/blah2.txt")
mock_lexists.assert_called_with("blah/blah1/blah2.txt")
mock_split.assert_called_with("blah/blah1/blah2.txt")
self.assertEquals(mock_rmdir.call_count, 0)
self.assertEquals(mock_listdir.call_count, 0)
mock_unlink.assert_called_with("blah/blah1/blah2.txt")

self.reset_removedirmocks(mock_rmdir, mock_unlink,
mock_listdir, mock_split, mock_lexists)
# Split returns valid stuff. but directory is not empty
mock_lexists.return_value = True
mock_split.return_value = ["blah/blah1/", "blah2.txt"]
mock_listdir.return_value = ["stuff.txt", "more_stuff.txt"]
blktap2.rmdirs("blah/blah1/blah2.txt")
mock_lexists.assert_called_with("blah/blah1/blah2.txt")
mock_split.assert_called_with("blah/blah1/blah2.txt")
self.assertEquals(mock_rmdir.call_count, 0)
mock_listdir.assert_called_with("blah/blah1/")
mock_unlink.assert_called_with("blah/blah1/blah2.txt")

self.reset_removedirmocks(mock_rmdir, mock_unlink,
mock_listdir, mock_split, mock_lexists)
# Everything is good to remove
mock_lexists.return_value = True
mock_split.return_value = ["blah/blah1/", "blah2.txt"]
mock_listdir.return_value = []
blktap2.rmdirs("blah/blah1/blah2.txt")
mock_lexists.assert_called_with("blah/blah1/blah2.txt")
mock_split.assert_called_with("blah/blah1/blah2.txt")
mock_rmdir.assert_called_with("blah/blah1/")
mock_listdir.assert_called_with("blah/blah1/")
mock_unlink.assert_called_with("blah/blah1/blah2.txt")

0 comments on commit 5aae9a1

Please sign in to comment.