Save updated libvirt domain XML after swapping volume
When a user calls the volume-update API, we swap_volume in the libvirt
driver from the old volume attachment to the new volume attachment.
Currently, we're saving the domain XML with the old configuration prior
to updating the volume and upon a soft-reboot request, it results in an
error:
Instance soft reboot failed: Cannot access storage file <old path>
and falls back to a hard reboot, which is like pulling the power cord,
possibly resulting in file system inconsistencies.
This changes to saving the new, updated domain XML after the volume
swap.
Closes-Bug: #1713857
NOTE(melwitt): The assert in test_swap_volume_block_old_libvirt had to
be modified to account for the additional XMLDesc call from the bug
fix. The test_swap_volume_block_old_libvirt doesn't exist in Pike, so
that's why we didn't hit this in the master patch.
Change-Id: I166cde5ad8b00699e4ec02609f0d7b69236d855d
(cherry picked from commit 5b008c6540
)
This commit is contained in:
parent
324d57bec3
commit
aa0e4ebd2c
|
@ -14809,8 +14809,10 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
|||
self.assertTrue(instance.cleaned)
|
||||
save.assert_called_once_with()
|
||||
|
||||
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
|
||||
def test_swap_volume_file(self, mock_is_job_complete):
|
||||
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete',
|
||||
return_value=True)
|
||||
def _test_swap_volume(self, mock_is_job_complete, source_type,
|
||||
resize=False, fail=False):
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
|
||||
|
||||
mock_dom = mock.MagicMock()
|
||||
|
@ -14818,67 +14820,87 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
|||
|
||||
with mock.patch.object(drvr._conn, 'defineXML',
|
||||
create=True) as mock_define:
|
||||
xmldoc = "<domain/>"
|
||||
srcfile = "/first/path"
|
||||
dstfile = "/second/path"
|
||||
|
||||
mock_dom.XMLDesc.return_value = xmldoc
|
||||
mock_dom.XMLDesc.return_value = mock.sentinel.orig_xml
|
||||
mock_dom.isPersistent.return_value = True
|
||||
mock_is_job_complete.return_value = True
|
||||
|
||||
drvr._swap_volume(guest, srcfile,
|
||||
mock.MagicMock(source_type='file',
|
||||
source_path=dstfile),
|
||||
1)
|
||||
def fake_rebase_success(*args, **kwargs):
|
||||
# Make sure the XML is set after the rebase so we know
|
||||
# get_xml_desc was called after the update.
|
||||
mock_dom.XMLDesc.return_value = mock.sentinel.new_xml
|
||||
|
||||
mock_dom.XMLDesc.assert_called_once_with(
|
||||
if not fail:
|
||||
mock_dom.blockRebase.side_effect = fake_rebase_success
|
||||
# If the swap succeeds, make sure we use the new XML to
|
||||
# redefine the domain.
|
||||
expected_xml = mock.sentinel.new_xml
|
||||
else:
|
||||
if resize:
|
||||
mock_dom.blockResize.side_effect = test.TestingException()
|
||||
expected_exception = test.TestingException
|
||||
else:
|
||||
mock_dom.blockRebase.side_effect = test.TestingException()
|
||||
expected_exception = exception.VolumeRebaseFailed
|
||||
# If the swap fails, make sure we use the original domain XML
|
||||
# to redefine the domain.
|
||||
expected_xml = mock.sentinel.orig_xml
|
||||
|
||||
# Run the swap volume code.
|
||||
mock_conf = mock.MagicMock(source_type=source_type,
|
||||
source_path=dstfile)
|
||||
if not fail:
|
||||
drvr._swap_volume(guest, srcfile, mock_conf, 1)
|
||||
else:
|
||||
self.assertRaises(expected_exception, drvr._swap_volume, guest,
|
||||
srcfile, mock_conf, 1)
|
||||
|
||||
# Verify we read the original persistent config.
|
||||
expected_call_count = 1
|
||||
expected_calls = [mock.call(
|
||||
flags=(fakelibvirt.VIR_DOMAIN_XML_INACTIVE |
|
||||
fakelibvirt.VIR_DOMAIN_XML_SECURE))
|
||||
mock_dom.blockRebase.assert_called_once_with(
|
||||
srcfile, dstfile, 0, flags=(
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_COPY |
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))
|
||||
mock_dom.blockResize.assert_called_once_with(
|
||||
srcfile, 1 * units.Gi / units.Ki)
|
||||
mock_define.assert_called_once_with(xmldoc)
|
||||
fakelibvirt.VIR_DOMAIN_XML_SECURE))]
|
||||
if not fail:
|
||||
# Verify we read the updated live config.
|
||||
expected_call_count = 2
|
||||
expected_calls.append(
|
||||
mock.call(flags=fakelibvirt.VIR_DOMAIN_XML_SECURE))
|
||||
self.assertEqual(expected_call_count, mock_dom.XMLDesc.call_count)
|
||||
mock_dom.XMLDesc.assert_has_calls(expected_calls)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
|
||||
def test_swap_volume_block(self, mock_is_job_complete):
|
||||
# Verify we called with the correct flags.
|
||||
expected_flags = (fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_COPY |
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT)
|
||||
if source_type == 'block':
|
||||
expected_flags = (expected_flags |
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)
|
||||
mock_dom.blockRebase.assert_called_once_with(srcfile, dstfile, 0,
|
||||
flags=expected_flags)
|
||||
|
||||
# Verify we defined the expected XML.
|
||||
mock_define.assert_called_once_with(expected_xml)
|
||||
|
||||
# Verify we called resize with the correct args.
|
||||
if resize:
|
||||
mock_dom.blockResize.assert_called_once_with(
|
||||
srcfile, 1 * units.Gi / units.Ki)
|
||||
|
||||
def test_swap_volume_file(self):
|
||||
self._test_swap_volume('file')
|
||||
|
||||
def test_swap_volume_block(self):
|
||||
"""If the swapped volume is type="block", make sure that we give
|
||||
libvirt the correct VIR_DOMAIN_BLOCK_REBASE_COPY_DEV flag to ensure the
|
||||
correct type="block" XML is generated (bug 1691195)
|
||||
"""
|
||||
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI())
|
||||
self._test_swap_volume('block')
|
||||
|
||||
mock_dom = mock.MagicMock()
|
||||
guest = libvirt_guest.Guest(mock_dom)
|
||||
def test_swap_volume_rebase_fail(self):
|
||||
self._test_swap_volume('block', fail=True)
|
||||
|
||||
with mock.patch.object(drvr._conn, 'defineXML',
|
||||
create=True) as mock_define:
|
||||
xmldoc = "<domain/>"
|
||||
srcfile = "/first/path"
|
||||
dstfile = "/second/path"
|
||||
|
||||
mock_dom.XMLDesc.return_value = xmldoc
|
||||
mock_dom.isPersistent.return_value = True
|
||||
mock_is_job_complete.return_value = True
|
||||
|
||||
drvr._swap_volume(guest, srcfile,
|
||||
mock.MagicMock(source_type='block',
|
||||
source_path=dstfile),
|
||||
1)
|
||||
|
||||
mock_dom.XMLDesc.assert_called_once_with(
|
||||
flags=(fakelibvirt.VIR_DOMAIN_XML_INACTIVE |
|
||||
fakelibvirt.VIR_DOMAIN_XML_SECURE))
|
||||
mock_dom.blockRebase.assert_called_once_with(
|
||||
srcfile, dstfile, 0, flags=(
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_COPY |
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_COPY_DEV |
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT))
|
||||
mock_dom.blockResize.assert_called_once_with(
|
||||
srcfile, 1 * units.Gi / units.Ki)
|
||||
mock_define.assert_called_once_with(xmldoc)
|
||||
def test_swap_volume_resize_fail(self):
|
||||
self._test_swap_volume('file', resize=True, fail=True)
|
||||
|
||||
@mock.patch('nova.virt.libvirt.guest.BlockDevice.is_job_complete')
|
||||
def test_swap_volume_block_old_libvirt(self, mock_is_job_complete):
|
||||
|
@ -14914,9 +14936,11 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
|||
libvirt_driver.libvirt.VIR_DOMAIN_BLOCK_REBASE_COPY_DEV = \
|
||||
copy_dev
|
||||
|
||||
mock_dom.XMLDesc.assert_called_once_with(
|
||||
flags=(fakelibvirt.VIR_DOMAIN_XML_INACTIVE |
|
||||
fakelibvirt.VIR_DOMAIN_XML_SECURE))
|
||||
self.assertEqual(2, mock_dom.XMLDesc.call_count)
|
||||
call1 = mock.call(flags=(fakelibvirt.VIR_DOMAIN_XML_INACTIVE |
|
||||
fakelibvirt.VIR_DOMAIN_XML_SECURE))
|
||||
call2 = mock.call(flags=fakelibvirt.VIR_DOMAIN_XML_SECURE)
|
||||
mock_dom.XMLDesc.assert_has_calls([call1, call2])
|
||||
mock_dom.blockRebase.assert_called_once_with(
|
||||
srcfile, dstfile, 0, flags=(
|
||||
fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_COPY |
|
||||
|
|
|
@ -1266,7 +1266,8 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
"""Swap existing disk with a new block device."""
|
||||
dev = guest.get_block_device(disk_path)
|
||||
|
||||
# Save a copy of the domain's persistent XML file
|
||||
# Save a copy of the domain's persistent XML file. We'll use this
|
||||
# to redefine the domain if anything fails during the volume swap.
|
||||
xml = guest.get_xml_desc(dump_inactive=True, dump_sensitive=True)
|
||||
|
||||
# Abort is an idempotent operation, so make sure any block
|
||||
|
@ -1324,6 +1325,14 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
|
||||
if resize_to:
|
||||
dev.resize(resize_to * units.Gi / units.Ki)
|
||||
|
||||
# Make sure we will redefine the domain using the updated
|
||||
# configuration after the volume was swapped. The dump_inactive
|
||||
# keyword arg controls whether we pull the inactive (persistent)
|
||||
# or active (live) config from the domain. We want to pull the
|
||||
# live config after the volume was updated to use when we redefine
|
||||
# the domain.
|
||||
xml = guest.get_xml_desc(dump_inactive=False, dump_sensitive=True)
|
||||
finally:
|
||||
self._host.write_instance_config(xml)
|
||||
|
||||
|
|
Loading…
Reference in New Issue