Simplify libvirt snapshot code path

The "cold" snapshot code path was unnecessarily complex. We were
traversing 3 layers of abstraction to reach 3 lines of code. I've
completely eliminated snapshot.py by moving all "cold" snapshot
functions into imagebackend.py and adjusting the libvirt driver
accordingly. This is the first pass of several I intend to make
against imagebackend.py, as it is far too difficult to read given
how basic the basic level of functionality it is intended to
provide.

Change-Id: Idfacffb7210ae61193ee4f2360d4b38f330a177c

6 files changed, 56 insertions, 121 deletions

diff --git a/nova/tests/fake_libvirt_utils.py b/nova/tests/fake_libvirt_utils.py
index b3d842468..285a4b7e3 100644
--- a/nova/tests/fake_libvirt_utils.py
+++ b/nova/tests/fake_libvirt_utils.py
@@ -144,3 +144,7 @@ def fetch_image(context, target, image_id, user_id, project_id):
 def get_instance_path(instance):
     # TODO(mikal): we should really just call the real one here
     return os.path.join(CONF.instances_path, instance['name'])
+
+
+def pick_disk_driver_name(is_block_dev=False):
+    return "qemu"
diff --git a/nova/tests/test_libvirt.py b/nova/tests/test_libvirt.py
index 12b1c1a69..c93bb0168 100644
--- a/nova/tests/test_libvirt.py
+++ b/nova/tests/test_libvirt.py
@@ -273,7 +273,7 @@ class LibvirtConnTestCase(test.TestCase):
             'nova.virt.libvirt.driver.libvirt_utils',
             fake_libvirt_utils))
         self.useFixture(fixtures.MonkeyPatch(
-            'nova.virt.libvirt.snapshots.libvirt_utils',
+            'nova.virt.libvirt.imagebackend.libvirt_utils',
             fake_libvirt_utils))
 
         def fake_extend(image, size):
diff --git a/nova/tests/test_virt_drivers.py b/nova/tests/test_virt_drivers.py
index 67452bc24..a94fdc3c5 100644
--- a/nova/tests/test_virt_drivers.py
+++ b/nova/tests/test_virt_drivers.py
@@ -84,7 +84,7 @@ class _FakeDriverBackendTestCase(object):
             'nova.virt.libvirt.driver.libvirt_utils',
             fake_libvirt_utils))
         self.useFixture(fixtures.MonkeyPatch(
-            'nova.virt.libvirt.snapshots.libvirt_utils',
+            'nova.virt.libvirt.imagebackend.libvirt_utils',
             fake_libvirt_utils))
         self.useFixture(fixtures.MonkeyPatch(
             'nova.virt.libvirt.firewall.libvirt',
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index d37d6e7df..485f661e5 100755
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -827,7 +827,8 @@ class LibvirtDriver(driver.ComputeDriver):
         #            can be validated.
         if self.has_min_version(MIN_LIBVIRT_LIVESNAPSHOT_VERSION,
                                 MIN_QEMU_LIVESNAPSHOT_VERSION,
-                                REQ_HYPERVISOR_LIVESNAPSHOT):
+                                REQ_HYPERVISOR_LIVESNAPSHOT) \
+                and not source_format == "lvm":
             live_snapshot = True
         else:
             live_snapshot = False
@@ -843,15 +844,17 @@ class LibvirtDriver(driver.ComputeDriver):
             if state == power_state.RUNNING or state == power_state.PAUSED:
                 virt_dom.managedSave(0)
 
+        snapshot_backend = self.image_backend.snapshot(disk_path,
+                snapshot_name,
+                image_type=source_format)
+
         if live_snapshot:
             LOG.info(_("Beginning live snapshot process"),
                      instance=instance)
         else:
             LOG.info(_("Beginning cold snapshot process"),
                      instance=instance)
-            snapshot = self.image_backend.snapshot(disk_path, snapshot_name,
-                                                   image_type=source_format)
-            snapshot.create()
+            snapshot_backend.snapshot_create()
 
         update_task_state(task_state=task_states.IMAGE_PENDING_UPLOAD)
         snapshot_directory = CONF.libvirt_snapshots_directory
@@ -866,10 +869,10 @@ class LibvirtDriver(driver.ComputeDriver):
                     self._live_snapshot(virt_dom, disk_path, out_path,
                                         image_format)
                 else:
-                    snapshot.extract(out_path, image_format)
+                    snapshot_backend.snapshot_extract(out_path, image_format)
             finally:
                 if not live_snapshot:
-                    snapshot.delete()
+                    snapshot_backend.snapshot_delete()
                 # NOTE(dkang): because previous managedSave is not called
                 #              for LXC, _create_domain must not be called.
                 if CONF.libvirt_type != 'lxc' and not live_snapshot:
diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py
index 0d5e8cb66..74148a866 100755
--- a/nova/virt/libvirt/imagebackend.py
+++ b/nova/virt/libvirt/imagebackend.py
@@ -25,8 +25,8 @@ from nova.openstack.common import fileutils
 from nova.openstack.common import lockutils
 from nova import utils
 from nova.virt.disk import api as disk
+from nova.virt import images
 from nova.virt.libvirt import config as vconfig
-from nova.virt.libvirt import snapshots
 from nova.virt.libvirt import utils as libvirt_utils
 
 __imagebackend_opts = [
@@ -129,22 +129,25 @@ class Image(object):
             self.create_image(call_if_not_exists, base, size,
                               *args, **kwargs)
 
-    @abc.abstractmethod
-    def snapshot(self, name):
-        """Create snapshot object for this image
+    def snapshot_create(self):
+        raise NotImplementedError
 
-        :name: snapshot name
-        """
-        pass
+    def snapshot_extract(self, target, out_format):
+        raise NotImplementedError
+
+    def snapshot_delete(self):
+        raise NotImplementedError
 
 
 class Raw(Image):
-    def __init__(self, instance=None, name=None, path=None):
+    def __init__(self, instance=None, disk_name=None, path=None,
+                 snapshot_name=None):
         super(Raw, self).__init__("file", "raw", is_block_dev=False)
 
         self.path = (path or
                      os.path.join(libvirt_utils.get_instance_path(instance),
-                                  name))
+                                  disk_name))
+        self.snapshot_name = snapshot_name
 
     def create_image(self, prepare_template, base, size, *args, **kwargs):
         @lockutils.synchronized(base, 'nova-', external=True,
@@ -164,17 +167,25 @@ class Raw(Image):
                 with utils.remove_path_on_error(self.path):
                     copy_raw_image(base, self.path, size)
 
-    def snapshot(self, name):
-        return snapshots.RawSnapshot(self.path, name)
+    def snapshot_create(self):
+        pass
+
+    def snapshot_extract(self, target, out_format):
+        images.convert_image(self.path, target, out_format)
+
+    def snapshot_delete(self):
+        pass
 
 
 class Qcow2(Image):
-    def __init__(self, instance=None, name=None, path=None):
+    def __init__(self, instance=None, disk_name=None, path=None,
+                 snapshot_name=None):
         super(Qcow2, self).__init__("file", "qcow2", is_block_dev=False)
 
         self.path = (path or
                      os.path.join(libvirt_utils.get_instance_path(instance),
-                                  name))
+                                  disk_name))
+        self.snapshot_name = snapshot_name
 
     def create_image(self, prepare_template, base, size, *args, **kwargs):
         @lockutils.synchronized(base, 'nova-', external=True,
@@ -190,8 +201,16 @@ class Qcow2(Image):
             with utils.remove_path_on_error(self.path):
                 copy_qcow2_image(base, self.path, size)
 
-    def snapshot(self, name):
-        return snapshots.Qcow2Snapshot(self.path, name)
+    def snapshot_create(self):
+        libvirt_utils.create_snapshot(self.path, self.snapshot_name)
+
+    def snapshot_extract(self, target, out_format):
+        libvirt_utils.extract_snapshot(self.path, 'qcow2',
+                                       self.snapshot_name, target,
+                                       out_format)
+
+    def snapshot_delete(self):
+        libvirt_utils.delete_snapshot(self.path, self.snapshot_name)
 
 
 class Lvm(Image):
@@ -199,7 +218,8 @@ class Lvm(Image):
     def escape(filename):
         return filename.replace('_', '__')
 
-    def __init__(self, instance=None, name=None, path=None):
+    def __init__(self, instance=None, disk_name=None, path=None,
+                 snapshot_name=None):
         super(Lvm, self).__init__("block", "raw", is_block_dev=True)
 
         if path:
@@ -214,7 +234,7 @@ class Lvm(Image):
                                      ' flag to use LVM images.'))
             self.vg = CONF.libvirt_images_volume_group
             self.lv = '%s_%s' % (self.escape(instance['name']),
-                                 self.escape(name))
+                                 self.escape(disk_name))
             self.path = os.path.join('/dev', self.vg, self.lv)
 
         self.sparse = CONF.libvirt_sparse_logical_volumes
@@ -254,9 +274,6 @@ class Lvm(Image):
             with excutils.save_and_reraise_exception():
                 libvirt_utils.remove_logical_volumes(path)
 
-    def snapshot(self, name):
-        return snapshots.LvmSnapshot(self.path, name)
-
 
 class Backend(object):
     def __init__(self, use_cow):
@@ -275,7 +292,7 @@ class Backend(object):
             raise RuntimeError(_('Unknown image_type=%s') % image_type)
         return image
 
-    def image(self, instance, name, image_type=None):
+    def image(self, instance, disk_name, image_type=None):
         """Constructs image for selected backend
 
         :instance: Instance name.
@@ -284,9 +301,9 @@ class Backend(object):
         Optional, is CONF.libvirt_images_type by default.
         """
         backend = self.backend(image_type)
-        return backend(instance=instance, name=name)
+        return backend(instance=instance, disk_name=disk_name)
 
-    def snapshot(self, path, snapshot_name, image_type=None):
+    def snapshot(self, disk_path, snapshot_name, image_type=None):
         """Returns snapshot for given image
 
         :path: path to image
@@ -294,4 +311,4 @@ class Backend(object):
         :image_type: type of image
         """
         backend = self.backend(image_type)
-        return backend(path=path).snapshot(snapshot_name)
+        return backend(path=disk_path, snapshot_name=snapshot_name)
diff --git a/nova/virt/libvirt/snapshots.py b/nova/virt/libvirt/snapshots.py
deleted file mode 100644
index c85550eae..000000000
--- a/nova/virt/libvirt/snapshots.py
+++ /dev/null
@@ -1,89 +0,0 @@
-# vim: tabstop=4 shiftwidth=4 softtabstop=4
-
-# Copyright 2012 Grid Dynamics
-# All Rights Reserved.
-#
-#    Licensed under the Apache License, Version 2.0 (the "License"); you may
-#    not use this file except in compliance with the License. You may obtain
-#    a copy of the License at
-#
-#         http://www.apache.org/licenses/LICENSE-2.0
-#
-#    Unless required by applicable law or agreed to in writing, software
-#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-#    License for the specific language governing permissions and limitations
-#    under the License.
-
-import abc
-
-from nova.virt import images
-from nova.virt.libvirt import utils as libvirt_utils
-
-
-class Snapshot(object):
-    @abc.abstractmethod
-    def create(self):
-        """Create new snapshot."""
-        pass
-
-    @abc.abstractmethod
-    def extract(self, target, out_format):
-        """Extract snapshot content to file
-
-        :target: path to extraction
-        :out_format: format of extraction (raw, qcow2, ...)
-        """
-        pass
-
-    @abc.abstractmethod
-    def delete(self):
-        """Delete snapshot."""
-        pass
-
-
-class RawSnapshot(object):
-    def __init__(self, path, name):
-        self.path = path
-        self.name = name
-
-    def create(self):
-        pass
-
-    def extract(self, target, out_format):
-        images.convert_image(self.path, target, out_format)
-
-    def delete(self):
-        pass
-
-
-class Qcow2Snapshot(object):
-    def __init__(self, path, name):
-        self.path = path
-        self.name = name
-
-    def create(self):
-        libvirt_utils.create_snapshot(self.path, self.name)
-
-    def extract(self, target, out_format):
-        libvirt_utils.extract_snapshot(self.path, 'qcow2',
-                                       self.name, target,
-                                       out_format)
-
-    def delete(self):
-        libvirt_utils.delete_snapshot(self.path, self.name)
-
-
-class LvmSnapshot(object):
-    def __init__(self, path, name):
-        self.path = path
-        self.name = name
-
-    def create(self):
-        raise NotImplementedError(_("LVM snapshots not implemented"))
-
-    def extract(self, target, out_format):
-        raise NotImplementedError(_("LVM snapshots not implemented"))
-
-    def delete(self):
-        raise NotImplementedError(_("LVM snapshots not implemented"))