From 7458c85daaffc56199094f66f29b2b86a4725955 Mon Sep 17 00:00:00 2001 From: Deepak C Shetty Date: Mon, 30 Mar 2015 06:46:37 +0000 Subject: [PATCH] libvirt: Use 'relative' flag for online snapshot's commit/rebase operations libvirt added support for _RELATIVE flag in v1.2.7 and when available this flag _must_ be used to ensure backing paths are maintained relative during snapshot delete operations. This patch uses _RELATIVE flag if available, for both blockCommit and blockRebase cases for both file and network backed disks. If _RELATIVE flag is unavailable, it raises excp for blockCommit case (since we can't get relative paths and Cinder flow is broken), but only puts out a warning message for blockRebase case (since relative paths works even for older libvirt) Closes-Bug: 1438027 Change-Id: I9bda95cd4ab09aed88536d4a988d6c6579441c37 --- nova/tests/unit/virt/libvirt/fakelibvirt.py | 2 + nova/tests/unit/virt/libvirt/test_driver.py | 179 +++++++++++++++++++- nova/virt/libvirt/driver.py | 47 +++-- 3 files changed, 211 insertions(+), 17 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py index 56a61846d7d3..e5009ff35703 100644 --- a/nova/tests/unit/virt/libvirt/fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py @@ -133,6 +133,8 @@ VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE = 64 # blockCommit flags VIR_DOMAIN_BLOCK_COMMIT_RELATIVE = 4 +# blockRebase flags +VIR_DOMAIN_BLOCK_REBASE_RELATIVE = 8 VIR_CONNECT_LIST_DOMAINS_ACTIVE = 1 diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index b98baf02bfad..0135bec0ea28 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -13188,6 +13188,10 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): def test_volume_snapshot_delete_1(self): """Deleting newest snapshot -- blockRebase.""" + # libvirt lib doesn't have VIR_DOMAIN_BLOCK_REBASE_RELATIVE flag + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_REBASE_RELATIVE') + self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) + instance = objects.Instance(**self.inst) snapshot_id = 'snapshot-1234' @@ -13215,9 +13219,14 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): snapshot_id, self.delete_info_1) self.mox.VerifyAll() + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_REBASE_RELATIVE': 8}) - def test_volume_snapshot_delete_2(self): - """Deleting older snapshot -- blockCommit.""" + def test_volume_snapshot_delete_relative_1(self): + """Deleting newest snapshot -- blockRebase using relative flag""" + + # Ensure the libvirt lib has VIR_DOMAIN_BLOCK_REBASE_RELATIVE + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_REBASE_RELATIVE': 8}) + self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) instance = objects.Instance(**self.inst) snapshot_id = 'snapshot-1234' @@ -13235,7 +13244,80 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): self.drvr._host.get_domain(instance).AndReturn(domain) self.drvr._host.has_min_version(mox.IgnoreArg()).AndReturn(True) - domain.blockCommit('vda', 'other-snap.img', 'snap.img', 0, 0) + domain.blockRebase('vda', 'snap.img', 0, + fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_RELATIVE) + + domain.blockJobInfo('vda', 0).AndReturn({'cur': 1, 'end': 1000}) + domain.blockJobInfo('vda', 0).AndReturn({'cur': 1000, 'end': 1000}) + + self.mox.ReplayAll() + + self.drvr._volume_snapshot_delete(self.c, instance, self.volume_uuid, + snapshot_id, self.delete_info_1) + + self.mox.VerifyAll() + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_REBASE_RELATIVE') + + def test_volume_snapshot_delete_2(self): + """Deleting older snapshot -- blockCommit.""" + + # libvirt lib doesn't have VIR_DOMAIN_BLOCK_COMMIT_RELATIVE + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_COMMIT_RELATIVE') + self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) + + instance = objects.Instance(**self.inst) + snapshot_id = 'snapshot-1234' + + domain = FakeVirtDomain(fake_xml=self.dom_xml) + self.mox.StubOutWithMock(domain, 'XMLDesc') + domain.XMLDesc(0).AndReturn(self.dom_xml) + + self.mox.StubOutWithMock(self.drvr._host, 'get_domain') + self.mox.StubOutWithMock(self.drvr._host, 'has_min_version') + self.mox.StubOutWithMock(domain, 'blockRebase') + self.mox.StubOutWithMock(domain, 'blockCommit') + self.mox.StubOutWithMock(domain, 'blockJobInfo') + + self.drvr._host.get_domain(instance).AndReturn(domain) + self.drvr._host.has_min_version(mox.IgnoreArg()).AndReturn(True) + + self.mox.ReplayAll() + + self.assertRaises(exception.Invalid, + self.drvr._volume_snapshot_delete, + self.c, + instance, + self.volume_uuid, + snapshot_id, + self.delete_info_2) + + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_COMMIT_RELATIVE': 4}) + + def test_volume_snapshot_delete_relative_2(self): + """Deleting older snapshot -- blockCommit using relative flag""" + + # Ensure the libvirt lib has VIR_DOMAIN_BLOCK_COMMIT_RELATIVE + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_COMMIT_RELATIVE': 4}) + self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) + + instance = objects.Instance(**self.inst) + snapshot_id = 'snapshot-1234' + + domain = FakeVirtDomain(fake_xml=self.dom_xml) + self.mox.StubOutWithMock(domain, 'XMLDesc') + domain.XMLDesc(0).AndReturn(self.dom_xml) + + self.mox.StubOutWithMock(self.drvr._host, 'get_domain') + self.mox.StubOutWithMock(self.drvr._host, 'has_min_version') + self.mox.StubOutWithMock(domain, 'blockRebase') + self.mox.StubOutWithMock(domain, 'blockCommit') + self.mox.StubOutWithMock(domain, 'blockJobInfo') + + self.drvr._host.get_domain(instance).AndReturn(domain) + self.drvr._host.has_min_version(mox.IgnoreArg()).AndReturn(True) + + domain.blockCommit('vda', 'other-snap.img', 'snap.img', 0, + fakelibvirt.VIR_DOMAIN_BLOCK_COMMIT_RELATIVE) domain.blockJobInfo('vda', 0).AndReturn({'cur': 1, 'end': 1000}) domain.blockJobInfo('vda', 0).AndReturn({}) @@ -13246,6 +13328,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): snapshot_id, self.delete_info_2) self.mox.VerifyAll() + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_COMMIT_RELATIVE') def test_volume_snapshot_delete_outer_success(self): instance = objects.Instance(**self.inst) @@ -13344,7 +13427,8 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): def XMLDesc(self, *args): return self.dom_netdisk_xml - # Ensure the libvirt lib has VIR_DOMAIN_BLOCK_COMMIT_RELATIVE + # libvirt lib doesn't have VIR_DOMAIN_BLOCK_REBASE_RELATIVE + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_REBASE_RELATIVE') self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) instance = objects.Instance(**self.inst) @@ -13370,14 +13454,99 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): self.mox.ReplayAll() + self.drvr._volume_snapshot_delete(self.c, instance, self.volume_uuid, + snapshot_id, self.delete_info_1) + self.mox.VerifyAll() + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_REBASE_RELATIVE': 8}) + + def test_volume_snapshot_delete_netdisk_relative_1(self): + """Delete newest snapshot -- blockRebase for libgfapi/network disk.""" + + class FakeNetdiskDomain(FakeVirtDomain): + def __init__(self, *args, **kwargs): + super(FakeNetdiskDomain, self).__init__(*args, **kwargs) + + def XMLDesc(self, *args): + return self.dom_netdisk_xml + + # Ensure the libvirt lib has VIR_DOMAIN_BLOCK_REBASE_RELATIVE + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_REBASE_RELATIVE': 8}) + self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) + + instance = objects.Instance(**self.inst) + snapshot_id = 'snapshot-1234' + + domain = FakeNetdiskDomain(fake_xml=self.dom_netdisk_xml) + self.mox.StubOutWithMock(domain, 'XMLDesc') + domain.XMLDesc(0).AndReturn(self.dom_netdisk_xml) + + self.mox.StubOutWithMock(self.drvr._host, 'get_domain') + self.mox.StubOutWithMock(self.drvr._host, 'has_min_version') + self.mox.StubOutWithMock(domain, 'blockRebase') + self.mox.StubOutWithMock(domain, 'blockCommit') + self.mox.StubOutWithMock(domain, 'blockJobInfo') + + self.drvr._host.get_domain(instance).AndReturn(domain) + self.drvr._host.has_min_version(mox.IgnoreArg()).AndReturn(True) + + domain.blockRebase('vdb', 'vdb[1]', 0, + fakelibvirt.VIR_DOMAIN_BLOCK_REBASE_RELATIVE) + + domain.blockJobInfo('vdb', 0).AndReturn({'cur': 1, 'end': 1000}) + domain.blockJobInfo('vdb', 0).AndReturn({'cur': 1000, 'end': 1000}) + + self.mox.ReplayAll() + self.drvr._volume_snapshot_delete(self.c, instance, self.volume_uuid, snapshot_id, self.delete_info_1) self.mox.VerifyAll() + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_REBASE_RELATIVE') def test_volume_snapshot_delete_netdisk_2(self): """Delete older snapshot -- blockCommit for libgfapi/network disk.""" + class FakeNetdiskDomain(FakeVirtDomain): + def __init__(self, *args, **kwargs): + super(FakeNetdiskDomain, self).__init__(*args, **kwargs) + + def XMLDesc(self, *args): + return self.dom_netdisk_xml + + # libvirt lib doesn't have VIR_DOMAIN_BLOCK_COMMIT_RELATIVE + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_COMMIT_RELATIVE') + self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) + + instance = objects.Instance(**self.inst) + snapshot_id = 'snapshot-1234' + + domain = FakeNetdiskDomain(fake_xml=self.dom_netdisk_xml) + self.mox.StubOutWithMock(domain, 'XMLDesc') + domain.XMLDesc(0).AndReturn(self.dom_netdisk_xml) + + self.mox.StubOutWithMock(self.drvr._host, 'get_domain') + self.mox.StubOutWithMock(self.drvr._host, 'has_min_version') + self.mox.StubOutWithMock(domain, 'blockRebase') + self.mox.StubOutWithMock(domain, 'blockCommit') + self.mox.StubOutWithMock(domain, 'blockJobInfo') + + self.drvr._host.get_domain(instance).AndReturn(domain) + self.drvr._host.has_min_version(mox.IgnoreArg()).AndReturn(True) + + self.mox.ReplayAll() + + self.assertRaises(exception.Invalid, + self.drvr._volume_snapshot_delete, + self.c, + instance, + self.volume_uuid, + snapshot_id, + self.delete_info_netdisk) + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_COMMIT_RELATIVE': 4}) + + def test_volume_snapshot_delete_netdisk_relative_2(self): + """Delete older snapshot -- blockCommit for libgfapi/network disk.""" + class FakeNetdiskDomain(FakeVirtDomain): def __init__(self, *args, **kwargs): super(FakeNetdiskDomain, self).__init__(*args, **kwargs) @@ -13386,6 +13555,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): return self.dom_netdisk_xml # Ensure the libvirt lib has VIR_DOMAIN_BLOCK_COMMIT_RELATIVE + fakelibvirt.__dict__.update({'VIR_DOMAIN_BLOCK_COMMIT_RELATIVE': 4}) self.stubs.Set(libvirt_driver, 'libvirt', fakelibvirt) instance = objects.Instance(**self.inst) @@ -13417,6 +13587,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): self.delete_info_netdisk) self.mox.VerifyAll() + fakelibvirt.__dict__.pop('VIR_DOMAIN_BLOCK_COMMIT_RELATIVE') def _fake_convert_image(source, dest, out_format, diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 312f5a330561..f6e7a686b42b 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -341,9 +341,9 @@ MIN_QEMU_LIVESNAPSHOT_VERSION = (1, 3, 0) MIN_LIBVIRT_BLOCKIO_VERSION = (0, 10, 2) # BlockJobInfo management requirement MIN_LIBVIRT_BLOCKJOBINFO_VERSION = (1, 1, 1) -# Relative block commit (feature is detected, +# Relative block commit & rebase (feature is detected, # this version is only used for messaging) -MIN_LIBVIRT_BLOCKCOMMIT_RELATIVE_VERSION = (1, 2, 7) +MIN_LIBVIRT_BLOCKJOB_RELATIVE_VERSION = (1, 2, 7) # libvirt discard feature MIN_LIBVIRT_DISCARD_VERSION = (1, 0, 6) MIN_QEMU_DISCARD_VERSION = (1, 6, 0) @@ -1883,6 +1883,19 @@ class LibvirtDriver(driver.ComputeDriver): active_disk_object.backing_store) rebase_bw = 0 + # NOTE(deepakcs): libvirt added support for _RELATIVE in v1.2.7, + # and when available this flag _must_ be used to ensure backing + # paths are maintained relative by qemu. + # + # If _RELATIVE flag not found, continue with old behaviour + # (relative backing path seems to work for this case) + try: + if rebase_base is not None: + rebase_flags |= libvirt.VIR_DOMAIN_BLOCK_REBASE_RELATIVE + except AttributeError: + LOG.warn(_LW("Relative blockrebase support was not detected. " + "Continuing with old behaviour.")) + LOG.debug('disk: %(disk)s, base: %(base)s, ' 'bw: %(bw)s, flags: %(flags)s', {'disk': rebase_disk, @@ -1908,22 +1921,30 @@ class LibvirtDriver(driver.ComputeDriver): commit_disk = my_dev commit_flags = 0 + # NOTE(deepakcs): libvirt added support for _RELATIVE in v1.2.7, + # and when available this flag _must_ be used to ensure backing + # paths are maintained relative by qemu. + # + # If _RELATIVE flag not found, raise exception as relative backing + # path may not be maintained and Cinder flow is broken if allowed + # to continue. + try: + commit_flags |= libvirt.VIR_DOMAIN_BLOCK_COMMIT_RELATIVE + except AttributeError: + ver = '.'.join( + [str(x) for x in + MIN_LIBVIRT_BLOCKJOB_RELATIVE_VERSION]) + msg = _("Relative blockcommit support was not detected. " + "Libvirt '%s' or later is required for online " + "deletion of file/network storage-backed volume " + "snapshots.") % ver + raise exception.Invalid(msg) + if active_protocol is not None: my_snap_base = _get_snap_dev(delete_info['merge_target_file'], active_disk_object.backing_store) my_snap_top = _get_snap_dev(delete_info['file_to_merge'], active_disk_object.backing_store) - try: - commit_flags |= libvirt.VIR_DOMAIN_BLOCK_COMMIT_RELATIVE - except AttributeError: - ver = '.'.join( - [str(x) for x in - MIN_LIBVIRT_BLOCKCOMMIT_RELATIVE_VERSION]) - msg = _("Relative blockcommit support was not detected. " - "Libvirt '%s' or later is required for online " - "deletion of network storage-backed volume " - "snapshots.") % ver - raise exception.Invalid(msg) commit_base = my_snap_base or delete_info['merge_target_file'] commit_top = my_snap_top or delete_info['file_to_merge']