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 commitf02afc6569
) (cherry picked from commitd5d81a2905
)
This commit is contained in:
parent
fc10b54f25
commit
64f773ab96
|
@ -3522,6 +3522,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)
|
||||
|
||||
|
|
|
@ -16524,8 +16524,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,
|
||||
|
@ -16548,7 +16548,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)
|
||||
|
@ -16566,6 +16567,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'),
|
||||
|
@ -16573,12 +16576,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)
|
||||
|
@ -16587,6 +16590,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})
|
||||
|
@ -16605,7 +16647,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)
|
||||
|
@ -16622,17 +16665,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)
|
||||
|
|
|
@ -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:
|
||||
|
@ -7584,7 +7589,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):
|
||||
|
|
Loading…
Reference in New Issue