On 4/1/19 12:03 PM, Nikolay Shirokovskiy wrote:

qemuMigrationSrcPerform callers expect it to call virDomainObjEndAPI in any case.

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

diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 419a729..ce4c443 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -4799,6 +4799,7 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, if (cookieinlen) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("received unexpected cookie with P2P migration")); + virDomainObjEndAPI(&vm); return -1; }

@@ -4813,6 +4814,7 @@ qemuMigrationSrcPerform(virQEMUDriverPtr driver, if (dconnuri) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Unexpected dconnuri parameter with non-peer2peer migration")); + virDomainObjEndAPI(&vm); return -1; }

Nice catch! Although for code cleanliness I don't think that these calls belong here. They belong into callers where @vm is obtained (e.g. qemuDomainMigratePerform3()). Then we can also remove virDomainObjEndAPI() calls from other functions called from the one you're fixing (e.g. the call can be removed from qemuMigrationSrcPerformJob(), qemuMigrationSrcPerformPhase() and qemuMigrationSrcPerformJob()). The reason that free is not done on the same level as alloc (or EndAPI() is not done at the same level as ObjFromDomain()) is the reason we have a bug like this in the first place. In my mind it should allways be like this: foo() { vm = qemuDomainObjFromDomain(dom); ... bar(vm); ... virDomainObjEndAPI(&vm); } and if bar() needs a reference, it can do virObjectRef(vm) + virObjectUnref(vm). Michal