On Thu, Mar 31, 2022 at 11:08:53AM -0400, Peter Xu wrote: > It's useful for specifying tls credentials all in the cmdline (along with > the -object tls-creds-*), especially for debugging purpose. > > The trick here is we must remember to not free these fields again in the > finalize() function of migration object, otherwise it'll cause double-free. > > The thing is when destroying an object, we'll first destroy the properties > that bound to the object, then the object itself. To be explicit, when > destroy the object in object_finalize() we have such sequence of > operations: > > object_property_del_all(obj); > object_deinit(obj, ti); > > So after this change the two fields are properly released already even > before reaching the finalize() function but in object_property_del_all(), > hence we don't need to free them anymore in finalize() or it's double-free.
I believe this is also fixing a small memory leak > > Signed-off-by: Peter Xu <pet...@redhat.com> > --- > migration/migration.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/migration/migration.c b/migration/migration.c > index 899084f993..1dc80be1f4 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -4349,6 +4349,9 @@ static Property migration_properties[] = { > DEFAULT_MIGRATE_ANNOUNCE_STEP), > DEFINE_PROP_BOOL("x-postcopy-preempt-break-huge", MigrationState, > postcopy_preempt_break_huge, true), > + DEFINE_PROP_STRING("tls-creds", MigrationState, parameters.tls_creds), > + DEFINE_PROP_STRING("tls-hostname", MigrationState, > parameters.tls_hostname), > + DEFINE_PROP_STRING("tls-authz", MigrationState, parameters.tls_authz), > > /* Migration capabilities */ > DEFINE_PROP_MIG_CAP("x-xbzrle", MIGRATION_CAPABILITY_XBZRLE), > @@ -4382,12 +4385,9 @@ static void migration_class_init(ObjectClass *klass, > void *data) > static void migration_instance_finalize(Object *obj) > { > MigrationState *ms = MIGRATION_OBJ(obj); > - MigrationParameters *params = &ms->parameters; > > qemu_mutex_destroy(&ms->error_mutex); > qemu_mutex_destroy(&ms->qemu_file_lock); > - g_free(params->tls_hostname); > - g_free(params->tls_creds); tls_authz wasn't previously freed here, and now it will be > qemu_sem_destroy(&ms->wait_unplug_sem); > qemu_sem_destroy(&ms->rate_limit_sem); > qemu_sem_destroy(&ms->pause_sem); With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|