[PATCH 00/10] qemu: Fix block job cancelling
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
Signed-off-by: Peter Krempa <pkrempa@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
Use automatic memory freeing and remove the cleanup section. Signed-off-by: Peter Krempa <pkrempa@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
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@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
'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@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
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@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
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@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
Rename the parameter so that it's more clear what state we are in and fix all callees. Signed-off-by: Peter Krempa <pkrempa@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
We don't require that the data is consistent on the destination if aborting the migration. Signed-off-by: Peter Krempa <pkrempa@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
Jumping back in the code is an anti-pattern that should be avoided if possible. Signed-off-by: Peter Krempa <pkrempa@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
The API is unused since last commit. Remove it. Signed-off-by: Peter Krempa <pkrempa@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
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@redhat.com> Michal
participants (2)
-
Michal Privoznik -
Peter Krempa