4:14 a.m.
So, as Peter pointed out, we may want to updated startupPolicy for
live domains too. That's what patches 2/7-7/7 do.
Michal Privoznik (7):
qemuDomainUpdateDeviceConfig: Allow startupPolicy update, yet again
qemu: s/qemuDomainChangeDiskMediaLive/qemuDomainChangeDiskLive/
qemu_domain: Introduce qemuDomainDiskSourceDiffers
qemuDomainChangeDiskLive: rework slightly
qemu: s/virDomainDiskDiffersSourceOnly/qemuDomainDiskChangeSupported/
qemuDomainDiskChangeSupported: Fill in missing checks
qemuDomainChangeDiskLive: Allow startupPolicy change
src/conf/domain_conf.c | 127 ----------------------------------
src/conf/domain_conf.h | 2 -
src/libvirt_private.syms | 1 -
src/qemu/qemu_domain.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 7 ++
src/qemu/qemu_driver.c | 80 ++++++++++++---------
6 files changed, 232 insertions(+), 161 deletions(-)
--
2.4.6
4:14 a.m.
New subject: [libvirt] [PATCH v2 1/7] qemuDomainUpdateDeviceConfig: Allow startupPolicy update, yet again
https://bugzilla.redhat.com/show_bug.cgi?id=1159219
So, in 11e058ca589808bd I've tried to make UpdateDevice update
startupPolicy too. And it worked well until somebody came around
and pushed d0dc6c036914da which accidentally removed my
contribution. Redo my commit.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fcf86b6..fadead5 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8356,6 +8356,7 @@ qemuDomainUpdateDeviceConfig(virQEMUCapsPtr qemuCaps,
* We allow updating src/type//driverType/cachemode/
*/
orig->cachemode = disk->cachemode;
+ orig->startupPolicy = disk->startupPolicy;
virStorageSourceFree(orig->src);
orig->src = disk->src;
--
2.4.6
4:14 a.m.
New subject: [libvirt] [PATCH v2 2/7] qemu: s/qemuDomainChangeDiskMediaLive/qemuDomainChangeDiskLive/
While we currently only allow changing a media in a disk, this is
going to change in a while, so the function name would be
invalid. Moreover, the old name does not match the pattern laid
out by other update functions.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index fadead5..8946b20 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7914,11 +7914,11 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
}
static int
-qemuDomainChangeDiskMediaLive(virConnectPtr conn,
- virDomainObjPtr vm,
- virDomainDeviceDefPtr dev,
- virQEMUDriverPtr driver,
- bool force)
+qemuDomainChangeDiskLive(virConnectPtr conn,
+ virDomainObjPtr vm,
+ virDomainDeviceDefPtr dev,
+ virQEMUDriverPtr driver,
+ bool force)
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
@@ -7983,7 +7983,7 @@ qemuDomainUpdateDeviceLive(virConnectPtr conn,
switch ((virDomainDeviceType) dev->type) {
case VIR_DOMAIN_DEVICE_DISK:
qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1);
- ret = qemuDomainChangeDiskMediaLive(conn, vm, dev, driver, force);
+ ret = qemuDomainChangeDiskLive(conn, vm, dev, driver, force);
break;
case VIR_DOMAIN_DEVICE_GRAPHICS:
ret = qemuDomainChangeGraphics(driver, vm, dev->data.graphics);
--
2.4.6
4:14 a.m.
New subject: [libvirt] [PATCH v2 3/7] qemu_domain: Introduce qemuDomainDiskSourceDiffers
This new private API should return true iff sources of two disks
differs in sense that qemu should be instructed to change the
disk backend. For instance, ejecting a CDROM is such case, or
pointing disk into a different ISO location, and so on.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 33 +++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 4 ++++
2 files changed, 37 insertions(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index c8b0ccd..60bd560 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3002,6 +3002,39 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
bool
+qemuDomainDiskSourceDiffers(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virDomainDiskDefPtr origDisk)
+{
+ char *diskSrc = NULL, *origDiskSrc = NULL;
+ bool diskEmpty, origDiskEmpty;
+ bool ret = true;
+
+ diskEmpty = virStorageSourceIsEmpty(disk->src);
+ origDiskEmpty = virStorageSourceIsEmpty(origDisk->src);
+
+ if (diskEmpty && origDiskEmpty)
+ return false;
+
+ if (diskEmpty ^ origDiskEmpty)
+ return true;
+
+ if (qemuGetDriveSourceString(disk->src, conn, &diskSrc) < 0 ||
+ qemuGetDriveSourceString(origDisk->src, conn, &origDiskSrc) < 0)
+ goto cleanup;
+
+ /* So far in qemu disk sources are considered different
+ * if either path to disk or its format changes. */
+ ret = virDomainDiskGetFormat(disk) != virDomainDiskGetFormat(origDisk) ||
+ STRNEQ_NULLABLE(diskSrc, origDiskSrc);
+ cleanup:
+ VIR_FREE(diskSrc);
+ VIR_FREE(origDiskSrc);
+ return ret;
+}
+
+
+bool
qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk)
{
qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 91eaea1..0b9fd1c 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -411,6 +411,10 @@ int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
bool force_probe,
bool report_broken);
+bool qemuDomainDiskSourceDiffers(virConnectPtr conn,
+ virDomainDiskDefPtr disk,
+ virDomainDiskDefPtr origDisk);
+
int qemuDomainStorageFileInit(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virStorageSourcePtr src);
--
2.4.6
4:14 a.m.
New subject: [libvirt] [PATCH v2 4/7] qemuDomainChangeDiskLive: rework slightly
Firstly, our coding guidelines suggest using 'cleanup' label
instead of 'end'. Then, @ret should be set to value representing
success as the last statement before the 'cleanup' label.
And while I am at this function, lets enumerate all the possible
enum items (virDomainDiskDevice) and avoid using 'default' in
switch(). Pooh. Also, nothing bad happens if we look up the disk
to change in the domain upfront. In fact, it's going to be
helpful later when we want to keep some old values for performing
a rollback.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_driver.c | 48 +++++++++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 21 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8946b20..6a8e863 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7925,48 +7925,54 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
int ret = -1;
if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
- goto end;
+ goto cleanup;
if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0)
- goto end;
+ goto cleanup;
- switch (disk->device) {
+ if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
+ disk->bus, disk->dst))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("No device with bus '%s' and target
'%s'"),
+ virDomainDiskBusTypeToString(disk->bus),
+ disk->dst);
+ goto cleanup;
+ }
+
+ switch ((virDomainDiskDevice) disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
- if (!(orig_disk = virDomainDiskFindByBusAndDst(vm->def,
- disk->bus, disk->dst))) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("No device with bus '%s' and target
'%s'"),
- virDomainDiskBusTypeToString(disk->bus),
- disk->dst);
- goto end;
- }
-
if (!virDomainDiskDiffersSourceOnly(disk, orig_disk))
- goto end;
+ goto cleanup;
/* Add the new disk src into shared disk hash table */
if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
- goto end;
+ goto cleanup;
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;
+ orig_disk, disk->src, force) < 0) {
+ ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
+ goto cleanup;
}
- dev->data.disk->src = NULL;
- ret = 0;
+ disk->src = NULL;
break;
- default:
+ case VIR_DOMAIN_DISK_DEVICE_DISK:
+ case VIR_DOMAIN_DISK_DEVICE_LUN:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk bus '%s' cannot be updated."),
virDomainDiskBusTypeToString(disk->bus));
+ goto cleanup;
+ break;
+
+ case VIR_DOMAIN_DISK_DEVICE_LAST:
+ /* nada */
break;
}
- end:
+ ret = 0;
+ cleanup:
return ret;
}
--
2.4.6
4:15 a.m.
New subject: [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>
---
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
4:15 a.m.
New subject: [libvirt] [PATCH v2 6/7] qemuDomainDiskChangeSupported: Fill in missing checks
So far this function was not kept in sync with changing
virDomainDiskDef. Fill in all the missing checks and reorganize
their order so it's easier to track which items are not being
checked for.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 45 +++++++++++++++++++++++++++++++--------------
1 file changed, 31 insertions(+), 14 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 21f5002..ed92d8a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3057,15 +3057,15 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
} 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);
+ CHECK_EQ(bus, "bus", false);
+ if (STRNEQ(disk->dst, orig_disk->dst)) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("cannot modify field '%s' of the disk"),
+ "bus");
+ return false;
+ }
+ CHECK_EQ(tray_status, "tray", true);
+ CHECK_EQ(removable, "removable", true);
if (disk->geometry.cylinders &&
disk->geometry.heads &&
@@ -3081,9 +3081,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
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);
@@ -3124,8 +3121,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
"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"),
@@ -3154,7 +3149,29 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
return 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(snapshot, "snapshot", true);
+ CHECK_EQ(startupPolicy, "startupPolicy", true);
+ CHECK_EQ(transient, "transient", true);
CHECK_EQ(info.bootIndex, "boot order", true);
+ CHECK_EQ(rawio, "rawio", true);
+ CHECK_EQ(sgio, "sgio", true);
+ CHECK_EQ(discard, "discard", true);
+ CHECK_EQ(iothread, "iothread", true);
+
+ if (disk->domain_name &&
+ STRNEQ_NULLABLE(disk->domain_name, orig_disk->domain_name)) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+ _("cannot modify field '%s' of the disk"),
+ "backenddomain");
+ return false;
+ }
#undef CHECK_EQ
--
2.4.6
4:15 a.m.
New subject: [libvirt] [PATCH v2 7/7] qemuDomainChangeDiskLive: Allow startupPolicy change
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 1 -
src/qemu/qemu_driver.c | 29 ++++++++++++++++++++---------
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed92d8a..fb8ab30 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3157,7 +3157,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
CHECK_EQ(event_idx, "event_idx", true);
CHECK_EQ(copy_on_read, "copy_on_read", true);
CHECK_EQ(snapshot, "snapshot", true);
- CHECK_EQ(startupPolicy, "startupPolicy", true);
CHECK_EQ(transient, "transient", true);
CHECK_EQ(info.bootIndex, "boot order", true);
CHECK_EQ(rawio, "rawio", true);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1a189cc..d74255b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7922,6 +7922,7 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
+ int startupPolicy;
int ret = -1;
if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
@@ -7939,23 +7940,29 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
goto cleanup;
}
+ startupPolicy = orig_disk->startupPolicy;
+
switch ((virDomainDiskDevice) disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
if (!qemuDomainDiskChangeSupported(disk, orig_disk))
goto cleanup;
- /* Add the new disk src into shared disk hash table */
- if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
- goto cleanup;
+ orig_disk->startupPolicy = dev->data.disk->startupPolicy;
- if (qemuDomainChangeEjectableMedia(driver, conn, vm,
- orig_disk, disk->src, force) < 0) {
- ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- goto cleanup;
+ if (qemuDomainDiskSourceDiffers(conn, disk, orig_disk)) {
+ /* Add the new disk src into shared disk hash table */
+ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
+ goto cleanup;
+
+ if (qemuDomainChangeEjectableMedia(driver, conn, vm,
+ orig_disk, dev->data.disk->src,
force) < 0) {
+ ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
vm->def->name));
+ goto rollback;
+ }
+
+ dev->data.disk->src = NULL;
}
-
- disk->src = NULL;
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
@@ -7974,6 +7981,10 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
ret = 0;
cleanup:
return ret;
+
+ rollback:
+ orig_disk->startupPolicy = startupPolicy;
+ goto cleanup;
}
static int
--
2.4.6
3:47 p.m.
New subject: [libvirt] [PATCH v2 7/7] qemuDomainChangeDiskLive: Allow startupPolicy change
On 09/16/2015 05:15 AM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_domain.c | 1 -
src/qemu/qemu_driver.c | 29 ++++++++++++++++++++---------
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed92d8a..fb8ab30 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3157,7 +3157,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
CHECK_EQ(event_idx, "event_idx", true);
CHECK_EQ(copy_on_read, "copy_on_read", true);
CHECK_EQ(snapshot, "snapshot", true);
- CHECK_EQ(startupPolicy, "startupPolicy", true);
Perhaps leave a comment indicating that startupPolicy is special so that
someone doesn't come along one day and say - oh look startupPolicy is
missing, I'm going to add it here... Assuming one reads comments ;-)
It's really too bad there isn't some "simple mechanism" to ensure new
fields are listed here too.
CHECK_EQ(transient, "transient", true);
CHECK_EQ(info.bootIndex, "boot order", true);
CHECK_EQ(rawio, "rawio", true);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1a189cc..d74255b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7922,6 +7922,7 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
{
virDomainDiskDefPtr disk = dev->data.disk;
virDomainDiskDefPtr orig_disk = NULL;
+ int startupPolicy;
int ret = -1;
if (virStorageTranslateDiskSourcePool(conn, disk) < 0)
@@ -7939,23 +7940,29 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
goto cleanup;
}
+ startupPolicy = orig_disk->startupPolicy;
+
switch ((virDomainDiskDevice) disk->device) {
case VIR_DOMAIN_DISK_DEVICE_CDROM:
case VIR_DOMAIN_DISK_DEVICE_FLOPPY:
if (!qemuDomainDiskChangeSupported(disk, orig_disk))
goto cleanup;
- /* Add the new disk src into shared disk hash table */
- if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
- goto cleanup;
+ orig_disk->startupPolicy = dev->data.disk->startupPolicy;
- if (qemuDomainChangeEjectableMedia(driver, conn, vm,
- orig_disk, disk->src, force) < 0) {
- ignore_value(qemuRemoveSharedDisk(driver, disk, vm->def->name));
- goto cleanup;
+ if (qemuDomainDiskSourceDiffers(conn, disk, orig_disk)) {
+ /* Add the new disk src into shared disk hash table */
+ if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0)
+ goto cleanup;
+
+ if (qemuDomainChangeEjectableMedia(driver, conn, vm,
+ orig_disk, dev->data.disk->src,
force) < 0) {
+ ignore_value(qemuRemoveSharedDisk(driver, dev->data.disk,
vm->def->name));
+ goto rollback;
+ }
+
+ dev->data.disk->src = NULL;
}
-
- disk->src = NULL;
break;
case VIR_DOMAIN_DISK_DEVICE_DISK:
@@ -7974,6 +7981,10 @@ qemuDomainChangeDiskLive(virConnectPtr conn,
ret = 0;
cleanup:
return ret;
+
+ rollback:
+ orig_disk->startupPolicy = startupPolicy;
+ goto cleanup;
}
static int
12:26 a.m.
New subject: [libvirt] [PATCH v2 7/7] qemuDomainChangeDiskLive: Allow startupPolicy change
On 18.09.2015 22:47, John Ferlan wrote:
On 09/16/2015 05:15 AM, Michal Privoznik wrote:
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_domain.c | 1 -
> src/qemu/qemu_driver.c | 29 ++++++++++++++++++++---------
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index ed92d8a..fb8ab30 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -3157,7 +3157,6 @@ qemuDomainDiskChangeSupported(virDomainDiskDefPtr disk,
> CHECK_EQ(event_idx, "event_idx", true);
> CHECK_EQ(copy_on_read, "copy_on_read", true);
> CHECK_EQ(snapshot, "snapshot", true);
> - CHECK_EQ(startupPolicy, "startupPolicy", true);
Perhaps leave a comment indicating that startupPolicy is special so that
someone doesn't come along one day and say - oh look startupPolicy is
missing, I'm going to add it here... Assuming one reads comments ;-)
Good point. Added.
It's really too bad there isn't some "simple mechanism" to ensure new
fields are listed here too.
Yeah. Perhaps some autogenerated function. But that would mean parsing C
code, which alone explodes straight away.
Michal
3:48 p.m.
On 09/16/2015 05:14 AM, Michal Privoznik wrote:
So, as Peter pointed out, we may want to updated startupPolicy for
live domains too. That's what patches 2/7-7/7 do.
Michal Privoznik (7):
qemuDomainUpdateDeviceConfig: Allow startupPolicy update, yet again
qemu: s/qemuDomainChangeDiskMediaLive/qemuDomainChangeDiskLive/
qemu_domain: Introduce qemuDomainDiskSourceDiffers
qemuDomainChangeDiskLive: rework slightly
qemu: s/virDomainDiskDiffersSourceOnly/qemuDomainDiskChangeSupported/
qemuDomainDiskChangeSupported: Fill in missing checks
qemuDomainChangeDiskLive: Allow startupPolicy change
src/conf/domain_conf.c | 127 ----------------------------------
src/conf/domain_conf.h | 2 -
src/libvirt_private.syms | 1 -
src/qemu/qemu_domain.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 7 ++
src/qemu/qemu_driver.c | 80 ++++++++++++---------
6 files changed, 232 insertions(+), 161 deletions(-)
ACK series (although there is a minor comment in 7/7)
John
3351
days inactive
3356
days old
10 comments
2 participants
participants (2)
-
John Ferlan -
Michal Privoznik