openstack
/
nova
Code Issues Proposed changes

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
This commit is contained in:
Michael Still 2012-12-10 15:17:12 +11:00
parent 235239c97b
commit ab3c602ac5
3 changed files with 52 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
etc/nova/nova.conf.sample
View File

@ -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 ########

62
nova/tests/virt/disk/test_nbd.py
View File

@ -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):

26
nova/virt/disk/mount/nbd.py
View File

@ -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)