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@xxxxxxxxxxxxx>
---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
