On Thu, Sep 01, 2022 at 14:47:40 +0200, Jiri Denemark wrote:
We will need a little bit more code around qemuMonitorMigrateCancel to make sure it works as expected. The new qemuMigrationSrcCancel helper will avoid repeating the code in several places.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 9 +-------- src/qemu/qemu_migration.c | 37 +++++++++++++++++++++++-------------- src/qemu/qemu_migration.h | 4 ++++ src/qemu/qemu_process.c | 5 +---- 4 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 707f4cc1bb..a86efc769a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12808,17 +12808,10 @@ qemuDomainGetJobStats(virDomainPtr dom, static int qemuDomainAbortJobMigration(virDomainObj *vm) { - qemuDomainObjPrivate *priv = vm->privateData; - int ret; - VIR_DEBUG("Cancelling migration job at client request");
qemuDomainObjAbortAsyncJob(vm); - qemuDomainObjEnterMonitor(vm); - ret = qemuMonitorMigrateCancel(priv->mon);
So this caller cared about the return value of 'qemuMonitorMigrateCancel' ...
- qemuDomainObjExitMonitor(vm); - - return ret; + return qemuMigrationSrcCancel(vm, VIR_ASYNC_JOB_NONE); }
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 67d83ca743..5845dfdb9c 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4611,6 +4611,24 @@ qemuMigrationSrcStart(virDomainObj *vm, }
+int +qemuMigrationSrcCancel(virDomainObj *vm, + virDomainAsyncJob asyncJob) +{ + qemuDomainObjPrivate *priv = vm->privateData; + + VIR_DEBUG("Cancelling outgoing migration of domain %s", vm->def->name); + + if (qemuDomainObjEnterMonitorAsync(vm, asyncJob) < 0) + return -1; + + qemuMonitorMigrateCancel(priv->mon);
... but here you don't propagate it out. Instead the only possibility for this function to fail is from 'qemuDomainObjEnterMonitorAsync', but any caller that pases non-NONE asyncjob doesn't care about the return value at all.
+ qemuDomainObjExitMonitor(vm); + + return 0; +} + +
Also consider adding a comment too. With the above bug fixed: Reviewed-by: Peter Krempa <pkrempa@redhat.com>