9:04 a.m.
Since -blockdev support was introduced qemuDomainBlockJobAbort was using
the wrong API to terminate the blockjob since we care about sync
finishing semantics.
Additionally for cases where we don't care we can force-finish the jobs
such as when cancelling an migration with NBD disk copy.
Peter Krempa (10):
qemumonitorjsontest: Add test for 'qemuMonitorJSONBlockJobCancel'
qemuMonitorJSONBlockJobCancel: Refactor cleanup
qemu: monitor: Add 'force' argument for 'block-job-cancel' QMP command
qemuDomainBlockJobAbort: Don't use 'job-cancel' instead of
'block-job-cancel'
qemuBackupJobCancelBlockjobs: Replace qemuMonitorJobCancel by
qemuMonitorBlockJobCancel
qemuBlockJobRefreshJobs: Replace qemuMonitorJobCancel by
qemuMonitorBlockJobCancel
qemuMigrationSrcNBDCopyCancel*: Rename 'check' to 'abortMigration'
qemuMigrationSrcNBDCopyCancelOne: Force-cancel disk copy jobs when
aborting migration
qemuMigrationSrcNBDCopyCancelled: Use do-while loop instead of jumping
back
qemu: monitor: Remove qemuMonitorJobCancel
src/qemu/qemu_backup.c | 2 +-
src/qemu/qemu_blockjob.c | 2 +-
src/qemu/qemu_driver.c | 7 +--
src/qemu/qemu_migration.c | 106 ++++++++++++++++++-----------------
src/qemu/qemu_monitor.c | 20 ++-----
src/qemu/qemu_monitor.h | 8 +--
src/qemu/qemu_monitor_json.c | 48 +++-------------
src/qemu/qemu_monitor_json.h | 8 +--
tests/qemumonitorjsontest.c | 4 +-
9 files changed, 75 insertions(+), 130 deletions(-)
--
2.30.2
9:04 a.m.
New subject: [PATCH 01/10] qemumonitorjsontest: Add test for 'qemuMonitorJSONBlockJobCancel'
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
tests/qemumonitorjsontest.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 0866ebe2bf..8ed6509159 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1218,6 +1218,7 @@ GEN_TEST_FUNC(qemuMonitorJSONBitmapRemove, "foodev",
"newnode")
GEN_TEST_FUNC(qemuMonitorJSONJobDismiss, "jobname")
GEN_TEST_FUNC(qemuMonitorJSONJobCancel, "jobname", false)
GEN_TEST_FUNC(qemuMonitorJSONJobComplete, "jobname")
+GEN_TEST_FUNC(qemuMonitorJSONBlockJobCancel, "jobname")
static int
testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque)
@@ -3122,6 +3123,7 @@ mymain(void)
DO_TEST_GEN(qemuMonitorJSONJobDismiss);
DO_TEST_GEN(qemuMonitorJSONJobCancel);
DO_TEST_GEN(qemuMonitorJSONJobComplete);
+ DO_TEST_GEN(qemuMonitorJSONBlockJobCancel);
DO_TEST(qemuMonitorJSONGetBalloonInfo);
DO_TEST(qemuMonitorJSONGetBlockInfo);
DO_TEST(qemuMonitorJSONGetAllBlockStatsInfo);
--
2.30.2
9:04 a.m.
New subject: [PATCH 02/10] qemuMonitorJSONBlockJobCancel: Refactor cleanup
Use automatic memory freeing and remove the cleanup section.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor_json.c | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 652034472a..8f3ccb0c63 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5219,9 +5219,8 @@ int
qemuMonitorJSONBlockJobCancel(qemuMonitor *mon,
const char *jobname)
{
- int ret = -1;
- virJSONValue *cmd = NULL;
- virJSONValue *reply = NULL;
+ g_autoptr(virJSONValue) cmd = NULL;
+ g_autoptr(virJSONValue) reply = NULL;
if (!(cmd = qemuMonitorJSONMakeCommand("block-job-cancel",
"s:device", jobname,
@@ -5229,17 +5228,12 @@ qemuMonitorJSONBlockJobCancel(qemuMonitor *mon,
return -1;
if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
- goto cleanup;
+ return -1;
if (qemuMonitorJSONBlockJobError(cmd, reply, jobname) < 0)
- goto cleanup;
-
- ret = 0;
+ return -1;
- cleanup:
- virJSONValueFree(cmd);
- virJSONValueFree(reply);
- return ret;
+ return 0;
}
--
2.30.2
9:04 a.m.
New subject: [PATCH 03/10] qemu: monitor: Add 'force' argument for 'block-job-cancel' QMP command
In certain cases such as when aborting migration we don't really care
for completion of the blockjob. Add 'force' as parameter of
'block-job-cancel'.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 2 +-
src/qemu/qemu_migration.c | 2 +-
src/qemu/qemu_monitor.c | 7 ++++---
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 4 +++-
src/qemu/qemu_monitor_json.h | 3 ++-
tests/qemumonitorjsontest.c | 2 +-
7 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d908e95ba7..7a9ad03489 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14510,7 +14510,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
if (blockdev)
ret = qemuMonitorJobCancel(priv->mon, job->name, false);
else
- ret = qemuMonitorBlockJobCancel(priv->mon, job->name);
+ ret = qemuMonitorBlockJobCancel(priv->mon, job->name, false);
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
ret = -1;
goto endjob;
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index df88f954ed..b9143166cb 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -732,7 +732,7 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriver *driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
- rv = qemuMonitorBlockJobCancel(priv->mon, job->name);
+ rv = qemuMonitorBlockJobCancel(priv->mon, job->name, false);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
return -1;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f3f14c46b6..fa8a027aa6 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3403,13 +3403,14 @@ qemuMonitorBlockStream(qemuMonitor *mon,
int
qemuMonitorBlockJobCancel(qemuMonitor *mon,
- const char *jobname)
+ const char *jobname,
+ bool force)
{
- VIR_DEBUG("jobname=%s", jobname);
+ VIR_DEBUG("jobname=%s force=%d", jobname, force);
QEMU_CHECK_MONITOR(mon);
- return qemuMonitorJSONBlockJobCancel(mon, jobname);
+ return qemuMonitorJSONBlockJobCancel(mon, jobname, force);
}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 230d00a894..95f1a10e31 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1081,7 +1081,8 @@ int qemuMonitorBlockStream(qemuMonitor *mon,
ATTRIBUTE_NONNULL(2);
int qemuMonitorBlockJobCancel(qemuMonitor *mon,
- const char *jobname)
+ const char *jobname,
+ bool force)
ATTRIBUTE_NONNULL(2);
int qemuMonitorBlockJobSetSpeed(qemuMonitor *mon,
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 8f3ccb0c63..dc74c86158 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5217,13 +5217,15 @@ qemuMonitorJSONBlockStream(qemuMonitor *mon,
int
qemuMonitorJSONBlockJobCancel(qemuMonitor *mon,
- const char *jobname)
+ const char *jobname,
+ bool force)
{
g_autoptr(virJSONValue) cmd = NULL;
g_autoptr(virJSONValue) reply = NULL;
if (!(cmd = qemuMonitorJSONMakeCommand("block-job-cancel",
"s:device", jobname,
+ "B:force", force,
NULL)))
return -1;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 0846325a81..486ba5a593 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -325,7 +325,8 @@ int qemuMonitorJSONBlockStream(qemuMonitor *mon,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int qemuMonitorJSONBlockJobCancel(qemuMonitor *mon,
- const char *jobname)
+ const char *jobname,
+ bool force)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
int
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 8ed6509159..9e53b65289 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1218,7 +1218,7 @@ GEN_TEST_FUNC(qemuMonitorJSONBitmapRemove, "foodev",
"newnode")
GEN_TEST_FUNC(qemuMonitorJSONJobDismiss, "jobname")
GEN_TEST_FUNC(qemuMonitorJSONJobCancel, "jobname", false)
GEN_TEST_FUNC(qemuMonitorJSONJobComplete, "jobname")
-GEN_TEST_FUNC(qemuMonitorJSONBlockJobCancel, "jobname")
+GEN_TEST_FUNC(qemuMonitorJSONBlockJobCancel, "jobname", true)
static int
testQemuMonitorJSONqemuMonitorJSONNBDServerStart(const void *opaque)
--
2.30.2
9:04 a.m.
New subject: [PATCH 04/10] qemuDomainBlockJobAbort: Don't use 'job-cancel' instead of 'block-job-cancel'
'block-job-cancel' has one very important semantic difference to
'job-cancel', docummented in qemu as:
Note that if you issue 'block-job-cancel' after 'drive-mirror' has
indicated
(via the event BLOCK_JOB_READY) that the source and destination are
synchronized, then the event triggered by this command changes to
BLOCK_JOB_COMPLETED, to indicate that the mirroring has ended and the
destination now has a point-in-time copy tied to the time of the cancellation.
Since libvirt advertises the block copy job as having the synchronous
abort feature we must not use 'job-cancel' here.
Fixes: 4817b5ca1d0
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_driver.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7a9ad03489..c90d52edc0 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14461,7 +14461,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
g_autoptr(qemuBlockJobData) job = NULL;
virDomainObj *vm;
qemuDomainObjPrivate *priv = NULL;
- bool blockdev = false;
int ret = -1;
virCheckFlags(VIR_DOMAIN_BLOCK_JOB_ABORT_ASYNC |
@@ -14489,7 +14488,6 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
}
priv = vm->privateData;
- blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV);
if (job->state == QEMU_BLOCKJOB_STATE_ABORTING ||
job->state == QEMU_BLOCKJOB_STATE_PIVOTING) {
@@ -14507,10 +14505,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom,
goto endjob;
} else {
qemuDomainObjEnterMonitor(driver, vm);
- if (blockdev)
- ret = qemuMonitorJobCancel(priv->mon, job->name, false);
- else
- ret = qemuMonitorBlockJobCancel(priv->mon, job->name, false);
+ ret = qemuMonitorBlockJobCancel(priv->mon, job->name, false);
if (qemuDomainObjExitMonitor(driver, vm) < 0) {
ret = -1;
goto endjob;
--
2.30.2
9:04 a.m.
New subject: [PATCH 05/10] qemuBackupJobCancelBlockjobs: Replace qemuMonitorJobCancel by qemuMonitorBlockJobCancel
We want to unify on one block job cancellation API. Use
qemuMonitorBlockJobCancel which has more features.
In case of backup jobs we can cancel the jobs forcefully since the code
is on a cleanup path when the job fails.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_backup.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_backup.c b/src/qemu/qemu_backup.c
index df9ca5b99f..4f1e3b7bad 100644
--- a/src/qemu/qemu_backup.c
+++ b/src/qemu/qemu_backup.c
@@ -676,7 +676,7 @@ qemuBackupJobCancelBlockjobs(virDomainObj *vm,
if (qemuDomainObjEnterMonitorAsync(priv->driver, vm, asyncJob) < 0)
return;
- rc = qemuMonitorJobCancel(priv->mon, job->name, false);
+ rc = qemuMonitorBlockJobCancel(priv->mon, job->name, true);
if (qemuDomainObjExitMonitor(priv->driver, vm) < 0)
return;
--
2.30.2
9:04 a.m.
New subject: [PATCH 06/10] qemuBlockJobRefreshJobs: Replace qemuMonitorJobCancel by qemuMonitorBlockJobCancel
We want to unify on one block job cancellation API. Use
qemuMonitorBlockJobCancel which has more features.
In case of job refresh, we are killing off any unknown jobs so we don't
care about their fate.
Another difference is that an possible error from the block job
cancellation might be reported, but we don't really care here ince
it's a very unlikely scenario and we also report a warning.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_blockjob.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_blockjob.c b/src/qemu/qemu_blockjob.c
index 9ae4500f4d..faf9a9fb7d 100644
--- a/src/qemu/qemu_blockjob.c
+++ b/src/qemu/qemu_blockjob.c
@@ -526,7 +526,7 @@ qemuBlockJobRefreshJobs(virQEMUDriver *driver,
qemuDomainObjEnterMonitor(driver, vm);
- rc = qemuMonitorJobCancel(priv->mon, job->name, true);
+ rc = qemuMonitorBlockJobCancel(priv->mon, job->name, true);
if (rc == -1 && jobinfo[i]->status ==
QEMU_MONITOR_JOB_STATUS_CONCLUDED)
VIR_WARN("can't cancel job '%s' with invalid data",
job->name);
--
2.30.2
9:04 a.m.
New subject: [PATCH 07/10] qemuMigrationSrcNBDCopyCancel*: Rename 'check' to 'abortMigration'
Rename the parameter so that it's more clear what state we are in and
fix all callees.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 31 ++++++++++++++++---------------
1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index b9143166cb..c09f6de153 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -615,9 +615,10 @@ qemuMigrationSrcNBDStorageCopyReady(virDomainObj *vm,
/*
- * If @check is true, the function will report an error and return a different
- * code in case a block job fails. This way we can properly abort migration in
- * case some block jobs failed once all memory has already been transferred.
+ * If @abortMigration is false, the function will report an error and return a
+ * different code in case a block job fails. This way we can properly abort
+ * migration in case some block jobs failed once all memory has already been
+ * transferred.
*
* Returns 1 if all mirrors are gone,
* 0 if some mirrors are still active,
@@ -627,7 +628,7 @@ qemuMigrationSrcNBDStorageCopyReady(virDomainObj *vm,
static int
qemuMigrationSrcNBDCopyCancelled(virDomainObj *vm,
qemuDomainAsyncJob asyncJob,
- bool check)
+ bool abortMigration)
{
size_t i;
size_t active = 0;
@@ -649,7 +650,7 @@ qemuMigrationSrcNBDCopyCancelled(virDomainObj *vm,
qemuBlockJobUpdate(vm, job, asyncJob);
switch (job->state) {
case VIR_DOMAIN_BLOCK_JOB_FAILED:
- if (check) {
+ if (!abortMigration) {
qemuMigrationNBDReportMirrorError(job, disk->dst);
failed = true;
}
@@ -710,7 +711,7 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriver *driver,
virDomainObj *vm,
virDomainDiskDef *disk,
qemuBlockJobData *job,
- bool failNoJob,
+ bool abortMigration,
qemuDomainAsyncJob asyncJob)
{
qemuDomainObjPrivate *priv = vm->privateData;
@@ -720,7 +721,7 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriver *driver,
switch (job->state) {
case VIR_DOMAIN_BLOCK_JOB_FAILED:
case VIR_DOMAIN_BLOCK_JOB_CANCELED:
- if (failNoJob) {
+ if (!abortMigration) {
qemuMigrationNBDReportMirrorError(job, disk->dst);
return -1;
}
@@ -745,7 +746,7 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriver *driver,
* qemuMigrationSrcNBDCopyCancel:
* @driver: qemu driver
* @vm: domain
- * @check: if true report an error when some of the mirrors fails
+ * @abortMigration: The migration is being cancelled.
*
* Cancel all drive-mirrors started by qemuMigrationSrcNBDStorageCopy.
* Any pending block job events for the affected disks will be processed and
@@ -757,7 +758,7 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriver *driver,
static int
qemuMigrationSrcNBDCopyCancel(virQEMUDriver *driver,
virDomainObj *vm,
- bool check,
+ bool abortMigration,
qemuDomainAsyncJob asyncJob,
virConnectPtr dconn)
{
@@ -784,7 +785,7 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriver *driver,
}
rv = qemuMigrationSrcNBDCopyCancelOne(driver, vm, disk, job,
- check, asyncJob);
+ abortMigration, asyncJob);
if (rv != 0) {
if (rv < 0) {
if (!err)
@@ -798,8 +799,8 @@ qemuMigrationSrcNBDCopyCancel(virQEMUDriver *driver,
virObjectUnref(job);
}
- while ((rv = qemuMigrationSrcNBDCopyCancelled(vm, asyncJob, check)) != 1) {
- if (check && !failed &&
+ while ((rv = qemuMigrationSrcNBDCopyCancelled(vm, asyncJob, abortMigration)) != 1) {
+ if (!abortMigration && !failed &&
dconn && virConnectIsAlive(dconn) <= 0) {
virReportError(VIR_ERR_OPERATION_FAILED, "%s",
_("Lost connection to destination host"));
@@ -4272,7 +4273,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver,
}
if (mig->nbd &&
- qemuMigrationSrcNBDCopyCancel(driver, vm, true,
+ qemuMigrationSrcNBDCopyCancel(driver, vm, false,
QEMU_ASYNC_JOB_MIGRATION_OUT,
dconn) < 0)
goto error;
@@ -4350,7 +4351,7 @@ qemuMigrationSrcRun(virQEMUDriver *driver,
/* cancel any outstanding NBD jobs */
if (mig && mig->nbd)
- qemuMigrationSrcNBDCopyCancel(driver, vm, false,
+ qemuMigrationSrcNBDCopyCancel(driver, vm, true,
QEMU_ASYNC_JOB_MIGRATION_OUT,
dconn);
@@ -6054,7 +6055,7 @@ qemuMigrationSrcCancel(virQEMUDriver *driver,
}
if (storage &&
- qemuMigrationSrcNBDCopyCancel(driver, vm, false,
+ qemuMigrationSrcNBDCopyCancel(driver, vm, true,
QEMU_ASYNC_JOB_NONE, NULL) < 0)
return -1;
--
2.30.2
9:04 a.m.
New subject: [PATCH 08/10] qemuMigrationSrcNBDCopyCancelOne: Force-cancel disk copy jobs when aborting migration
We don't require that the data is consistent on the destination if
aborting the migration.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c09f6de153..acbae36964 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -733,7 +733,9 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriver *driver,
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
- rv = qemuMonitorBlockJobCancel(priv->mon, job->name, false);
+ /* when we are aborting the migration we don't care about the data
+ * consistency on the destination so that we can force cancel the mirror */
+ rv = qemuMonitorBlockJobCancel(priv->mon, job->name, abortMigration);
if (qemuDomainObjExitMonitor(driver, vm) < 0 || rv < 0)
return -1;
--
2.30.2
9:04 a.m.
New subject: [PATCH 09/10] qemuMigrationSrcNBDCopyCancelled: Use do-while loop instead of jumping back
Jumping back in the code is an anti-pattern that should be avoided if
possible.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_migration.c | 73 +++++++++++++++++++--------------------
1 file changed, 36 insertions(+), 37 deletions(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index acbae36964..dcecee0ed5 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -635,50 +635,49 @@ qemuMigrationSrcNBDCopyCancelled(virDomainObj *vm,
size_t completed = 0;
bool failed = false;
- retry:
- for (i = 0; i < vm->def->ndisks; i++) {
- virDomainDiskDef *disk = vm->def->disks[i];
- qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
- qemuBlockJobData *job;
+ do {
+ active = 0;
+ completed = 0;
- if (!diskPriv->migrating)
- continue;
+ for (i = 0; i < vm->def->ndisks; i++) {
+ virDomainDiskDef *disk = vm->def->disks[i];
+ qemuDomainDiskPrivate *diskPriv = QEMU_DOMAIN_DISK_PRIVATE(disk);
+ qemuBlockJobData *job;
- if (!(job = qemuBlockJobDiskGetJob(disk)))
- continue;
+ if (!diskPriv->migrating)
+ continue;
- qemuBlockJobUpdate(vm, job, asyncJob);
- switch (job->state) {
- case VIR_DOMAIN_BLOCK_JOB_FAILED:
- if (!abortMigration) {
- qemuMigrationNBDReportMirrorError(job, disk->dst);
- failed = true;
- }
- G_GNUC_FALLTHROUGH;
- case VIR_DOMAIN_BLOCK_JOB_CANCELED:
- case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
- diskPriv->migrating = false;
- break;
+ if (!(job = qemuBlockJobDiskGetJob(disk)))
+ continue;
- default:
- active++;
- }
+ qemuBlockJobUpdate(vm, job, asyncJob);
+ switch (job->state) {
+ case VIR_DOMAIN_BLOCK_JOB_FAILED:
+ if (!abortMigration) {
+ qemuMigrationNBDReportMirrorError(job, disk->dst);
+ failed = true;
+ }
+ G_GNUC_FALLTHROUGH;
+ case VIR_DOMAIN_BLOCK_JOB_CANCELED:
+ case VIR_DOMAIN_BLOCK_JOB_COMPLETED:
+ diskPriv->migrating = false;
+ break;
- if (job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
- completed++;
+ default:
+ active++;
+ }
- virObjectUnref(job);
- }
+ if (job->state == VIR_DOMAIN_BLOCK_JOB_COMPLETED)
+ completed++;
- /* Updating completed block job drops the lock thus we have to recheck
- * block jobs for disks that reside before the disk(s) with completed
- * block job.
- */
- if (completed > 0) {
- completed = 0;
- active = 0;
- goto retry;
- }
+ virObjectUnref(job);
+ }
+
+ /* Updating completed block job drops the lock thus we have to recheck
+ * block jobs for disks that reside before the disk(s) with completed
+ * block job.
+ */
+ } while (completed > 0);
if (failed) {
if (active) {
--
2.30.2
9:04 a.m.
New subject: [PATCH 10/10] qemu: monitor: Remove qemuMonitorJobCancel
The API is unused since last commit. Remove it.
Signed-off-by: Peter Krempa <pkrempa(a)redhat.com>
---
src/qemu/qemu_monitor.c | 13 -------------
src/qemu/qemu_monitor.h | 5 -----
src/qemu/qemu_monitor_json.c | 28 ----------------------------
src/qemu/qemu_monitor_json.h | 5 -----
tests/qemumonitorjsontest.c | 2 --
5 files changed, 53 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index fa8a027aa6..23161a0088 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3448,19 +3448,6 @@ qemuMonitorJobDismiss(qemuMonitor *mon,
}
-int
-qemuMonitorJobCancel(qemuMonitor *mon,
- const char *jobname,
- bool quiet)
-{
- VIR_DEBUG("jobname='%s' quiet=%d", jobname, quiet);
-
- QEMU_CHECK_MONITOR(mon);
-
- return qemuMonitorJSONJobCancel(mon, jobname, quiet);
-}
-
-
int
qemuMonitorJobComplete(qemuMonitor *mon,
const char *jobname)
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 95f1a10e31..420a85942a 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -1106,11 +1106,6 @@ int qemuMonitorJobDismiss(qemuMonitor *mon,
const char *jobname)
ATTRIBUTE_NONNULL(2);
-int qemuMonitorJobCancel(qemuMonitor *mon,
- const char *jobname,
- bool quiet)
- ATTRIBUTE_NONNULL(2);
-
int qemuMonitorJobComplete(qemuMonitor *mon,
const char *jobname)
ATTRIBUTE_NONNULL(2);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index dc74c86158..8de0e4b638 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5319,34 +5319,6 @@ qemuMonitorJSONJobDismiss(qemuMonitor *mon,
}
-int
-qemuMonitorJSONJobCancel(qemuMonitor *mon,
- const char *jobname,
- bool quiet)
-{
- g_autoptr(virJSONValue) cmd = NULL;
- g_autoptr(virJSONValue) reply = NULL;
-
- if (!(cmd = qemuMonitorJSONMakeCommand("job-cancel",
- "s:id", jobname,
- NULL)))
- return -1;
-
- if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0)
- return -1;
-
- if (quiet) {
- if (virJSONValueObjectHasKey(reply, "error") != 0)
- return -1;
- } else {
- if (qemuMonitorJSONBlockJobError(cmd, reply, jobname) < 0)
- return -1;
- }
-
- return 0;
-}
-
-
int
qemuMonitorJSONJobComplete(qemuMonitor *mon,
const char *jobname)
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 486ba5a593..8d8f2de479 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -348,11 +348,6 @@ int qemuMonitorJSONJobDismiss(qemuMonitor *mon,
const char *jobname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-int qemuMonitorJSONJobCancel(qemuMonitor *mon,
- const char *jobname,
- bool quiet)
- ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
-
int qemuMonitorJSONJobComplete(qemuMonitor *mon,
const char *jobname)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 9e53b65289..e36036e0d2 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1216,7 +1216,6 @@ GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumRemove,
"foodev")
GEN_TEST_FUNC(qemuMonitorJSONBlockdevMediumInsert, "foodev",
"newnode")
GEN_TEST_FUNC(qemuMonitorJSONBitmapRemove, "foodev", "newnode")
GEN_TEST_FUNC(qemuMonitorJSONJobDismiss, "jobname")
-GEN_TEST_FUNC(qemuMonitorJSONJobCancel, "jobname", false)
GEN_TEST_FUNC(qemuMonitorJSONJobComplete, "jobname")
GEN_TEST_FUNC(qemuMonitorJSONBlockJobCancel, "jobname", true)
@@ -3121,7 +3120,6 @@ mymain(void)
DO_TEST_GEN(qemuMonitorJSONBlockdevMediumInsert);
DO_TEST_GEN(qemuMonitorJSONBitmapRemove);
DO_TEST_GEN(qemuMonitorJSONJobDismiss);
- DO_TEST_GEN(qemuMonitorJSONJobCancel);
DO_TEST_GEN(qemuMonitorJSONJobComplete);
DO_TEST_GEN(qemuMonitorJSONBlockJobCancel);
DO_TEST(qemuMonitorJSONGetBalloonInfo);
--
2.30.2
10:34 a.m.
On 4/21/21 4:04 PM, Peter Krempa wrote:
Since -blockdev support was introduced qemuDomainBlockJobAbort was
using
the wrong API to terminate the blockjob since we care about sync
finishing semantics.
Additionally for cases where we don't care we can force-finish the jobs
such as when cancelling an migration with NBD disk copy.
Peter Krempa (10):
qemumonitorjsontest: Add test for 'qemuMonitorJSONBlockJobCancel'
qemuMonitorJSONBlockJobCancel: Refactor cleanup
qemu: monitor: Add 'force' argument for 'block-job-cancel' QMP
command
qemuDomainBlockJobAbort: Don't use 'job-cancel' instead of
'block-job-cancel'
qemuBackupJobCancelBlockjobs: Replace qemuMonitorJobCancel by
qemuMonitorBlockJobCancel
qemuBlockJobRefreshJobs: Replace qemuMonitorJobCancel by
qemuMonitorBlockJobCancel
qemuMigrationSrcNBDCopyCancel*: Rename 'check' to 'abortMigration'
qemuMigrationSrcNBDCopyCancelOne: Force-cancel disk copy jobs when
aborting migration
qemuMigrationSrcNBDCopyCancelled: Use do-while loop instead of jumping
back
qemu: monitor: Remove qemuMonitorJobCancel
src/qemu/qemu_backup.c | 2 +-
src/qemu/qemu_blockjob.c | 2 +-
src/qemu/qemu_driver.c | 7 +--
src/qemu/qemu_migration.c | 106 ++++++++++++++++++-----------------
src/qemu/qemu_monitor.c | 20 ++-----
src/qemu/qemu_monitor.h | 8 +--
src/qemu/qemu_monitor_json.c | 48 +++-------------
src/qemu/qemu_monitor_json.h | 8 +--
tests/qemumonitorjsontest.c | 4 +-
9 files changed, 75 insertions(+), 130 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn(a)redhat.com>
Michal
1445
days inactive
1445
days old
11 comments
2 participants
participants (2)
-
Michal Privoznik -
Peter Krempa