During API call connection is referenced and close callback is called when
connection is disposed. Thus during API call close callback cannot be triggered
and we don't need to unset callback on API call duration.

This code was added in [1] and commit does not explain this part of the
patch.

[1] 1fdc53c3: qemu: Avoid dangling migration-out job when client dies

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
---
 src/qemu/qemu_migration.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 301475a..7615dab 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4743,7 +4743,6 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver,
  */
 static int
 qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
-                             virConnectPtr conn,
                              virDomainObjPtr vm,
                              const char *persist_xml,
                              const char *uri,
@@ -4772,8 +4771,6 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
     }
 
     qemuMigrationJobStartPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3);
-    virCloseCallbacksUnset(driver->closeCallbacks, vm,
-                           qemuMigrationSrcCleanup);
 
     ret = qemuMigrationSrcPerformNative(driver, vm, persist_xml, uri, cookiein, cookieinlen,
                                         cookieout, cookieoutlen,
@@ -4787,10 +4784,6 @@ qemuMigrationSrcPerformPhase(virQEMUDriverPtr driver,
 
     qemuMigrationJobSetPhase(driver, vm, QEMU_MIGRATION_PHASE_PERFORM3_DONE);
 
-    if (virCloseCallbacksSet(driver->closeCallbacks, vm, conn,
-                             qemuMigrationSrcCleanup) < 0)
-        goto endjob;
-
  endjob:
     if (ret < 0) {
         qemuMigrationParamsReset(driver, vm, QEMU_ASYNC_JOB_MIGRATION_OUT,
@@ -4862,7 +4855,7 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver,
         }
 
         if (v3proto) {
-            return qemuMigrationSrcPerformPhase(driver, conn, vm, persist_xml, uri,
+            return qemuMigrationSrcPerformPhase(driver, vm, persist_xml, uri,
                                                 graphicsuri,
                                                 nmigrate_disks, migrate_disks,
                                                 migParams,
-- 
1.8.3.1

On Tue, Aug 18, 2020 at 13:26:36 +0300, Nikolay Shirokovskiy wrote:
> During API call connection is referenced and close callback is called when
> connection is disposed. Thus during API call close callback cannot be triggered
> and we don't need to unset callback on API call duration.
> 
> This code was added in [1] and commit does not explain this part of the
> patch.
> 
> [1] 1fdc53c3: qemu: Avoid dangling migration-out job when client dies

I suspect this was not the case 8 years ago when close callbacks were
introduced. In any case, I don't think calling Unset in the begining of
qemuMigrationSrcPerformPhase and Set at the end could cause any harm.
The idea is to keep the callback set only when there's no running API.
So while it may not be needed, I don't think there's a real need to
change it.

Jirka

On 06.11.2020 17:03, Jiri Denemark wrote:
> On Tue, Aug 18, 2020 at 13:26:36 +0300, Nikolay Shirokovskiy wrote:
>> During API call connection is referenced and close callback is called when
>> connection is disposed. Thus during API call close callback cannot be triggered
>> and we don't need to unset callback on API call duration.
>>
>> This code was added in [1] and commit does not explain this part of the
>> patch.
>>
>> [1] 1fdc53c3: qemu: Avoid dangling migration-out job when client dies
> 
> I suspect this was not the case 8 years ago when close callbacks were
> introduced. In any case, I don't think calling Unset in the begining of
> qemuMigrationSrcPerformPhase and Set at the end could cause any harm.
> The idea is to keep the callback set only when there's no running API.
> So while it may not be needed, I don't think there's a real need to
> change it.
> 
> Jirka
> 

Ok, thank you for review! First patch pushed with suggested changes.

Nikolay