[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 13/14] qemu: hotplug: Sanitize shared device removal on media change



Instead of tediously copying of the disk source to remove it later
ensure that the media change function removes the old device after it
succeeds.
---
 src/qemu/qemu_conf.c    |  2 +-
 src/qemu/qemu_conf.h    |  5 ++++
 src/qemu/qemu_driver.c  | 38 +++++++-----------------------
 src/qemu/qemu_hotplug.c | 62 +++++++++++++++++++++++--------------------------
 4 files changed, 43 insertions(+), 64 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 00405cc..6b4a497 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1097,7 +1097,7 @@ qemuAddSharedDevice(virQEMUDriverPtr driver,
 }


-static int
+int
 qemuRemoveSharedDisk(virQEMUDriverPtr driver,
                      virDomainDiskDefPtr disk,
                      const char *name)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index 90aebef..2f60b6a 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -302,6 +302,11 @@ int qemuRemoveSharedDevice(virQEMUDriverPtr driver,
                            const char *name)
     ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);

+int qemuRemoveSharedDisk(virQEMUDriverPtr driver,
+                         virDomainDiskDefPtr disk,
+                         const char *name)
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
+
 int qemuSetUnprivSGIO(virDomainDeviceDefPtr dev);

 int qemuDriverAllocateID(virQEMUDriverPtr driver);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7088f19..4b1c69f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6621,9 +6621,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
 {
     virDomainDiskDefPtr disk = dev->data.disk;
     virDomainDiskDefPtr orig_disk = NULL;
-    virDomainDiskDefPtr tmp = NULL;
-    virDomainDeviceDefPtr dev_copy = NULL;
-    virStorageSourcePtr newsrc;
     virCapsPtr caps = NULL;
     int ret = -1;

@@ -6648,38 +6645,20 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,
         if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
             goto end;

-        tmp = dev->data.disk;
-        dev->data.disk = orig_disk;
-
-        if (!(dev_copy = virDomainDeviceDefCopy(dev, vm->def,
-                                                caps, driver->xmlopt))) {
-            dev->data.disk = tmp;
-            goto end;
-        }
-        dev->data.disk = tmp;
-
         /* Add the new disk src into shared disk hash table */
         if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
             goto end;

-        newsrc = disk->src;
-        disk->src = NULL;
-
-        ret = qemuDomainChangeEjectableMedia(driver, conn, vm,
-                                             orig_disk, newsrc, force);
-        /* 'disk' must not be accessed now - it has been freed.
-         * 'orig_disk' now points to the new disk, while 'dev_copy'
-         * now points to the old disk */
-
-        /* Need to remove the shared disk entry for the original
-         * disk src if the operation is either ejecting or updating.
-         */
-        if (ret == 0) {
-            dev->data.disk = NULL;
-            ignore_value(qemuRemoveSharedDevice(driver, dev_copy,
-                                                vm->def->name));
+        if (qemuDomainChangeEjectableMedia(driver, conn, vm,
+                                           orig_disk, dev->data.disk->src, force) < 0) {
+            ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk, vm->def->name));
+            goto end;
         }
+
+        dev->data.disk->src = NULL;
+        ret = 0;
         break;
+
     default:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("disk bus '%s' cannot be updated."),
@@ -6689,7 +6668,6 @@ qemuDomainChangeDiskMediaLive(virConnectPtr conn,

  end:
     virObjectUnref(caps);
-    virDomainDeviceDefFree(dev_copy);
     return ret;
 }

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 39907ab..4c69c5e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -137,12 +137,29 @@ qemuDomainPrepareDisk(virQEMUDriverPtr driver,
 }


-int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
-                                   virConnectPtr conn,
-                                   virDomainObjPtr vm,
-                                   virDomainDiskDefPtr disk,
-                                   virStorageSourcePtr newsrc,
-                                   bool force)
+/**
+ * qemuDomainChangeEjectableMedia:
+ * @driver: qemu driver structure
+ * @conn: connection structure
+ * @vm: domain definition
+ * @disk: disk definition to change the source of
+ * @newsrc: new disk source to change to
+ * @force: force the change of media
+ *
+ * Change the media in an ejectable device to the one described by
+ * @newsrc. This function also removes the old source from the
+ * shared device table if appropriate. Note that newsrc is consumed
+ * on success and the old source is freed on success.
+ *
+ * Returns 0 on success, -1 on error and reports libvirt error
+ */
+int
+qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
+                               virConnectPtr conn,
+                               virDomainObjPtr vm,
+                               virDomainDiskDefPtr disk,
+                               virStorageSourcePtr newsrc,
+                               bool force)
 {
     int ret = -1;
     char *driveAlias = NULL;
@@ -224,6 +241,8 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
     if (ret < 0)
         goto error;

+    /* remove the old source from shared device list */
+    ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
     ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));

     virStorageSourceFree(disk->src);
@@ -231,7 +250,6 @@ int qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
     newsrc = NULL;

  cleanup:
-    virStorageSourceFree(newsrc);
     VIR_FREE(driveAlias);
     VIR_FREE(sourcestr);
     return ret;
@@ -739,9 +757,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
 {
     virDomainDiskDefPtr disk = dev->data.disk;
     virDomainDiskDefPtr orig_disk = NULL;
-    virDomainDeviceDefPtr dev_copy = NULL;
-    virStorageSourcePtr newsrc;
-    virDomainDiskDefPtr tmp = NULL;
     virCapsPtr caps = NULL;
     int ret = -1;
     const char *driverName = virDomainDiskGetDriver(disk);
@@ -783,32 +798,14 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
         if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
             goto end;

-
-        tmp = dev->data.disk;
-        dev->data.disk = orig_disk;
-
-        if (!(dev_copy = virDomainDeviceDefCopy(dev, vm->def,
-                                                caps, driver->xmlopt))) {
-            dev->data.disk = tmp;
+        if (qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk,
+                                           disk->src, false) < 0)
             goto end;
-        }
-        dev->data.disk = tmp;

-        newsrc = disk->src;
         disk->src = NULL;
-
-        ret = qemuDomainChangeEjectableMedia(driver, conn, vm, orig_disk, newsrc, false);
-        /* 'newsrc' must not be accessed now - it has been free'd.
-         * 'orig_disk' now points to the new disk, while 'dev_copy'
-         * now points to the old disk */
-
-        /* Need to remove the shared disk entry for the original disk src
-         * if the operation is either ejecting or updating.
-         */
-        if (ret == 0)
-            ignore_value(qemuRemoveSharedDevice(driver, dev_copy,
-                                                vm->def->name));
+        ret = 0;
         break;
+
     case VIR_DOMAIN_DISK_DEVICE_DISK:
     case VIR_DOMAIN_DISK_DEVICE_LUN:
         if (disk->bus == VIR_DOMAIN_DISK_BUS_USB) {
@@ -840,7 +837,6 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn,
     if (ret != 0)
         ignore_value(qemuRemoveSharedDevice(driver, dev, vm->def->name));
     virObjectUnref(caps);
-    virDomainDeviceDefFree(dev_copy);
     return ret;
 }

-- 
2.0.2


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]