qemu: s/virDomainDiskDiffersSourceOnly/qemuDomainDiskChangeSupported/

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@redhat.com>

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6df1618d75fd29afbc56221c19da98daea0c9f79..a3b3ccb0d7ccf3ca3dc95c61b67739b8b5eb8949 100644 (file)
--- 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 f0435545835ebe05743de896d211532566ee0a71..8be390b7811a2b4ab3066e31b86067fe616909f3 100644 (file)
--- 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 5b3da83ff720961cf54c11eb93bfaa360bd23317..1bb41f78115e589b84dea54ef6575b10a2ab8375 100644 (file)
--- 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 bfea18b00957c1b8abd91753cad773f48ea56832..506a43c1016f45b511fbc6d68230a507dcea532d 100644 (file)
--- 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 0b9fd1c14cae76251130854b721ee985fddd15f5..8cf535fbf1ab5351bd140626a2716ad0baa9f7f5 100644 (file)
--- 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 4debaa46d77a73c56b9fe1ff7d8f415551060235..d60745db0a27f03b5a72510531bdfb30b0c0e237 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7924,7 +7924,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 */