Merge lp:~vishvananda/nova/lp824433 into lp:~hudson-openstack/nova/milestone-proposed

[Vish Ishaya]

Fixes the handling of snapshotting in libvirt driver to actually use the proper image type instead of using raw for everything. Also cleans up an unneeded flag. Based on doude's initial work.

=== modified file 'nova/tests/test_libvirt.py'
--- nova/tests/test_libvirt.py	2011-09-16 17:36:43 +0000
+++ nova/tests/test_libvirt.py	2011-09-19 20:04:46 +0000
@@ -340,39 +340,77 @@
         instance_data = dict(self.test_instance)
         self._check_xml_and_container(instance_data)
 
-    def test_snapshot(self):
-        if not self.lazy_load_library_exists():
-            return
-
-        self.flags(image_service='nova.image.fake.FakeImageService')
-
-        # Start test
-        image_service = utils.import_object(FLAGS.image_service)
-
-        # Assuming that base image already exists in image_service
-        instance_ref = db.instance_create(self.context, self.test_instance)
-        properties = {'instance_id': instance_ref['id'],
-                      'user_id': str(self.context.user_id)}
-        snapshot_name = 'test-snap'
-        sent_meta = {'name': snapshot_name, 'is_public': False,
-                     'status': 'creating', 'properties': properties}
-        # Create new image. It will be updated in snapshot method
-        # To work with it from snapshot, the single image_service is needed
-        recv_meta = image_service.create(context, sent_meta)
-
-        self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn')
-        connection.LibvirtConnection._conn.lookupByName = self.fake_lookup
-        self.mox.StubOutWithMock(connection.utils, 'execute')
-        connection.utils.execute = self.fake_execute
-
-        self.mox.ReplayAll()
-
-        conn = connection.LibvirtConnection(False)
-        conn.snapshot(self.context, instance_ref, recv_meta['id'])
-
-        snapshot = image_service.show(context, recv_meta['id'])
-        self.assertEquals(snapshot['properties']['image_state'], 'available')
-        self.assertEquals(snapshot['status'], 'active')
+    def test_snapshot_in_raw_format(self):
+        if not self.lazy_load_library_exists():
+            return
+
+        self.flags(image_service='nova.image.fake.FakeImageService')
+
+        # Start test
+        image_service = utils.import_object(FLAGS.image_service)
+
+        # Assuming that base image already exists in image_service
+        instance_ref = db.instance_create(self.context, self.test_instance)
+        properties = {'instance_id': instance_ref['id'],
+                      'user_id': str(self.context.user_id)}
+        snapshot_name = 'test-snap'
+        sent_meta = {'name': snapshot_name, 'is_public': False,
+                     'status': 'creating', 'properties': properties}
+        # Create new image. It will be updated in snapshot method
+        # To work with it from snapshot, the single image_service is needed
+        recv_meta = image_service.create(context, sent_meta)
+
+        self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn')
+        connection.LibvirtConnection._conn.lookupByName = self.fake_lookup
+        self.mox.StubOutWithMock(connection.utils, 'execute')
+        connection.utils.execute = self.fake_execute
+
+        self.mox.ReplayAll()
+
+        conn = connection.LibvirtConnection(False)
+        conn.snapshot(self.context, instance_ref, recv_meta['id'])
+
+        snapshot = image_service.show(context, recv_meta['id'])
+        self.assertEquals(snapshot['properties']['image_state'], 'available')
+        self.assertEquals(snapshot['status'], 'active')
+        self.assertEquals(snapshot['disk_format'], 'raw')
+        self.assertEquals(snapshot['name'], snapshot_name)
+
+    def test_snapshot_in_qcow2_format(self):
+        if not self.lazy_load_library_exists():
+            return
+
+        self.flags(image_service='nova.image.fake.FakeImageService')
+        self.flags(snapshot_image_format='qcow2')
+
+        # Start test
+        image_service = utils.import_object(FLAGS.image_service)
+
+        # Assuming that base image already exists in image_service
+        instance_ref = db.instance_create(self.context, self.test_instance)
+        properties = {'instance_id': instance_ref['id'],
+                      'user_id': str(self.context.user_id)}
+        snapshot_name = 'test-snap'
+        sent_meta = {'name': snapshot_name, 'is_public': False,
+                     'status': 'creating', 'properties': properties}
+        # Create new image. It will be updated in snapshot method
+        # To work with it from snapshot, the single image_service is needed
+        recv_meta = image_service.create(context, sent_meta)
+
+        self.mox.StubOutWithMock(connection.LibvirtConnection, '_conn')
+        connection.LibvirtConnection._conn.lookupByName = self.fake_lookup
+        self.mox.StubOutWithMock(connection.utils, 'execute')
+        connection.utils.execute = self.fake_execute
+
+        self.mox.ReplayAll()
+
+        conn = connection.LibvirtConnection(False)
+        conn.snapshot(self.context, instance_ref, recv_meta['id'])
+
+        snapshot = image_service.show(context, recv_meta['id'])
+        self.assertEquals(snapshot['properties']['image_state'], 'available')
+        self.assertEquals(snapshot['status'], 'active')
+        self.assertEquals(snapshot['disk_format'], 'qcow2')
         self.assertEquals(snapshot['name'], snapshot_name)
 
     def test_snapshot_no_image_architecture(self):

=== modified file 'nova/virt/libvirt/connection.py'
--- nova/virt/libvirt/connection.py	2011-09-18 19:24:46 +0000
+++ nova/virt/libvirt/connection.py	2011-09-19 20:04:46 +0000
@@ -125,8 +125,10 @@
                     'Define block migration behavior.')
 flags.DEFINE_integer('live_migration_bandwidth', 0,
                     'Define live migration behavior')
-flags.DEFINE_string('qemu_img', 'qemu-img',
-                    'binary to use for qemu-img commands')
+flags.DEFINE_string('snapshot_image_format', None,
+                    'Snapshot image format (valid options are : '
+                    'raw, qcow2, vmdk, vdi).'
+                    'Defaults to same as source image')
 flags.DEFINE_string('libvirt_vif_type', 'bridge',
                     'Type of VIF to create.')
 flags.DEFINE_string('libvirt_vif_driver',
@@ -391,10 +393,7 @@
     def snapshot(self, context, instance, image_href):
         """Create snapshot from a running VM instance.
 
-        This command only works with qemu 0.14+, the qemu_img flag is
-        provided so that a locally compiled binary of qemu-img can be used
-        to support this command.
-
+        This command only works with qemu 0.14+
         """
         virt_dom = self._lookup_by_name(instance['name'])
 
@@ -420,8 +419,11 @@
             arch = base['properties']['architecture']
             metadata['properties']['architecture'] = arch
 
-        if 'disk_format' in base:
-            metadata['disk_format'] = base['disk_format']
+        source_format = base.get('disk_format') or 'raw'
+        image_format = FLAGS.snapshot_image_format or source_format
+        if FLAGS.use_cow_images:
+            source_format = 'qcow2'
+        metadata['disk_format'] = image_format
 
         if 'container_format' in base:
             metadata['container_format'] = base['container_format']
@@ -444,12 +446,12 @@
         # Export the snapshot to a raw image
         temp_dir = tempfile.mkdtemp()
         out_path = os.path.join(temp_dir, snapshot_name)
-        qemu_img_cmd = (FLAGS.qemu_img,
+        qemu_img_cmd = ('qemu-img',
                         'convert',
                         '-f',
-                        'qcow2',
+                        source_format,
                         '-O',
-                        'raw',
+                        image_format,
                         '-s',
                         snapshot_name,
                         disk_path,