Block job QMP commands with underscores rather than dashes were never
released in upstream qemu, (they were added, but modified in the same
release [1]), but a certain distro managed to backport the version in the
middle.
The change also slightly modified semantics for the abort command, which
made us have a lot of code which was only ever present in certain
downstream distros.
Clean the upstream code from the legacy cruft and support only the
upstream implementations.
[1] See qemu commit v1.0-2176-gdb58f9c060
---
src/qemu/qemu_domain.c | 13 +++--------
src/qemu/qemu_domain.h | 2 +-
src/qemu/qemu_driver.c | 60 +++++++++++++-------------------------------------
3 files changed, 19 insertions(+), 56 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 664366b9d..05f8e9488 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -6434,29 +6434,22 @@ qemuDomainGetMonitor(virDomainObjPtr vm)
/**
* qemuDomainSupportsBlockJobs:
* @vm: domain object
- * @modern: pointer to bool that returns whether modern block jobs are supported
*
* Returns -1 in case when qemu does not support block jobs at all. Otherwise
- * returns 0 and optionally fills @modern to denote that modern (async) block
- * jobs are supported.
+ * returns 0.
*/
int
-qemuDomainSupportsBlockJobs(virDomainObjPtr vm,
- bool *modern)
+qemuDomainSupportsBlockJobs(virDomainObjPtr vm)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
bool asynchronous = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC);
- bool synchronous = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKJOB_SYNC);
- if (!synchronous && !asynchronous) {
+ if (!asynchronous) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("block jobs not supported with this QEMU binary"));
return -1;
}
- if (modern)
- *modern = asynchronous;
-
return 0;
}
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 468308e4b..b291dc308 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -715,7 +715,7 @@ int qemuDomainJobInfoToParams(qemuDomainJobInfoPtr jobInfo,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4);
-int qemuDomainSupportsBlockJobs(virDomainObjPtr vm, bool *modern)
+int qemuDomainSupportsBlockJobs(virDomainObjPtr vm)
ATTRIBUTE_NONNULL(1);
bool qemuDomainDiskBlockJobIsActive(virDomainDiskDefPtr disk);
bool qemuDomainHasBlockjob(virDomainObjPtr vm, bool copy_only)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e95683965..c7d93dcb2 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16413,7 +16413,6 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
{
qemuDomainObjPrivatePtr priv = vm->privateData;
char *device = NULL;
- bool modern;
virDomainDiskDefPtr disk;
virStorageSourcePtr baseSource = NULL;
unsigned int baseIndex = 0;
@@ -16438,25 +16437,9 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
goto endjob;
}
- if (qemuDomainSupportsBlockJobs(vm, &modern) < 0)
+ if (qemuDomainSupportsBlockJobs(vm) < 0)
goto endjob;
- if (!modern) {
- if (base) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("partial block pull not supported with this "
- "QEMU binary"));
- goto endjob;
- }
-
- if (bandwidth) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("setting bandwidth at start of block pull not "
- "supported with this QEMU binary"));
- goto endjob;
- }
- }
-
if (!(disk = qemuDomainDiskByName(vm->def, path)))
goto endjob;
@@ -16511,7 +16494,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver,
baseSource);
if (!baseSource || basePath)
ret = qemuMonitorBlockStream(priv->mon, device, basePath, backingPath,
- speed, modern);
+ speed, true);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
ret = -1;
@@ -16542,7 +16525,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
virDomainDiskDefPtr disk = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
bool save = false;
- bool modern;
bool pivot = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT);
bool async = !!(flags & VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC);
virDomainObjPtr vm;
@@ -16566,7 +16548,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
goto endjob;
}
- if (qemuDomainSupportsBlockJobs(vm, &modern) < 0)
+ if (qemuDomainSupportsBlockJobs(vm) < 0)
goto endjob;
if (!(disk = qemuDomainDiskByName(vm->def, path)))
@@ -16583,7 +16565,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
goto endjob;
}
- if (modern && !async)
+ if (!async)
qemuBlockJobSyncBegin(disk);
if (pivot) {
@@ -16596,7 +16578,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
}
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), device, modern);
+ ret = qemuMonitorBlockJobCancel(qemuDomainGetMonitor(vm), device, true);
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
ret = -1;
goto endjob;
@@ -16623,25 +16605,14 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
* while still holding the VM job, to prevent newly scheduled
* block jobs from confusing us. */
if (!async) {
- if (!modern) {
- /* Older qemu that lacked async reporting also lacked
- * blockcopy and active commit, so we can hardcode the
- * event to pull and let qemuBlockJobEventProcess() handle
- * the rest as usual */
- qemuBlockJobEventProcess(driver, vm, disk,
- QEMU_ASYNC_JOB_NONE,
- VIR_DOMAIN_BLOCK_JOB_TYPE_PULL,
- VIR_DOMAIN_BLOCK_JOB_CANCELED);
- } else {
- qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
- qemuBlockJobUpdate(driver, vm, QEMU_ASYNC_JOB_NONE, disk);
- while (diskPriv->blockjob) {
- if (virDomainObjWait(vm) < 0) {
- ret = -1;
- goto endjob;
- }
- qemuBlockJobUpdate(driver, vm, QEMU_ASYNC_JOB_NONE, disk);
+ qemuDomainDiskPrivatePtr diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+ qemuBlockJobUpdate(driver, vm, QEMU_ASYNC_JOB_NONE, disk);
+ while (diskPriv->blockjob) {
+ if (virDomainObjWait(vm) < 0) {
+ ret = -1;
+ goto endjob;
}
+ qemuBlockJobUpdate(driver, vm, QEMU_ASYNC_JOB_NONE, disk);
}
}
@@ -16728,7 +16699,7 @@ qemuDomainGetBlockJobInfo(virDomainPtr dom,
goto endjob;
}
- if (qemuDomainSupportsBlockJobs(vm, NULL) < 0)
+ if (qemuDomainSupportsBlockJobs(vm) < 0)
goto endjob;
if (!(disk = virDomainDiskByName(vm->def, path, true))) {
@@ -16784,7 +16755,6 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom,
virDomainDiskDefPtr disk;
int ret = -1;
virDomainObjPtr vm;
- bool modern;
const char *device;
unsigned long long speed = bandwidth;
@@ -16816,7 +16786,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom,
goto endjob;
}
- if (qemuDomainSupportsBlockJobs(vm, &modern) < 0)
+ if (qemuDomainSupportsBlockJobs(vm) < 0)
goto endjob;
if (!(disk = qemuDomainDiskByName(vm->def, path)))
@@ -16829,7 +16799,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom,
ret = qemuMonitorBlockJobSetSpeed(qemuDomainGetMonitor(vm),
device,
speed,
- modern);
+ true);
if (qemuDomainObjExitMonitor(driver, vm) < 0)
ret = -1;
--
2.14.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 09/13/2017 08:52 AM, Peter Krempa wrote: > Block job QMP commands with underscores rather than dashes were never > released in upstream qemu, (they were added, but modified in the same > release [1]), but a certain distro managed to backport the version in the > middle. I find it fine that you don't call out the "certain distro" by name in the commit message - but several readers can probably already guess, so I'm not spilling the beans further if I clear my throat a bit "<cough>RHEL 6</cough>". Let's visit the consequences of this patch: If downstream Red Hat wants to rebase to a version of libvirt containing this patch, that downstream version will HAVE to use the modern spelling only - but presumably that rebase will only be done in tandem with a new enough qemu that also supplies the modern spelling. Meanwhile, when it comes to supporting loooong lifecycles of older releases, downstream has already picked the version of libvirt they intend to maintain forever, and will NOT be rebasing either libvirt or qemu, so they won't be missing anything dropped in this patch because they won't be using this patch. So I welcome this cleanup. > > The change also slightly modified semantics for the abort command, which > made us have a lot of code which was only ever present in certain > downstream distros. > > Clean the upstream code from the legacy cruft and support only the > upstream implementations. > > [1] See qemu commit v1.0-2176-gdb58f9c060 > --- > src/qemu/qemu_domain.c | 13 +++-------- > src/qemu/qemu_domain.h | 2 +- > src/qemu/qemu_driver.c | 60 +++++++++++++------------------------------------- > 3 files changed, 19 insertions(+), 56 deletions(-) > > @@ -16511,7 +16494,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, > baseSource); > if (!baseSource || basePath) > ret = qemuMonitorBlockStream(priv->mon, device, basePath, backingPath, > - speed, modern); > + speed, true); Presumably a later patch cleans this up further? Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.