Peter Xu <[email protected]> writes: > On Mon, Feb 02, 2026 at 07:41:00PM -0300, Fabiano Rosas wrote: >> Expecting every pointer member from s->parameters to be freed >> individually is prone to leave some fields forgotten when the code is >> eventually updated. Use a dealloc visitor to free them all at once. >> >> Signed-off-by: Fabiano Rosas <[email protected]> >> --- >> migration/options.c | 24 +++++++++++++++++------- >> 1 file changed, 17 insertions(+), 7 deletions(-) >> >> diff --git a/migration/options.c b/migration/options.c >> index 733aae51a8..cc5a66750c 100644 >> --- a/migration/options.c >> +++ b/migration/options.c >> @@ -1002,11 +1002,21 @@ AnnounceParameters *migrate_announce_params(void) >> return ≈ >> } >> >> -static void migrate_tls_opts_free(MigrationParameters *params) >> +static bool migrate_params_free(MigrationParameters *params, Error **errp) >> { >> - qapi_free_StrOrNull(params->tls_creds); >> - qapi_free_StrOrNull(params->tls_hostname); >> - qapi_free_StrOrNull(params->tls_authz); >> + Visitor *v = qapi_dealloc_visitor_new(); >> + bool ret; >> + >> + /* >> + * qapi_free_MigrationParameters can't be used here because >> + * MigrationParameters is embedded in MigrationState due to qdev >> + * needing to access the offset of the migration properties inside >> + * the migration object. >> + */ > > Ohhhhhhhh.... thanks for the comment! >
There's some amount of rigidness caused by qdev requirements unfortunately. I'm not sure if I ever posted it, but I wrote some code to move the parameters into a MigrationOptions object so we could make MigrationState not be a TYPE_DEVICE anymore. But then we'd end up with something like -global migration-options by default, so it kinda killed the idea. > Reviewed-by: Peter Xu <[email protected]> > >> + ret = visit_type_MigrationParameters_members(v, params, errp); >> + visit_free(v); >> + >> + return ret; >> }
