Merge "Autodetect nbd devices."

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)