Make snapshots with qemu-img instead of libvirt

* snapshot is only supposed to snapshot the root drive, whereas
   libvirt snapshots snapshot the memory and all attached disks
 * removes silly qemu_img flag
 * fixes bug 946830

Change-Id: I6afc9dbaa855f06864cd5a37f89ad63555e35d23

nova/rootwrap/compute.py

@@ -179,6 +179,10 @@ filterlist = [
     # nova/virt/xenapi/vm_utils.py: 'mkfs'
     filters.CommandFilter("/sbin/mkfs", "root"),
 
+    # nova/virt/libvirt/utils.py: 'qemu-img'
+    filters.CommandFilter("/usr/bin/qemu-img", "root"),
+
     # nova/virt/libvirt/connection.py:
     filters.ReadFileFilter("/etc/iscsi/initiatorname.iscsi"),
+
     ]

nova/tests/fake_libvirt_utils.py

@@ -61,6 +61,14 @@ def chown(path, owner):
     pass
 
 
+def create_snapshot(disk_path, snapshot_name):
+    pass
+
+
+def delete_snapshot(disk_path, snapshot_name):
+    pass
+
+
 def extract_snapshot(disk_path, source_fmt, snapshot_name, out_path, dest_fmt):
     files[out_path] = ''
 

nova/tests/test_libvirt.py

@@ -96,8 +96,14 @@ class FakeVirtDomain(object):
     def name(self):
         return "fake-domain %s" % self
 
-    def snapshotCreateXML(self, *args):
+    def info(self):
-        return FakeVirDomainSnapshot(self)
+        return [power_state.RUNNING, None, None, None, None]
+
+    def create(self):
+        pass
+
+    def managedSave(self, *args):
+        pass
 
     def createWithFlags(self, launch_flags):
         pass

nova/virt/libvirt/connection.py

@@ -616,21 +616,20 @@ class LibvirtConnection(driver.ComputeDriver):
         if 'container_format' in base:
             metadata['container_format'] = base['container_format']
 
-        # Make the snapshot
-        snapshot_name = uuid.uuid4().hex
-        snapshot_xml = """
-        <domainsnapshot>
-            <name>%s</name>
-        </domainsnapshot>
-        """ % snapshot_name
-        snapshot_ptr = virt_dom.snapshotCreateXML(snapshot_xml, 0)
-
         # Find the disk
         xml_desc = virt_dom.XMLDesc(0)
         domain = ElementTree.fromstring(xml_desc)
         source = domain.find('devices/disk/source')
         disk_path = source.get('file')
 
+        snapshot_name = uuid.uuid4().hex
+
+        (state, _max_mem, _mem, _cpus, _t) = virt_dom.info()
+        if state == power_state.RUNNING:
+            virt_dom.managedSave(0)
+        # Make the snapshot
+        libvirt_utils.create_snapshot(disk_path, snapshot_name)
+
         # Export the snapshot to a raw image
         with utils.tempdir() as tmpdir:
             try:
@@ -638,6 +637,11 @@ class LibvirtConnection(driver.ComputeDriver):
                 libvirt_utils.extract_snapshot(disk_path, source_format,
                                                snapshot_name, out_path,
                                                image_format)
+            finally:
+                libvirt_utils.delete_snapshot(disk_path, snapshot_name)
+                if state == power_state.RUNNING:
+                    virt_dom.create()
+
             # Upload that image to the image service
             with libvirt_utils.file_open(out_path) as image_file:
                 image_service.update(context,
@@ -645,9 +649,6 @@ class LibvirtConnection(driver.ComputeDriver):
                                      metadata,
                                      image_file)
 
-            finally:
-                snapshot_ptr.delete(0)
-
     @exception.wrap_exception()
     def reboot(self, instance, network_info, reboot_type='SOFT'):
         """Reboot a virtual machine, given an instance reference."""

nova/virt/libvirt/utils.py

@@ -24,17 +24,11 @@ import random
 
 from nova import exception
 from nova import flags
-from nova.openstack.common import cfg
 from nova import utils
 from nova.virt import images
 
 
-qemu_img_opt = cfg.StrOpt('qemu_img',
-                          default='qemu-img',
-                          help='binary to use for qemu-img commands')
-
 FLAGS = flags.FLAGS
-FLAGS.register_opt(qemu_img_opt)
 
 
 def execute(*args, **kwargs):
@@ -63,7 +57,7 @@ def create_image(disk_format, path, size):
                  for megabytes, 'g' for gigabytes, 't' for terabytes). If no
                  prefix is given, it will be interpreted as bytes.
     """
-    execute(FLAGS.qemu_img, 'create', '-f', disk_format, path, size)
+    execute('qemu-img', 'create', '-f', disk_format, path, size)
 
 
 def create_cow_image(backing_file, path):
@@ -74,7 +68,7 @@ def create_cow_image(backing_file, path):
     :param backing_file: Existing image on which to base the COW image
     :param path: Desired location of the COW image
     """
-    execute(FLAGS.qemu_img, 'create', '-f', 'qcow2', '-o',
+    execute('qemu-img', 'create', '-f', 'qcow2', '-o',
              'cluster_size=2M,backing_file=%s' % backing_file, path)
 
 
@@ -85,7 +79,7 @@ def get_disk_size(path):
     :returns: Size (in bytes) of the given disk image as it would be seen
               by a virtual machine.
     """
-    out, err = execute(FLAGS.qemu_img, 'info', path)
+    out, err = execute('qemu-img', 'info', path)
     size = [i.split('(')[1].split()[0] for i in out.split('\n')
         if i.strip().find('virtual size') >= 0]
     return int(size[0])
@@ -97,7 +91,7 @@ def get_disk_backing_file(path):
     :param path: Path to the disk image
     :returns: a path to the image's backing store
     """
-    out, err = execute(FLAGS.qemu_img, 'info', path)
+    out, err = execute('qemu-img', 'info', path)
     backing_file = [i.split('actual path:')[1].strip()[:-1]
         for i in out.split('\n') if 0 <= i.find('backing file')]
     if backing_file:
@@ -168,7 +162,37 @@ def chown(path, owner):
     :param path: File or directory whose ownership to change
     :param owner: Desired new owner (given as uid or username)
     """
-    utils.execute('chown', owner, path, run_as_root=True)
+    execute('chown', owner, path, run_as_root=True)
+
+
+def create_snapshot(disk_path, snapshot_name):
+    """Create a snapshot in a disk image
+
+    :param disk_path: Path to disk image
+    :param snapshot_name: Name of snapshot in disk image
+    """
+    qemu_img_cmd = ('qemu-img',
+                    'snapshot',
+                    '-c',
+                    snapshot_name,
+                    disk_path)
+    # NOTE(vish): libvirt changes ownership of images
+    execute(*qemu_img_cmd, run_as_root=True)
+
+
+def delete_snapshot(disk_path, snapshot_name):
+    """Create a snapshot in a disk image
+
+    :param disk_path: Path to disk image
+    :param snapshot_name: Name of snapshot in disk image
+    """
+    qemu_img_cmd = ('qemu-img',
+                    'snapshot',
+                    '-d',
+                    snapshot_name,
+                    disk_path)
+    # NOTE(vish): libvirt changes ownership of images
+    execute(*qemu_img_cmd, run_as_root=True)
 
 
 def extract_snapshot(disk_path, source_fmt, snapshot_name, out_path, dest_fmt):
@@ -178,7 +202,7 @@ def extract_snapshot(disk_path, source_fmt, snapshot_name, out_path, dest_fmt):
     :param snapshot_name: Name of snapshot in disk image
     :param out_path: Desired path of extracted snapshot
     """
-    qemu_img_cmd = (FLAGS.qemu_img,
+    qemu_img_cmd = ('qemu-img',
                     'convert',
                     '-f',
                     source_fmt,