[libvirt] [PATCH v2 5/7] qemu: s/virDomainDiskDiffersSourceOnly/qemuDomainDiskChangeSupported/

Michal Privoznik mprivozn at redhat.com
Wed Sep 16 09:15:00 UTC 2015


I always felt like this function is qemu specific rather than
libvirt-wide. Other drivers may act differently on virDomainDef
change and in fact may require talking to underlying hypervisor
even if something else's than disk->src has changed.  I know that
the function is still incomplete, but lets break that into two
commits that are easier to review. This one is pure code
movement.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/conf/domain_conf.c   | 127 -----------------------------------------------
 src/conf/domain_conf.h   |   2 -
 src/libvirt_private.syms |   1 -
 src/qemu/qemu_domain.c   | 127 +++++++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_domain.h   |   3 ++
 src/qemu/qemu_driver.c   |   2 +-
 6 files changed, 131 insertions(+), 131 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6df1618..a3b3ccb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5838,133 +5838,6 @@ virDomainDiskFindByBusAndDst(virDomainDefPtr def,
 }
 
 
-/*
- * Makes sure the @disk differs from @orig_disk only by the source
- * path and nothing else.  Fields that are being checked and the
- * information whether they are nullable (may not be specified) or is
- * taken from the virDomainDiskDefFormat() code.
- */
-bool
-virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk,
-                               virDomainDiskDefPtr orig_disk)
-{
-#define CHECK_EQ(field, field_name, nullable)                           \
-    do {                                                                \
-        if (nullable && !disk->field)                                   \
-            break;                                                      \
-        if (disk->field != orig_disk->field) {                          \
-            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,               \
-                           _("cannot modify field '%s' of the disk"),   \
-                           field_name);                                 \
-            return false;                                               \
-        }                                                               \
-    } while (0)
-
-    CHECK_EQ(device, "device", false);
-    CHECK_EQ(cachemode, "cache", true);
-    CHECK_EQ(error_policy, "error_policy", true);
-    CHECK_EQ(rerror_policy, "rerror_policy", true);
-    CHECK_EQ(iomode, "io", true);
-    CHECK_EQ(ioeventfd, "ioeventfd", true);
-    CHECK_EQ(event_idx, "event_idx", true);
-    CHECK_EQ(copy_on_read, "copy_on_read", true);
-    CHECK_EQ(discard, "discard", true);
-    CHECK_EQ(iothread, "iothread", true);
-
-    if (disk->geometry.cylinders &&
-        disk->geometry.heads &&
-        disk->geometry.sectors) {
-        CHECK_EQ(geometry.cylinders, "geometry cylinders", false);
-        CHECK_EQ(geometry.heads, "geometry heads", false);
-        CHECK_EQ(geometry.sectors, "geometry sectors", false);
-        CHECK_EQ(geometry.trans, "BIOS-translation-modus", true);
-    }
-
-    CHECK_EQ(blockio.logical_block_size,
-             "blockio logical_block_size", false);
-    CHECK_EQ(blockio.physical_block_size,
-             "blockio physical_block_size", false);
-
-    if (disk->bus == VIR_DOMAIN_DISK_BUS_USB)
-        CHECK_EQ(removable, "removable", true);
-
-    CHECK_EQ(blkdeviotune.total_bytes_sec,
-             "blkdeviotune total_bytes_sec",
-             true);
-    CHECK_EQ(blkdeviotune.read_bytes_sec,
-             "blkdeviotune read_bytes_sec",
-             true);
-    CHECK_EQ(blkdeviotune.write_bytes_sec,
-             "blkdeviotune write_bytes_sec",
-             true);
-    CHECK_EQ(blkdeviotune.total_iops_sec,
-             "blkdeviotune total_iops_sec",
-             true);
-    CHECK_EQ(blkdeviotune.read_iops_sec,
-             "blkdeviotune read_iops_sec",
-             true);
-    CHECK_EQ(blkdeviotune.write_iops_sec,
-             "blkdeviotune write_iops_sec",
-             true);
-    CHECK_EQ(blkdeviotune.total_bytes_sec_max,
-             "blkdeviotune total_bytes_sec_max",
-             true);
-    CHECK_EQ(blkdeviotune.read_bytes_sec_max,
-             "blkdeviotune read_bytes_sec_max",
-             true);
-    CHECK_EQ(blkdeviotune.write_bytes_sec_max,
-             "blkdeviotune write_bytes_sec_max",
-             true);
-    CHECK_EQ(blkdeviotune.total_iops_sec_max,
-             "blkdeviotune total_iops_sec_max",
-             true);
-    CHECK_EQ(blkdeviotune.read_iops_sec_max,
-             "blkdeviotune read_iops_sec_max",
-             true);
-    CHECK_EQ(blkdeviotune.write_iops_sec_max,
-             "blkdeviotune write_iops_sec_max",
-             true);
-    CHECK_EQ(blkdeviotune.size_iops_sec,
-             "blkdeviotune size_iops_sec",
-             true);
-
-    CHECK_EQ(transient, "transient", true);
-
-    if (disk->serial && STRNEQ_NULLABLE(disk->serial, orig_disk->serial)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-                       _("cannot modify field '%s' of the disk"),
-                       "serial");
-        return false;
-    }
-
-    if (disk->wwn && STRNEQ_NULLABLE(disk->wwn, orig_disk->wwn)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-                       _("cannot modify field '%s' of the disk"),
-                       "wwn");
-        return false;
-    }
-
-    if (disk->vendor && STRNEQ_NULLABLE(disk->vendor, orig_disk->vendor)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-                       _("cannot modify field '%s' of the disk"),
-                       "vendor");
-        return false;
-    }
-
-    if (disk->product && STRNEQ_NULLABLE(disk->product, orig_disk->product)) {
-        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
-                       _("cannot modify field '%s' of the disk"),
-                       "product");
-        return false;
-    }
-
-    CHECK_EQ(info.bootIndex, "boot order", true);
-
-#undef CHECK_EQ
-
-    return true;
-}
-
 int
 virDomainDiskDefAssignAddress(virDomainXMLOptionPtr xmlopt,
                               virDomainDiskDefPtr def,
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f043554..8be390b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2512,8 +2512,6 @@ int virDomainDeviceFindControllerModel(virDomainDefPtr def,
 virDomainDiskDefPtr virDomainDiskFindByBusAndDst(virDomainDefPtr def,
                                                  int bus,
                                                  char *dst);
-bool virDomainDiskDiffersSourceOnly(virDomainDiskDefPtr disk,
-                                    virDomainDiskDefPtr orig_disk);
 void virDomainControllerDefFree(virDomainControllerDefPtr def);
 void virDomainFSDefFree(virDomainFSDefPtr def);
 void virDomainActualNetDefFree(virDomainActualNetDefPtr def);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 5b3da83..1bb41f7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -251,7 +251,6 @@ virDomainDiskDefFree;
 virDomainDiskDefNew;
 virDomainDiskDefSourceParse;
 virDomainDiskDeviceTypeToString;
-virDomainDiskDiffersSourceOnly;
 virDomainDiskDiscardTypeToString;
 virDomainDiskErrorPolicyTypeFromString;
 virDomainDiskErrorPolicyTypeToString;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 60bd560..21f5002 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3034,6 +3034,133 @@ qemuDomainDiskSourceDiffers(virConnectPtr conn,
 }
 
 
+/*
+ * Makes sure the @disk differs from @orig_disk only by the source
+ * path and nothing else.  Fields that are being checked and the
+ * information whether they are nullable (may not be specified) or is
+ * taken from the virDomainDiskDefFormat() code.
+ */
+bool
+qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
+                              virDomainDiskDefPtr orig_disk)
+{
+#define CHECK_EQ(field, field_name, nullable)                           \
+    do {                                                                \
+        if (nullable && !disk->field)                                   \
+            break;                                                      \
+        if (disk->field != orig_disk->field) {                          \
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,               \
+                           _("cannot modify field '%s' of the disk"),   \
+                           field_name);                                 \
+            return false;                                               \
+        }                                                               \
+    } while (0)
+
+    CHECK_EQ(device, "device", false);
+    CHECK_EQ(cachemode, "cache", true);
+    CHECK_EQ(error_policy, "error_policy", true);
+    CHECK_EQ(rerror_policy, "rerror_policy", true);
+    CHECK_EQ(iomode, "io", true);
+    CHECK_EQ(ioeventfd, "ioeventfd", true);
+    CHECK_EQ(event_idx, "event_idx", true);
+    CHECK_EQ(copy_on_read, "copy_on_read", true);
+    CHECK_EQ(discard, "discard", true);
+    CHECK_EQ(iothread, "iothread", true);
+
+    if (disk->geometry.cylinders &&
+        disk->geometry.heads &&
+        disk->geometry.sectors) {
+        CHECK_EQ(geometry.cylinders, "geometry cylinders", false);
+        CHECK_EQ(geometry.heads, "geometry heads", false);
+        CHECK_EQ(geometry.sectors, "geometry sectors", false);
+        CHECK_EQ(geometry.trans, "BIOS-translation-modus", true);
+    }
+
+    CHECK_EQ(blockio.logical_block_size,
+             "blockio logical_block_size", false);
+    CHECK_EQ(blockio.physical_block_size,
+             "blockio physical_block_size", false);
+
+    if (disk->bus == VIR_DOMAIN_DISK_BUS_USB)
+        CHECK_EQ(removable, "removable", true);
+
+    CHECK_EQ(blkdeviotune.total_bytes_sec,
+             "blkdeviotune total_bytes_sec",
+             true);
+    CHECK_EQ(blkdeviotune.read_bytes_sec,
+             "blkdeviotune read_bytes_sec",
+             true);
+    CHECK_EQ(blkdeviotune.write_bytes_sec,
+             "blkdeviotune write_bytes_sec",
+             true);
+    CHECK_EQ(blkdeviotune.total_iops_sec,
+             "blkdeviotune total_iops_sec",
+             true);
+    CHECK_EQ(blkdeviotune.read_iops_sec,
+             "blkdeviotune read_iops_sec",
+             true);
+    CHECK_EQ(blkdeviotune.write_iops_sec,
+             "blkdeviotune write_iops_sec",
+             true);
+    CHECK_EQ(blkdeviotune.total_bytes_sec_max,
+             "blkdeviotune total_bytes_sec_max",
+             true);
+    CHECK_EQ(blkdeviotune.read_bytes_sec_max,
+             "blkdeviotune read_bytes_sec_max",
+             true);
+    CHECK_EQ(blkdeviotune.write_bytes_sec_max,
+             "blkdeviotune write_bytes_sec_max",
+             true);
+    CHECK_EQ(blkdeviotune.total_iops_sec_max,
+             "blkdeviotune total_iops_sec_max",
+             true);
+    CHECK_EQ(blkdeviotune.read_iops_sec_max,
+             "blkdeviotune read_iops_sec_max",
+             true);
+    CHECK_EQ(blkdeviotune.write_iops_sec_max,
+             "blkdeviotune write_iops_sec_max",
+             true);
+    CHECK_EQ(blkdeviotune.size_iops_sec,
+             "blkdeviotune size_iops_sec",
+             true);
+
+    CHECK_EQ(transient, "transient", true);
+
+    if (disk->serial && STRNEQ_NULLABLE(disk->serial, orig_disk->serial)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       _("cannot modify field '%s' of the disk"),
+                       "serial");
+        return false;
+    }
+
+    if (disk->wwn && STRNEQ_NULLABLE(disk->wwn, orig_disk->wwn)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       _("cannot modify field '%s' of the disk"),
+                       "wwn");
+        return false;
+    }
+
+    if (disk->vendor && STRNEQ_NULLABLE(disk->vendor, orig_disk->vendor)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       _("cannot modify field '%s' of the disk"),
+                       "vendor");
+        return false;
+    }
+
+    if (disk->product && STRNEQ_NULLABLE(disk->product, orig_disk->product)) {
+        virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                       _("cannot modify field '%s' of the disk"),
+                       "product");
+        return false;
+    }
+
+    CHECK_EQ(info.bootIndex, "boot order", true);
+
+#undef CHECK_EQ
+
+    return true;
+}
+
 bool
 qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk)
 {
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 0b9fd1c..8cf535f 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -415,6 +415,9 @@ bool qemuDomainDiskSourceDiffers(virConnectPtr conn,
                                  virDomainDiskDefPtr disk,
                                  virDomainDiskDefPtr origDisk);
 
+bool qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
+                                   virDomainDiskDefPtr orig_disk);
+
 int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
                               virDomainObjPtr vm,
                               virStorageSourcePtr src);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6a8e863..1a189cc 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7942,7 +7942,7 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
     switch ((virDomainDiskDevice) disk->device) {
     case VIR_DOMAIN_DISK_DEVICE_CDROM:
     case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
-        if (!virDomainDiskDiffersSourceOnly(disk, orig_disk))
+        if (!qemuDomainDiskChangeSupported(disk, orig_disk))
             goto cleanup;
 
         /* Add the new disk src into shared disk hash table */
-- 
2.4.6




More information about the libvir-list mailing list