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@xxxxxxxxxx>
---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list