On 03/14/2018 07:19 AM, John Ferlan wrote:
On 03/13/2018 01:26 PM, Jim Fehlig wrote:
libxlDomainMigrationPrepare adds the incoming domain def to the list of
domains via virDomainObjListAdd, which returns a locked and ref counted
virDomainObj. On exit the object is unlocked but not unref'ed. The same
is true for libxlDomainMigrationPrepareTunnel3. Convert both to use the
virDomainObjEndAPI function for cleanup.
Signed-off-by: Jim Fehlig <jfehlig@xxxxxxxx>
---
src/libxl/libxl_migration.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
These two leave me concerned - mainly because as I described in my
review of 2/4 the ListAdd function will return 2 refs and 1 lock on the
object unlike the libxlDomObjFromDomain where there are at least 3 refs
and 1 lock. Since neither of these code paths adds a Ref(vm) after the
ListAdd call (like CreateXML and RestoreFlags do) that means calling
EndAPI will remove 1 ref and the lock leaving only 1 ref on the object.
I'll send a V2 of this patch that adds a ref after ListAdd so the pattern is
consistent throughout the driver.
Later on when ListRemove is called it would enter with 2 refs and 1 lock
and leave with @vm being destroyed. Not having a clear picture of all
the paths a @vm could have in libxl makes me concerned because there are
a few places where ListRemove is called *and* @vm is referenced
afterwards such as libxlDomainDestroyFlags
I think once ListAdd returns with the right number of refs, then yes,
this is the proper adjustment, but for now unless there's an extra Ref
done after the ListAdd, then we should leave things as is.
Thoughts? And does this make sense?
Yes, it makes sense. Thanks again for the details!
Regards,
Jim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
