[PATCH 0/3] A few leak fixes for QTest

[PATCH 0/3] A few leak fixes for QTest

[Fabiano Rosas]

CI run: https://gitlab.com/farosas/qemu/-/pipelines/2818383684

Fabiano Rosas (3):
  ahci: Fix leak of IRQState
  migration: Drop iochannel reference during snapshot setup
  migration: Free the JSON writer during snapshots

 hw/ide/ahci.c         |  1 +
 migration/migration.c |  2 +-
 migration/migration.h |  1 +
 migration/savevm.c    | 11 ++++++++---
 4 files changed, 11 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH 1/3] ahci: Fix leak of IRQState

[Fabiano Rosas]

ASAN spotted an indirect leak in ahci-test:

Indirect leak of 576 byte(s) in 6 object(s) allocated from:
    #0 0x556112a08ca0 in malloc
    #1 0x7f21591e595d in g_malloc
    #2 0x7f21591ffe38 in g_slice_alloc
    #3 0x7f21591c988d in g_hash_table_new_full
    #4 0x556114d756f1 in object_initialize_with_type ../qom/object.c:506:23
    #5 0x556114d77590 in object_new_with_type ../qom/object.c:706:5
    #6 0x556114d777e8 in object_new ../qom/object.c:722:12
    #7 0x556114d6d2be in qemu_allocate_irq ../hw/core/irq.c:94:25
    #8 0x556114d6d1bc in qemu_extend_irqs ../hw/core/irq.c:82:16
    #9 0x556114d6d329 in qemu_allocate_irqs ../hw/core/irq.c:89:12
    #10 0x5561135da9bd in ahci_realize ../hw/ide/ahci.c:1644:12
    #11 0x55611360156a in pci_ich9_ahci_realize ../hw/ide/ich.c:132:5

Although ahci_realize() frees the return of qemu_allocate_irqs() right
away, the individual IRQStates are still left around.

Issue qemu_free_irq() at ahci_uninit().

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

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 6b04762c4a..86f6297dc9 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1688,6 +1688,7 @@ void ahci_uninit(AHCIState *s)
             }
             ide_exit(ide_state);
         }
+        qemu_free_irq(ad->port.irq);
         object_unparent(OBJECT(&ad->port));
     }
 
-- 
2.53.0

Re: [PATCH 1/3] ahci: Fix leak of IRQState

[Denis V. Lunev]

On 9/4/26 00:05, Fabiano Rosas wrote:
> ASAN spotted an indirect leak in ahci-test:
>
> Indirect leak of 576 byte(s) in 6 object(s) allocated from:
>     #0 0x556112a08ca0 in malloc
>     #1 0x7f21591e595d in g_malloc
>     #2 0x7f21591ffe38 in g_slice_alloc
>     #3 0x7f21591c988d in g_hash_table_new_full
>     #4 0x556114d756f1 in object_initialize_with_type ../qom/object.c:506:23
>     #5 0x556114d77590 in object_new_with_type ../qom/object.c:706:5
>     #6 0x556114d777e8 in object_new ../qom/object.c:722:12
>     #7 0x556114d6d2be in qemu_allocate_irq ../hw/core/irq.c:94:25
>     #8 0x556114d6d1bc in qemu_extend_irqs ../hw/core/irq.c:82:16
>     #9 0x556114d6d329 in qemu_allocate_irqs ../hw/core/irq.c:89:12
>     #10 0x5561135da9bd in ahci_realize ../hw/ide/ahci.c:1644:12
>     #11 0x55611360156a in pci_ich9_ahci_realize ../hw/ide/ich.c:132:5
>
> Although ahci_realize() frees the return of qemu_allocate_irqs() right
> away, the individual IRQStates are still left around.
>
> Issue qemu_free_irq() at ahci_uninit().
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  hw/ide/ahci.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 6b04762c4a..86f6297dc9 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1688,6 +1688,7 @@ void ahci_uninit(AHCIState *s)
>              }
>              ide_exit(ide_state);
>          }
> +        qemu_free_irq(ad->port.irq);
>          object_unparent(OBJECT(&ad->port));
>      }
>  
Reviewed-by: Denis V. Lunev <den@openvz.org>

Thanks!

Re: [PATCH 1/3] ahci: Fix leak of IRQState

[Peter Maydell]

On Thu, 3 Sept 2026 at 23:06, Fabiano Rosas <farosas@suse.de> wrote:
>
> ASAN spotted an indirect leak in ahci-test:
>
> Indirect leak of 576 byte(s) in 6 object(s) allocated from:
>     #0 0x556112a08ca0 in malloc
>     #1 0x7f21591e595d in g_malloc
>     #2 0x7f21591ffe38 in g_slice_alloc
>     #3 0x7f21591c988d in g_hash_table_new_full
>     #4 0x556114d756f1 in object_initialize_with_type ../qom/object.c:506:23
>     #5 0x556114d77590 in object_new_with_type ../qom/object.c:706:5
>     #6 0x556114d777e8 in object_new ../qom/object.c:722:12
>     #7 0x556114d6d2be in qemu_allocate_irq ../hw/core/irq.c:94:25
>     #8 0x556114d6d1bc in qemu_extend_irqs ../hw/core/irq.c:82:16
>     #9 0x556114d6d329 in qemu_allocate_irqs ../hw/core/irq.c:89:12
>     #10 0x5561135da9bd in ahci_realize ../hw/ide/ahci.c:1644:12
>     #11 0x55611360156a in pci_ich9_ahci_realize ../hw/ide/ich.c:132:5
>
> Although ahci_realize() frees the return of qemu_allocate_irqs() right
> away, the individual IRQStates are still left around.
>
> Issue qemu_free_irq() at ahci_uninit().
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Classic leak resulting from qemu_allocate_irqs(). Ideally I
would like to be able to remove that function exactly because
it does tend to result in leaks, but we still have some users
nobody's cared enough to try to rework yet.

Another way to fix this would be to embed the IRQState into
the AHCIDevice struct, and then use qemu_init_irq_child(),
similarly to how b38859be76abf removed the qemu_allocate_irqs()
use from hw/isa/piix.c.

But I have no objection to this "just fix the leak by calling
the free function" patch.

thanks
-- PMM

Re: [PATCH 1/3] ahci: Fix leak of IRQState

[Philippe Mathieu-Daudé]

Hi Peter,

On 4/9/26 10:31, Peter Maydell wrote:
> On Thu, 3 Sept 2026 at 23:06, Fabiano Rosas <farosas@suse.de> wrote:
>>
>> ASAN spotted an indirect leak in ahci-test:
>>
>> Indirect leak of 576 byte(s) in 6 object(s) allocated from:
>>      #0 0x556112a08ca0 in malloc
>>      #1 0x7f21591e595d in g_malloc
>>      #2 0x7f21591ffe38 in g_slice_alloc
>>      #3 0x7f21591c988d in g_hash_table_new_full
>>      #4 0x556114d756f1 in object_initialize_with_type ../qom/object.c:506:23
>>      #5 0x556114d77590 in object_new_with_type ../qom/object.c:706:5
>>      #6 0x556114d777e8 in object_new ../qom/object.c:722:12
>>      #7 0x556114d6d2be in qemu_allocate_irq ../hw/core/irq.c:94:25
>>      #8 0x556114d6d1bc in qemu_extend_irqs ../hw/core/irq.c:82:16
>>      #9 0x556114d6d329 in qemu_allocate_irqs ../hw/core/irq.c:89:12
>>      #10 0x5561135da9bd in ahci_realize ../hw/ide/ahci.c:1644:12
>>      #11 0x55611360156a in pci_ich9_ahci_realize ../hw/ide/ich.c:132:5
>>
>> Although ahci_realize() frees the return of qemu_allocate_irqs() right
>> away, the individual IRQStates are still left around.
>>
>> Issue qemu_free_irq() at ahci_uninit().
>>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> 
> Classic leak resulting from qemu_allocate_irqs(). Ideally I
> would like to be able to remove that function exactly because
> it does tend to result in leaks, but we still have some users
> nobody's cared enough to try to rework yet.

TBH "nobody cared enough" is a bit unfair, various developers
(included yourself, Mark, Bernhard, Daniel, myself and others)
wasted a huge amount of time in the past trying to do that on
legacy devices and various series efforts ended dropped (which
is why I used "wasted") because we were bikeshedding on unrelated
details.

Hasn't Alex already solved that now for all devices in his latest
series?
https://lore.kernel.org/qemu-devel/20260718213652.37673-118-graf@amazon.com/

> Another way to fix this would be to embed the IRQState into
> the AHCIDevice struct, and then use qemu_init_irq_child(),
> similarly to how b38859be76abf removed the qemu_allocate_irqs()
> use from hw/isa/piix.c.
> 
> But I have no objection to this "just fix the leak by calling
> the free function" patch.
> 
> thanks
> -- PMM
> 

[PATCH 2/3] migration: Drop iochannel reference during snapshot setup

[Fabiano Rosas]

During migration there are usually two references to the iochannel,
one from the channel creation itself and another from the ownership
transfer into QEMUFile.

- The reference from the QEMUFile is decremented at qemu_fclose.

- The original reference is decremented at migration_connect_outgoing()
for regular migration and not at all for snapshots.

The ide-test has recently added migration support and ASAN has
flagged:

Indirect leak of 32 byte(s) in 1 object(s) allocated from:
    #0 0x55ca12aac0b9 in realloc
    #1 0x7fd97d7e5a05 in g_realloc
    #3 0x7fd97d7c98c6 in g_hash_table_new_full
    #4 0x55ca14e186f1 in object_initialize_with_type ../qom/object.c:506:23
    #5 0x55ca14e1a590 in object_new_with_type ../qom/object.c:706:5
    #6 0x55ca14e1a7e8 in object_new ../qom/object.c:722:12
    #7 0x55ca12de896f in qio_channel_block_new ../migration/channel-block.c:32:29
    #8 0x55ca12ec8f2d in qemu_fopen_bdrv ../migration/savevm.c:172:49
    #9 0x55ca12ec8b42 in save_snapshot ../migration/savevm.c:3375:9
    #10 0x55ca12ee18e3 in hmp_savevm ../migration/migration-hmp-cmds.c:497:5

Change qemu_fopen_bdrv() to drop the original reference once the
QEMUFile has taken over the object. The xen code already does the
same.

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/savevm.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 5b0e89ca7c..8d4bdc28ab 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -168,11 +168,16 @@ static bool qemu_loadvm_thread_pool_wait(MigrationState *s,
 
 static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
 {
+    QIOChannel *ioc = QIO_CHANNEL(qio_channel_block_new(bs));
+    QEMUFile *f;
+
     if (is_writable) {
-        return qemu_file_new_output(QIO_CHANNEL(qio_channel_block_new(bs)));
+        f = qemu_file_new_output(ioc);
     } else {
-        return qemu_file_new_input(QIO_CHANNEL(qio_channel_block_new(bs)));
+        f = qemu_file_new_input(ioc);
     }
+    object_unref(ioc);
+    return f;
 }
 
 
-- 
2.53.0

Re: [PATCH 2/3] migration: Drop iochannel reference during snapshot setup

[Marc-André Lureau]

On Fri, Sep 4, 2026 at 2:06 AM Fabiano Rosas <farosas@suse.de> wrote:
>
> During migration there are usually two references to the iochannel,
> one from the channel creation itself and another from the ownership
> transfer into QEMUFile.
>
> - The reference from the QEMUFile is decremented at qemu_fclose.
>
> - The original reference is decremented at migration_connect_outgoing()
> for regular migration and not at all for snapshots.
>
> The ide-test has recently added migration support and ASAN has
> flagged:
>
> Indirect leak of 32 byte(s) in 1 object(s) allocated from:
>     #0 0x55ca12aac0b9 in realloc
>     #1 0x7fd97d7e5a05 in g_realloc
>     #3 0x7fd97d7c98c6 in g_hash_table_new_full
>     #4 0x55ca14e186f1 in object_initialize_with_type ../qom/object.c:506:23
>     #5 0x55ca14e1a590 in object_new_with_type ../qom/object.c:706:5
>     #6 0x55ca14e1a7e8 in object_new ../qom/object.c:722:12
>     #7 0x55ca12de896f in qio_channel_block_new ../migration/channel-block.c:32:29
>     #8 0x55ca12ec8f2d in qemu_fopen_bdrv ../migration/savevm.c:172:49
>     #9 0x55ca12ec8b42 in save_snapshot ../migration/savevm.c:3375:9
>     #10 0x55ca12ee18e3 in hmp_savevm ../migration/migration-hmp-cmds.c:497:5
>
> Change qemu_fopen_bdrv() to drop the original reference once the
> QEMUFile has taken over the object. The xen code already does the
> same.
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

> ---
>  migration/savevm.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 5b0e89ca7c..8d4bdc28ab 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -168,11 +168,16 @@ static bool qemu_loadvm_thread_pool_wait(MigrationState *s,
>
>  static QEMUFile *qemu_fopen_bdrv(BlockDriverState *bs, int is_writable)
>  {
> +    QIOChannel *ioc = QIO_CHANNEL(qio_channel_block_new(bs));
> +    QEMUFile *f;
> +
>      if (is_writable) {
> -        return qemu_file_new_output(QIO_CHANNEL(qio_channel_block_new(bs)));
> +        f = qemu_file_new_output(ioc);
>      } else {
> -        return qemu_file_new_input(QIO_CHANNEL(qio_channel_block_new(bs)));
> +        f = qemu_file_new_input(ioc);
>      }
> +    object_unref(ioc);
> +    return f;
>  }
>
>
> --
> 2.53.0
>
>


-- 
Marc-André Lureau

Re: [PATCH 2/3] migration: Drop iochannel reference during snapshot setup

[Peter Xu]

On Thu, Sep 03, 2026 at 07:05:22PM -0300, Fabiano Rosas wrote:
> During migration there are usually two references to the iochannel,
> one from the channel creation itself and another from the ownership
> transfer into QEMUFile.
> 
> - The reference from the QEMUFile is decremented at qemu_fclose.
> 
> - The original reference is decremented at migration_connect_outgoing()
> for regular migration and not at all for snapshots.
> 
> The ide-test has recently added migration support and ASAN has
> flagged:
> 
> Indirect leak of 32 byte(s) in 1 object(s) allocated from:
>     #0 0x55ca12aac0b9 in realloc
>     #1 0x7fd97d7e5a05 in g_realloc
>     #3 0x7fd97d7c98c6 in g_hash_table_new_full
>     #4 0x55ca14e186f1 in object_initialize_with_type ../qom/object.c:506:23
>     #5 0x55ca14e1a590 in object_new_with_type ../qom/object.c:706:5
>     #6 0x55ca14e1a7e8 in object_new ../qom/object.c:722:12
>     #7 0x55ca12de896f in qio_channel_block_new ../migration/channel-block.c:32:29
>     #8 0x55ca12ec8f2d in qemu_fopen_bdrv ../migration/savevm.c:172:49
>     #9 0x55ca12ec8b42 in save_snapshot ../migration/savevm.c:3375:9
>     #10 0x55ca12ee18e3 in hmp_savevm ../migration/migration-hmp-cmds.c:497:5
> 
> Change qemu_fopen_bdrv() to drop the original reference once the
> QEMUFile has taken over the object. The xen code already does the
> same.
> 
> Signed-off-by: Fabiano Rosas <farosas@suse.de>

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

-- 
Peter Xu

[PATCH 3/3] migration: Free the JSON writer during snapshots

[Fabiano Rosas]

The JSON writer is created at migrate_init() and freed at
migration_cleanup(). The latter is not called for snapshots. Add a
call to migration_cleanup_json_writer() to make sure it doesn't leak:

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x55ca12aabca0 in malloc
    #1 0x7fd97d7e595d in g_malloc
    #2 0x7fd97d7ffe38 in g_slice_alloc
    #3 0x7fd97d8042e2 in g_string_sized_new
    #4 0x55ca1571dd6b in json_writer_new ../qobject/json-writer.c:36:24
    #5 0x55ca12e0de56 in migrate_init ../migration/migration.c:1718:21
    #6 0x55ca12ec8fe6 in qemu_savevm_state ../migration/savevm.c:1921:11
    #7 0x55ca12ec8b93 in save_snapshot ../migration/savevm.c:3380:11

Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/migration.c | 2 +-
 migration/migration.h | 1 +
 migration/savevm.c    | 2 +-
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index b413d28622..d0b864a760 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1330,7 +1330,7 @@ void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
     }
 }
 
-static void migration_cleanup_json_writer(MigrationState *s)
+void migration_cleanup_json_writer(MigrationState *s)
 {
     g_clear_pointer(&s->vmdesc, json_writer_free);
 }
diff --git a/migration/migration.h b/migration/migration.h
index e47ff4e3d1..683bc7bdd5 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -633,4 +633,5 @@ void migration_bitmap_sync_precopy(bool last_stage);
 void dirty_bitmap_mig_init(void);
 bool should_send_vmdesc(void);
 
+void migration_cleanup_json_writer(MigrationState *s);
 #endif
diff --git a/migration/savevm.c b/migration/savevm.c
index 8d4bdc28ab..a31f34f25c 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -3404,7 +3404,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
 
  the_end:
     bdrv_drain_all_end();
-
+    migration_cleanup_json_writer(migrate_get_current());
     vm_resume(saved_state);
     return ret == 0;
 }
-- 
2.53.0

Re: [PATCH 3/3] migration: Free the JSON writer during snapshots

[Marc-André Lureau]

Hi


On Fri, Sep 4, 2026 at 2:06 AM Fabiano Rosas <farosas@suse.de> wrote:
>
> The JSON writer is created at migrate_init() and freed at
> migration_cleanup(). The latter is not called for snapshots. Add a
> call to migration_cleanup_json_writer() to make sure it doesn't leak:
>
> Indirect leak of 24 byte(s) in 1 object(s) allocated from:
>     #0 0x55ca12aabca0 in malloc
>     #1 0x7fd97d7e595d in g_malloc
>     #2 0x7fd97d7ffe38 in g_slice_alloc
>     #3 0x7fd97d8042e2 in g_string_sized_new
>     #4 0x55ca1571dd6b in json_writer_new ../qobject/json-writer.c:36:24
>     #5 0x55ca12e0de56 in migrate_init ../migration/migration.c:1718:21
>     #6 0x55ca12ec8fe6 in qemu_savevm_state ../migration/savevm.c:1921:11
>     #7 0x55ca12ec8b93 in save_snapshot ../migration/savevm.c:3380:11
>
> Signed-off-by: Fabiano Rosas <farosas@suse.de>
> ---
>  migration/migration.c | 2 +-
>  migration/migration.h | 1 +
>  migration/savevm.c    | 2 +-
>  3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index b413d28622..d0b864a760 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1330,7 +1330,7 @@ void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
>      }
>  }
>
> -static void migration_cleanup_json_writer(MigrationState *s)
> +void migration_cleanup_json_writer(MigrationState *s)
>  {
>      g_clear_pointer(&s->vmdesc, json_writer_free);
>  }
> diff --git a/migration/migration.h b/migration/migration.h
> index e47ff4e3d1..683bc7bdd5 100644
> --- a/migration/migration.h
> +++ b/migration/migration.h
> @@ -633,4 +633,5 @@ void migration_bitmap_sync_precopy(bool last_stage);
>  void dirty_bitmap_mig_init(void);
>  bool should_send_vmdesc(void);
>
> +void migration_cleanup_json_writer(MigrationState *s);
>  #endif
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 8d4bdc28ab..a31f34f25c 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -3404,7 +3404,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
>
>   the_end:
>      bdrv_drain_all_end();
> -
> +    migration_cleanup_json_writer(migrate_get_current());
>      vm_resume(saved_state);
>      return ret == 0;
>  }

rather than exposing internal helper, can it be done where it calls init?

diff --git a/migration/savevm.c b/migration/savevm.c
index e2aaedeab565..e08dc2e8c258 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1952,6 +1952,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
     }
 cleanup:
     qemu_savevm_state_cleanup();
+    g_clear_pointer(&ms->vmdesc, json_writer_free);


--
Marc-André Lureau

Re: [PATCH 3/3] migration: Free the JSON writer during snapshots

[Peter Xu]

On Fri, Sep 04, 2026 at 04:09:26PM +0400, Marc-André Lureau wrote:
> Hi
> 
> 
> On Fri, Sep 4, 2026 at 2:06 AM Fabiano Rosas <farosas@suse.de> wrote:
> >
> > The JSON writer is created at migrate_init() and freed at
> > migration_cleanup(). The latter is not called for snapshots. Add a
> > call to migration_cleanup_json_writer() to make sure it doesn't leak:
> >
> > Indirect leak of 24 byte(s) in 1 object(s) allocated from:
> >     #0 0x55ca12aabca0 in malloc
> >     #1 0x7fd97d7e595d in g_malloc
> >     #2 0x7fd97d7ffe38 in g_slice_alloc
> >     #3 0x7fd97d8042e2 in g_string_sized_new
> >     #4 0x55ca1571dd6b in json_writer_new ../qobject/json-writer.c:36:24
> >     #5 0x55ca12e0de56 in migrate_init ../migration/migration.c:1718:21
> >     #6 0x55ca12ec8fe6 in qemu_savevm_state ../migration/savevm.c:1921:11
> >     #7 0x55ca12ec8b93 in save_snapshot ../migration/savevm.c:3380:11
> >
> > Signed-off-by: Fabiano Rosas <farosas@suse.de>
> > ---
> >  migration/migration.c | 2 +-
> >  migration/migration.h | 1 +
> >  migration/savevm.c    | 2 +-
> >  3 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/migration/migration.c b/migration/migration.c
> > index b413d28622..d0b864a760 100644
> > --- a/migration/migration.c
> > +++ b/migration/migration.c
> > @@ -1330,7 +1330,7 @@ void migrate_set_state(MigrationStatus *state, MigrationStatus old_state,
> >      }
> >  }
> >
> > -static void migration_cleanup_json_writer(MigrationState *s)
> > +void migration_cleanup_json_writer(MigrationState *s)
> >  {
> >      g_clear_pointer(&s->vmdesc, json_writer_free);
> >  }
> > diff --git a/migration/migration.h b/migration/migration.h
> > index e47ff4e3d1..683bc7bdd5 100644
> > --- a/migration/migration.h
> > +++ b/migration/migration.h
> > @@ -633,4 +633,5 @@ void migration_bitmap_sync_precopy(bool last_stage);
> >  void dirty_bitmap_mig_init(void);
> >  bool should_send_vmdesc(void);
> >
> > +void migration_cleanup_json_writer(MigrationState *s);
> >  #endif
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 8d4bdc28ab..a31f34f25c 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -3404,7 +3404,7 @@ bool save_snapshot(const char *name, bool overwrite, const char *vmstate,
> >
> >   the_end:
> >      bdrv_drain_all_end();
> > -
> > +    migration_cleanup_json_writer(migrate_get_current());
> >      vm_resume(saved_state);
> >      return ret == 0;
> >  }
> 
> rather than exposing internal helper, can it be done where it calls init?
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index e2aaedeab565..e08dc2e8c258 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1952,6 +1952,7 @@ static int qemu_savevm_state(QEMUFile *f, Error **errp)
>      }
>  cleanup:
>      qemu_savevm_state_cleanup();
> +    g_clear_pointer(&ms->vmdesc, json_writer_free);

IMHO migration_cleanup_json_writer() is fine to be available in whole
migration/, but I also agree qemu_savevm_state() seems to be the better
place.

-- 
Peter Xu