From ab3c602ac5456a509057f915a741970afeab0d4b Mon Sep 17 00:00:00 2001 From: Michael Still Date: Mon, 10 Dec 2012 15:17:12 +1100 Subject: Autodetect nbd devices. This means we don't need to the max_nbd_devices flag any more at all. This patch also moves the internal representation of the list of available nbd devices to not including the leading /dev/, but that is not exposed as a change in the interface to the class. Resolves bug 861504 and its duplicate 1088339. DocImpact. Change-Id: I5c6218122ab09781d8a2ecd6d8b76a74be4a6e91 --- etc/nova/nova.conf.sample | 3 -- nova/tests/virt/disk/test_nbd.py | 62 ++++++++++++++++++++++++---------------- nova/virt/disk/mount/nbd.py | 26 +++++++++-------- 3 files changed, 52 insertions(+), 39 deletions(-) diff --git a/etc/nova/nova.conf.sample b/etc/nova/nova.conf.sample index 5cd8f6bc1..2fdd612b4 100644 --- a/etc/nova/nova.conf.sample +++ b/etc/nova/nova.conf.sample @@ -1339,9 +1339,6 @@ # timeout_nbd=10 #### (IntOpt) time to wait for a NBD device coming up -# max_nbd_devices=16 -#### (IntOpt) maximum number of possible nbd devices - ######## defined in nova.virt.firewall ######## diff --git a/nova/tests/virt/disk/test_nbd.py b/nova/tests/virt/disk/test_nbd.py index 59f1c29d8..41abace8e 100644 --- a/nova/tests/virt/disk/test_nbd.py +++ b/nova/tests/virt/disk/test_nbd.py @@ -23,12 +23,25 @@ from testtools.matchers import MatchesListwise from nova import test -from nova.openstack.common import cfg from nova import utils from nova.virt.disk.mount import nbd -CONF = cfg.CONF -CONF.import_opt('max_nbd_devices', 'nova.virt.disk.mount.nbd') +ORIG_EXISTS = os.path.exists +ORIG_LISTDIR = os.listdir + + +def _fake_exists_no_users(path): + if path.startswith('/sys/block/nbd'): + if path.endswith('pid'): + return False + return True + return ORIG_EXISTS(path) + + +def _fake_listdir_nbd_devices(path): + if path.startswith('/sys/block'): + return ['nbd0', 'nbd1'] + return ORIG_LISTDIR(path) ORIG_EXISTS = os.path.exists @@ -44,17 +57,18 @@ def _fake_exists_no_users(path): class NbdTestCase(test.TestCase): def setUp(self): super(NbdTestCase, self).setUp() - self.flags(max_nbd_devices=2) nbd.NbdMount._DEVICES_INITIALIZED = False nbd.NbdMount._DEVICES = [] + self.useFixture(fixtures.MonkeyPatch('os.listdir', + _fake_listdir_nbd_devices)) def test_nbd_initialize(self): tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(None, tempdir) self.assertTrue(n._DEVICES_INITIALIZED) self.assertThat(nbd.NbdMount._DEVICES, - MatchesListwise([Equals('/dev/nbd0'), - Equals('/dev/nbd1')])) + MatchesListwise([Equals('nbd0'), + Equals('nbd1')])) def test_nbd_no_free_devices(self): tempdir = self.useFixture(fixtures.TempDir()).path @@ -81,8 +95,8 @@ class NbdTestCase(test.TestCase): # And no device should be consumed self.assertThat(nbd.NbdMount._DEVICES, - MatchesListwise([Equals('/dev/nbd0'), - Equals('/dev/nbd1')])) + MatchesListwise([Equals('nbd0'), + Equals('nbd1')])) def test_nbd_all_allocated(self): tempdir = self.useFixture(fixtures.TempDir()).path @@ -103,7 +117,7 @@ class NbdTestCase(test.TestCase): # Allocate a nbd device self.assertEquals('/dev/nbd1', n._allocate_nbd()) - self.assertEquals(['/dev/nbd0'], nbd.NbdMount._DEVICES) + self.assertEquals(['nbd0'], nbd.NbdMount._DEVICES) # Allocate another self.assertEquals('/dev/nbd0', n._allocate_nbd()) @@ -130,28 +144,28 @@ class NbdTestCase(test.TestCase): # re-added. I will fix this in a later patch. self.assertEquals('/dev/nbd0', n._allocate_nbd()) self.assertEquals([], nbd.NbdMount._DEVICES) - self.assertTrue('/dev/nbd0' not in nbd.NbdMount._DEVICES) + self.assertTrue('nbd0' not in nbd.NbdMount._DEVICES) def test_free(self): tempdir = self.useFixture(fixtures.TempDir()).path n = nbd.NbdMount(None, tempdir) - d = '/dev/nosuch' + d = 'nosuch' self.assertFalse(d in nbd.NbdMount._DEVICES) # Now free it n._free_nbd(d) self.assertThat(nbd.NbdMount._DEVICES, - MatchesListwise([Equals('/dev/nbd0'), - Equals('/dev/nbd1'), - Equals('/dev/nosuch')])) + MatchesListwise([Equals('nbd0'), + Equals('nbd1'), + Equals(d)])) self.assertTrue(d in nbd.NbdMount._DEVICES) # Double free n._free_nbd(d) self.assertThat(nbd.NbdMount._DEVICES, - MatchesListwise([Equals('/dev/nbd0'), - Equals('/dev/nbd1'), - Equals('/dev/nosuch')])) + MatchesListwise([Equals('nbd0'), + Equals('nbd1'), + Equals(d)])) self.assertTrue(d in nbd.NbdMount._DEVICES) def test_get_dev_no_devices(self): @@ -175,8 +189,8 @@ class NbdTestCase(test.TestCase): self.assertFalse(n.get_dev()) self.assertTrue(n.error.startswith('qemu-nbd error')) self.assertThat(nbd.NbdMount._DEVICES, - MatchesListwise([Equals('/dev/nbd0'), - Equals('/dev/nbd1')])) + MatchesListwise([Equals('nbd0'), + Equals('nbd1')])) def test_get_dev_qemu_timeout(self): tempdir = self.useFixture(fixtures.TempDir()).path @@ -198,8 +212,8 @@ class NbdTestCase(test.TestCase): self.assertFalse(n.get_dev()) self.assertTrue(n.error.endswith('did not show up')) self.assertThat(nbd.NbdMount._DEVICES, - MatchesListwise([Equals('/dev/nbd0'), - Equals('/dev/nbd1')])) + MatchesListwise([Equals('nbd0'), + Equals('nbd1')])) def test_get_dev_works(self): tempdir = self.useFixture(fixtures.TempDir()).path @@ -242,7 +256,7 @@ class NbdTestCase(test.TestCase): self.assertTrue(n.get_dev()) self.assertTrue(n.linked) self.assertEquals('', n.error) - self.assertEquals(['/dev/nbd0'], nbd.NbdMount._DEVICES) + self.assertEquals(['nbd0'], nbd.NbdMount._DEVICES) self.assertEquals('/dev/nbd1', n.device) # Free @@ -250,8 +264,8 @@ class NbdTestCase(test.TestCase): self.assertFalse(n.linked) self.assertEquals('', n.error) self.assertThat(nbd.NbdMount._DEVICES, - MatchesListwise([Equals('/dev/nbd0'), - Equals('/dev/nbd1')])) + MatchesListwise([Equals('nbd0'), + Equals('nbd1')])) self.assertEquals(None, n.device) def test_unget_dev_simple(self): diff --git a/nova/virt/disk/mount/nbd.py b/nova/virt/disk/mount/nbd.py index baaf9f897..5298a3a8e 100644 --- a/nova/virt/disk/mount/nbd.py +++ b/nova/virt/disk/mount/nbd.py @@ -16,6 +16,7 @@ """Support for mounting images with qemu-nbd""" import os +import re import time from nova.openstack.common import cfg @@ -29,23 +30,21 @@ nbd_opts = [ cfg.IntOpt('timeout_nbd', default=10, help='time to wait for a NBD device coming up'), - cfg.IntOpt('max_nbd_devices', - default=16, - help='maximum number of possible nbd devices'), ] CONF = cfg.CONF CONF.register_opts(nbd_opts) +NBD_DEVICE_RE = re.compile('nbd[0-9]+') + class NbdMount(api.Mount): """qemu-nbd support disk images.""" mode = 'nbd' - # NOTE(padraig): There are three issues with this nbd device handling - # 1. max_nbd_devices should be inferred (#861504) - # 2. We assume nothing else on the system uses nbd devices - # 3. Multiple workers on a system can race against each other + # NOTE(padraig): There are two issues with this nbd device handling + # 1. We assume nothing else on the system uses nbd devices + # 2. Multiple workers on a system can race against each other # A patch has been proposed in Nov 2011, to add add a -f option to # qemu-nbd, akin to losetup -f. One could test for this by running qemu-nbd # with just the -f option, where it will fail if not supported, or if there @@ -64,10 +63,13 @@ class NbdMount(api.Mount): # to have been parsed before we initialize. Note the scoping to ensure # we're updating the class scoped variables. if not self._DEVICES_INITIALIZED: - NbdMount._DEVICES = ['/dev/nbd%s' % i for - i in range(CONF.max_nbd_devices)] + NbdMount._DEVICES = list(self._detect_nbd_devices()) NbdMount._DEVICES_INITIALIZED = True + def _detect_nbd_devices(self): + """Detect nbd device files.""" + return filter(NBD_DEVICE_RE.match, os.listdir('/sys/block/')) + def _allocate_nbd(self): if not os.path.exists("/sys/block/nbd0"): self.error = _('nbd unavailable: module not loaded') @@ -80,15 +82,15 @@ class NbdMount(api.Mount): return None device = self._DEVICES.pop() - if not os.path.exists("/sys/block/%s/pid" % - os.path.basename(device)): + if not os.path.exists('/sys/block/%s/pid' % device): break - return device + return os.path.join('/dev', device) def _free_nbd(self, device): # The device could already be present if unget_dev # is called right after a nova restart # (when destroying an LXC container for example). + device = os.path.basename(device) if not device in self._DEVICES: self._DEVICES.append(device) -- cgit