From 45d73c49bd2f37ff0091f10a928b2ad548256bae Mon Sep 17 00:00:00 2001
From: Dmitry Guryanov <dguryanov@parallels.com>
Date: Thu, 23 Jul 2015 02:52:31 +0300
Subject: [PATCH] libvirt: do relative block rebase only with non-null base

You can't use VIR_DOMAIN_BLOCK_REBASE_RELATIVE with null
base, libvirt reports error in this case, so check if
base is non-null before setting this flag (relative argument
of BlockDevice.rebase method).

The error was introduced by commit
e9639913294036ff94f354e9c8ea18cd816243ab

Change-Id: Ife50f211fd54190a2665ad090f6208048b841690
Closes-Bug: #1475202
---
 nova/tests/unit/virt/libvirt/fakelibvirt.py | 10 +++++-
 nova/tests/unit/virt/libvirt/test_driver.py | 36 +++++++++++++++++++++
 nova/virt/libvirt/driver.py                 |  2 +-
 3 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/nova/tests/unit/virt/libvirt/fakelibvirt.py b/nova/tests/unit/virt/libvirt/fakelibvirt.py
index 532e6567d8af..8ec277e6bfc4 100644
--- a/nova/tests/unit/virt/libvirt/fakelibvirt.py
+++ b/nova/tests/unit/virt/libvirt/fakelibvirt.py
@@ -108,6 +108,7 @@ VIR_FROM_NWFILTER = 330
 VIR_FROM_REMOTE = 340
 VIR_FROM_RPC = 345
 VIR_FROM_NODEDEV = 666
+VIR_ERR_INVALID_ARG = 8
 VIR_ERR_NO_SUPPORT = 3
 VIR_ERR_XML_DETAIL = 350
 VIR_ERR_NO_DOMAIN = 420
@@ -739,7 +740,14 @@ class Domain(object):
     def blockResize(self, disk, size):
         pass
 
-    def blockRebase(self, disk, base, flags):
+    def blockRebase(self, disk, base, bandwidth=0, flags=0):
+        if (not base) and (flags and VIR_DOMAIN_BLOCK_REBASE_RELATIVE):
+            raise make_libvirtError(
+                    libvirtError,
+                    'flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is '
+                    'valid only with non-null base',
+                    error_code=VIR_ERR_INVALID_ARG,
+                    error_domain=VIR_FROM_QEMU)
         return 0
 
     def blockCommit(self, disk, base, top, flags):
diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py
index d8715198cebb..47275298d1e7 100644
--- a/nova/tests/unit/virt/libvirt/test_driver.py
+++ b/nova/tests/unit/virt/libvirt/test_driver.py
@@ -13118,6 +13118,10 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase):
                               'file_to_merge': 'snap.img',
                               'merge_target_file': 'other-snap.img'}
 
+        self.delete_info_3 = {'type': 'qcow2',
+                              'file_to_merge': None,
+                              'merge_target_file': None}
+
         self.delete_info_netdisk = {'type': 'qcow2',
                                     'file_to_merge': 'snap.img',
                                     'merge_target_file': 'root.img'}
@@ -13480,6 +13484,38 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase):
 
         self.mox.VerifyAll()
 
+    def test_volume_snapshot_delete_nonrelative_null_base(self):
+        # Deleting newest and last snapshot of a volume
+        # with blockRebase. So base of the new image will be null.
+
+        instance = objects.Instance(**self.inst)
+        snapshot_id = 'snapshot-1234'
+
+        domain = FakeVirtDomain(fake_xml=self.dom_xml)
+        guest = libvirt_guest.Guest(domain)
+
+        with contextlib.nested(
+            mock.patch.object(domain, 'XMLDesc', return_value=self.dom_xml),
+            mock.patch.object(self.drvr._host, 'get_guest',
+                              return_value=guest),
+            mock.patch.object(self.drvr._host, 'has_min_version',
+                              return_value=True),
+            mock.patch.object(domain, 'blockRebase'),
+            mock.patch.object(domain, 'blockJobInfo',
+                              return_value={'cur': 1000, 'end': 1000})
+        ) as (mock_xmldesc, mock_get_guest, mock_has_min_version,
+              mock_rebase, mock_job_info):
+
+            self.drvr._volume_snapshot_delete(self.c, instance,
+                                              self.volume_uuid, snapshot_id,
+                                              self.delete_info_3)
+
+            mock_xmldesc.assert_called_once_with(flags=0)
+            mock_get_guest.assert_called_once_with(instance)
+            mock_has_min_version.assert_called_once_with((1, 1, 1,))
+            mock_rebase.assert_called_once_with('vda', None, 0, flags=0)
+            mock_job_info.assert_called_once_with('vda', flags=0)
+
     def test_volume_snapshot_delete_outer_success(self):
         instance = objects.Instance(**self.inst)
         snapshot_id = 'snapshot-1234'
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index 1be421aab65e..0187a848320c 100644
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -1887,7 +1887,7 @@ class LibvirtDriver(driver.ComputeDriver):
             # (relative backing path seems to work for this case)
             try:
                 libvirt.VIR_DOMAIN_BLOCK_REBASE_RELATIVE
-                relative = True
+                relative = rebase_base is not None
             except AttributeError:
                 LOG.warn(_LW("Relative blockrebase support was not detected. "
                              "Continuing with old behaviour."))