Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Feature/nbd #488

Merged
merged 4 commits into from
Mar 6, 2020
Merged

Feature/nbd #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d5dfe94
CP-31856 - Return nbd socket as parameter to XAPI
BenSimsCitrix Jan 10, 2020
1c4985b
CA-335178: If we don't need tap we dont have NBD
MarkSymsCtx Feb 17, 2020
1c1d97c
CA-335178: Fix unitialized variable
BenSimsCitrix Feb 24, 2020
1561a43
CA-335721: Add missing case blktap2 for cached devices.
BenSimsCitrix Mar 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 52 additions & 7 deletions drivers/blktap2.py
View file
Open in desktop
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
Open in desktop
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")