openstack
/
nova
Code Issues Proposed changes

libvirt: do not remove inst_base when volume-backed during resize 

When confirming a resize, the libvirt driver on the source host checks
to see if the instance base directory (which contains the domain xml
files, etc) exists and if the root disk image does not, it removes the
instance base directory.

However, the root image disk won't exist on local storage for a
volume-backed instance and if the instance base directory is on shared
storage, e.g. NFS or Ceph, between the source and destination host, the
instance base directory is incorrectly deleted.

This adds a check to see if the instance is volume-backed when checking
to see if the instance base directory should be removed from the source
host when confirming a resize.

Change-Id: I29fac80d08baf64bf69e54cf673e55123174de2a
Closes-Bug: #1728603
(cherry picked from commit f02afc6569)
This commit is contained in:
Matt Riedemann 2017-10-30 12:49:31 -04:00
parent 60d6e87cac
commit d5d81a2905
3 changed files with 65 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
nova/compute/manager.py
View File

@ -3587,6 +3587,7 @@ class ComputeManager(manager.Manager):
network_info = self.network_api.get_instance_nw_info(context,
instance)
# TODO(mriedem): Get BDMs here and pass them to the driver.
self.driver.confirm_migration(context, migration, instance,
network_info)

66
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -16703,8 +16703,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
ins_ref = self._create_instance()
self.mox.StubOutWithMock(self.drvr, "_cleanup_resize")
self.drvr._cleanup_resize(ins_ref,
_fake_network_info(self, 1))
self.drvr._cleanup_resize(self.context, ins_ref,
_fake_network_info(self, 1))
self.mox.ReplayAll()
self.drvr.confirm_migration(self.context, "migration_ref", ins_ref,
@ -16727,7 +16727,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
mock_exists.return_value = True
mock_get_path.return_value = '/fake/inst'
drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
drvr._cleanup_resize(
self.context, ins_ref, _fake_network_info(self, 1))
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)
@ -16745,6 +16746,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
drvr.image_backend.exists.return_value = False
with test.nested(
mock.patch('nova.compute.utils.is_volume_backed_instance',
return_value=False),
mock.patch.object(os.path, 'exists'),
mock.patch.object(libvirt_utils, 'get_instance_path'),
mock.patch.object(utils, 'execute'),
@ -16752,12 +16755,12 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
mock.patch.object(drvr, '_undefine_domain'),
mock.patch.object(drvr, 'unplug_vifs'),
mock.patch.object(drvr, 'unfilter_instance')
) as (mock_exists, mock_get_path, mock_exec, mock_rmtree,
mock_undef, mock_unplug, mock_unfilter):
) as (mock_volume_backed, mock_exists, mock_get_path,
mock_exec, mock_rmtree, mock_undef, mock_unplug, mock_unfilter):
mock_exists.return_value = True
mock_get_path.return_value = '/fake/inst'
drvr._cleanup_resize(ins_ref, fake_net)
drvr._cleanup_resize(self.context, ins_ref, fake_net)
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)
@ -16766,6 +16769,45 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
mock_unplug.assert_called_once_with(ins_ref, fake_net)
mock_unfilter.assert_called_once_with(ins_ref, fake_net)
def test_cleanup_resize_not_same_host_volume_backed(self):
"""Tests cleaning up after a resize is confirmed with a volume-backed
instance. The key point is that the instance base directory should not
be removed for volume-backed instances.
"""
CONF.set_override('policy_dirs', [], group='oslo_policy')
host = 'not' + CONF.host
ins_ref = self._create_instance({'host': host})
fake_net = _fake_network_info(self, 1)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
drvr.image_backend = mock.Mock()
drvr.image_backend.by_name.return_value = drvr.image_backend
drvr.image_backend.exists.return_value = False
with test.nested(
mock.patch('nova.compute.utils.is_volume_backed_instance',
return_value=True),
mock.patch.object(os.path, 'exists'),
mock.patch.object(libvirt_utils, 'get_instance_path'),
mock.patch.object(utils, 'execute'),
mock.patch.object(shutil, 'rmtree'),
mock.patch.object(drvr, '_undefine_domain'),
mock.patch.object(drvr, 'unplug_vifs'),
mock.patch.object(drvr, 'unfilter_instance')
) as (mock_volume_backed, mock_exists, mock_get_path,
mock_exec, mock_rmtree, mock_undef, mock_unplug, mock_unfilter):
mock_exists.return_value = True
mock_get_path.return_value = '/fake/inst'
drvr._cleanup_resize(self.context, ins_ref, fake_net)
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)
mock_rmtree.assert_not_called()
mock_undef.assert_called_once_with(ins_ref)
mock_unplug.assert_called_once_with(ins_ref, fake_net)
mock_unfilter.assert_called_once_with(ins_ref, fake_net)
def test_cleanup_resize_snap_backend(self):
CONF.set_override('policy_dirs', [], group='oslo_policy')
ins_ref = self._create_instance({'host': CONF.host})
@ -16784,7 +16826,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
mock_exists.return_value = True
mock_get_path.return_value = '/fake/inst'
drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
drvr._cleanup_resize(
self.context, ins_ref, _fake_network_info(self, 1))
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)
@ -16801,17 +16844,20 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
drvr.image_backend.exists.return_value = False
with test.nested(
mock.patch('nova.compute.utils.is_volume_backed_instance',
return_value=False),
mock.patch.object(os.path, 'exists'),
mock.patch.object(libvirt_utils, 'get_instance_path'),
mock.patch.object(utils, 'execute'),
mock.patch.object(shutil, 'rmtree'),
mock.patch.object(drvr.image_backend, 'remove_snap')) as (
mock_exists, mock_get_path, mock_exec, mock_rmtree,
mock_remove):
mock_volume_backed, mock_exists, mock_get_path,
mock_exec, mock_rmtree, mock_remove):
mock_exists.return_value = True
mock_get_path.return_value = '/fake/inst'
drvr._cleanup_resize(ins_ref, _fake_network_info(self, 1))
drvr._cleanup_resize(
self.context, ins_ref, _fake_network_info(self, 1))
mock_get_path.assert_called_once_with(ins_ref)
mock_exec.assert_called_once_with('rm', '-rf', '/fake/inst_resize',
delay_on_retry=True, attempts=5)

11
nova/virt/libvirt/driver.py
View File

@ -1120,7 +1120,7 @@ class LibvirtDriver(driver.ComputeDriver):
enforce_multipath=True,
host=CONF.host)
def _cleanup_resize(self, instance, network_info):
def _cleanup_resize(self, context, instance, network_info):
inst_base = libvirt_utils.get_instance_path(instance)
target = inst_base + '_resize'
@ -1146,7 +1146,12 @@ class LibvirtDriver(driver.ComputeDriver):
# NOTE(mjozefcz):
# self.image_backend.image for some backends recreates instance
# directory and image disk.info - remove it here if exists
if os.path.exists(inst_base) and not root_disk.exists():
# Do not remove inst_base for volume-backed instances since that
# could potentially remove the files on the destination host
# if using shared storage.
if (os.path.exists(inst_base) and not root_disk.exists() and
not compute_utils.is_volume_backed_instance(
context, instance)):
try:
shutil.rmtree(inst_base)
except OSError as e:
@ -7711,7 +7716,7 @@ class LibvirtDriver(driver.ComputeDriver):
def confirm_migration(self, context, migration, instance, network_info):
"""Confirms a resize, destroying the source VM."""
self._cleanup_resize(instance, network_info)
self._cleanup_resize(context, instance, network_info)
@staticmethod
def _get_io_devices(xml_doc):