Caught by inspection, but ASAN also reports:

Direct leak of 16 byte(s) in 1 object(s) allocated from:
 #0 in malloc
 #1 in g_malloc
 #2 in g_memdup
 #3 in qapi_clone_start_struct ../qapi/qapi-clone-visitor.c:40:12
 #4 in qapi_clone_start_list ../qapi/qapi-clone-visitor.c:59:12
 #5 in visit_start_list ../qapi/qapi-visit-core.c:80:10
 #6 in visit_type_BitmapMigrationNodeAliasList qapi/qapi-visit-migration.c:639:10
 #7 in migrate_params_apply ../migration/options.c:1407:13
 #8 in qmp_migrate_set_parameters ../migration/options.c:1463:5
 #9 in qmp_marshal_migrate_set_parameters qapi/qapi-commands-migration.c:214:5
 #10 in do_qmp_dispatch_bh ../qapi/qmp-dispatch.c:128:5

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/migration.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/migration.c b/migration/migration.c
index 4098870bce..7ec60d97f9 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -4050,6 +4050,7 @@ static void migration_instance_finalize(Object *obj)
 {
     MigrationState *ms = MIGRATION_OBJ(obj);
 
+    qapi_free_BitmapMigrationNodeAliasList(ms->parameters.block_bitmap_mapping);
     qemu_mutex_destroy(&ms->error_mutex);
     qemu_mutex_destroy(&ms->qemu_file_lock);
     qemu_sem_destroy(&ms->wait_unplug_sem);
-- 
2.35.3

Fabiano Rosas <farosas@suse.de> writes:

> Caught by inspection, but ASAN also reports:
>
> Direct leak of 16 byte(s) in 1 object(s) allocated from:
>  #0 in malloc
>  #1 in g_malloc
>  #2 in g_memdup
>  #3 in qapi_clone_start_struct ../qapi/qapi-clone-visitor.c:40:12
>  #4 in qapi_clone_start_list ../qapi/qapi-clone-visitor.c:59:12
>  #5 in visit_start_list ../qapi/qapi-visit-core.c:80:10
>  #6 in visit_type_BitmapMigrationNodeAliasList qapi/qapi-visit-migration.c:639:10
>  #7 in migrate_params_apply ../migration/options.c:1407:13
>  #8 in qmp_migrate_set_parameters ../migration/options.c:1463:5
>  #9 in qmp_marshal_migrate_set_parameters qapi/qapi-commands-migration.c:214:5
>  #10 in do_qmp_dispatch_bh ../qapi/qmp-dispatch.c:128:5

migration_instance_finalize() runs when a TYPE_MIGRATION object dies, we
have just one such object, pointed to by @current_migration, and it
lives until QEMU shuts down.

So this is as harmless as they get.  Please mentions this in the commit
message, to guide backporters.

> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/migration.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 4098870bce..7ec60d97f9 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -4050,6 +4050,7 @@ static void migration_instance_finalize(Object *obj)
>  {
>      MigrationState *ms = MIGRATION_OBJ(obj);
>  
> +    qapi_free_BitmapMigrationNodeAliasList(ms->parameters.block_bitmap_mapping);
>      qemu_mutex_destroy(&ms->error_mutex);
>      qemu_mutex_destroy(&ms->qemu_file_lock);
>      qemu_sem_destroy(&ms->wait_unplug_sem);

With an adjusted commit message:
Reviewed-by: Markus Armbruster <armbru@redhat.com>

On Tue, Jul 01, 2025 at 08:12:27AM +0200, Markus Armbruster wrote:
> Fabiano Rosas <farosas@suse.de> writes:
> 
> > Caught by inspection, but ASAN also reports:
> >
> > Direct leak of 16 byte(s) in 1 object(s) allocated from:
> >  #0 in malloc
> >  #1 in g_malloc
> >  #2 in g_memdup
> >  #3 in qapi_clone_start_struct ../qapi/qapi-clone-visitor.c:40:12
> >  #4 in qapi_clone_start_list ../qapi/qapi-clone-visitor.c:59:12
> >  #5 in visit_start_list ../qapi/qapi-visit-core.c:80:10
> >  #6 in visit_type_BitmapMigrationNodeAliasList qapi/qapi-visit-migration.c:639:10
> >  #7 in migrate_params_apply ../migration/options.c:1407:13
> >  #8 in qmp_migrate_set_parameters ../migration/options.c:1463:5
> >  #9 in qmp_marshal_migrate_set_parameters qapi/qapi-commands-migration.c:214:5
> >  #10 in do_qmp_dispatch_bh ../qapi/qmp-dispatch.c:128:5
> 
> migration_instance_finalize() runs when a TYPE_MIGRATION object dies, we
> have just one such object, pointed to by @current_migration, and it
> lives until QEMU shuts down.
> 
> So this is as harmless as they get.  Please mentions this in the commit
> message, to guide backporters.

If we do not copy qemu-stable, and do not attach Fixes, logically it should
imply no backport needed.  Not sure if it was intentional, though..  Agreed
some enrichment in the log would always be nicer.

> 
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> > ---
> >  migration/migration.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index 4098870bce..7ec60d97f9 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -4050,6 +4050,7 @@ static void migration_instance_finalize(Object *obj)
> >  {
> >      MigrationState *ms = MIGRATION_OBJ(obj);
> >  
> > +    qapi_free_BitmapMigrationNodeAliasList(ms->parameters.block_bitmap_mapping);
> >      qemu_mutex_destroy(&ms->error_mutex);
> >      qemu_mutex_destroy(&ms->qemu_file_lock);
> >      qemu_sem_destroy(&ms->wait_unplug_sem);
> 
> With an adjusted commit message:
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Peter Xu <peterx@redhat.com> writes:

> On Tue, Jul 01, 2025 at 08:12:27AM +0200, Markus Armbruster wrote:
>> Fabiano Rosas <farosas@suse.de> writes:
>> 
>> > Caught by inspection, but ASAN also reports:
>> >
>> > Direct leak of 16 byte(s) in 1 object(s) allocated from:
>> >  #0 in malloc
>> >  #1 in g_malloc
>> >  #2 in g_memdup
>> >  #3 in qapi_clone_start_struct ../qapi/qapi-clone-visitor.c:40:12
>> >  #4 in qapi_clone_start_list ../qapi/qapi-clone-visitor.c:59:12
>> >  #5 in visit_start_list ../qapi/qapi-visit-core.c:80:10
>> >  #6 in visit_type_BitmapMigrationNodeAliasList qapi/qapi-visit-migration.c:639:10
>> >  #7 in migrate_params_apply ../migration/options.c:1407:13
>> >  #8 in qmp_migrate_set_parameters ../migration/options.c:1463:5
>> >  #9 in qmp_marshal_migrate_set_parameters qapi/qapi-commands-migration.c:214:5
>> >  #10 in do_qmp_dispatch_bh ../qapi/qmp-dispatch.c:128:5
>> 
>> migration_instance_finalize() runs when a TYPE_MIGRATION object dies, we
>> have just one such object, pointed to by @current_migration, and it
>> lives until QEMU shuts down.
>> 
>> So this is as harmless as they get.  Please mentions this in the commit
>> message, to guide backporters.
>
> If we do not copy qemu-stable, and do not attach Fixes, logically it should
> imply no backport needed.  Not sure if it was intentional, though..

Yes, qemu-stable@ and Fixes: are how we indicate "consider backporting
this".  But since that's easily forgotten, absence doesn't imply "no
need to consider".

>                                                                      Agreed
> some enrichment in the log would always be nicer.

Spelling out the impact of the bug fixes is a good habit.  Or in this
case, the fact that it's not a bug.  No biggie here, just nice.  I like
nice commit messages :)

[...]