[libvirt] [PATCH v2 5/7] 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(a)redhat.com&gt; --- 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