On Thu, Jan 12, 2023 at 06:40:00PM +0000, Dr. David Alan Gilbert wrote: > * David Hildenbrand (da...@redhat.com) wrote: > > On 12.01.23 18:43, Dr. David Alan Gilbert wrote: > > > * David Hildenbrand (da...@redhat.com) wrote: > > > > ... and store it in the migration state. This is a preparation for > > > > storing selected vmds's already in qemu_savevm_state_setup(). > > > > > > > > Signed-off-by: David Hildenbrand <da...@redhat.com> > > > > --- > > > > migration/migration.c | 4 ++++ > > > > migration/migration.h | 4 ++++ > > > > migration/savevm.c | 18 ++++++++++++------ > > > > 3 files changed, 20 insertions(+), 6 deletions(-) > > > > > > > > [1] > > > > > > diff --git a/migration/migration.c b/migration/migration.c > > > > index 52b5d39244..1d33a7efa0 100644 > > > > --- a/migration/migration.c > > > > +++ b/migration/migration.c > > > > @@ -2170,6 +2170,9 @@ void migrate_init(MigrationState *s) > > > > s->vm_was_running = false; > > > > s->iteration_initial_bytes = 0; > > > > s->threshold_size = 0; > > > > + > > > > + json_writer_free(s->vmdesc); > > > > + s->vmdesc = NULL; > > > > } > > > > [...] > > > > > > trace_savevm_state_setup(); > > > > QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > > > > if (!se->ops || !se->ops->save_setup) { > > > > @@ -1390,15 +1395,12 @@ int > > > > qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, > > > > bool in_postcopy, > > > > bool > > > > inactivate_disks) > > > > { > > > > - g_autoptr(JSONWriter) vmdesc = NULL; > > > > + MigrationState *ms = migrate_get_current(); > > > > + JSONWriter *vmdesc = ms->vmdesc; > > > > int vmdesc_len; > > > > SaveStateEntry *se; > > > > int ret; > > > > - vmdesc = json_writer_new(false); > > > > - json_writer_start_object(vmdesc, NULL); > > > > - json_writer_int64(vmdesc, "page_size", qemu_target_page_size()); > > > > - json_writer_start_array(vmdesc, "devices"); > > > > QTAILQ_FOREACH(se, &savevm_state.handlers, entry) { > > > > ret = vmstate_save(f, se, vmdesc); > > > > if (ret) { > > > > @@ -1433,6 +1435,10 @@ int > > > > qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f, > > > > qemu_put_buffer(f, (uint8_t *)json_writer_get(vmdesc), > > > > vmdesc_len); > > > > } > > > > + /* Free it now to detect any inconsistencies. */ > > > > + json_writer_free(vmdesc); > > > > + ms->vmdesc = NULL; > > > > > > and this only happens when this succesfully exits; so if this errors > > > out, and then you retry an outwards migration, I think you've leaked a > > > writer. > > > > Shouldn't the change [1] to migrate_init() cover that? > > Hmm OK, yes it does - I guess it does mean you keep the allocation > around for a bit longer, but that's OK in practice since normally you'll > be quitting soon.
Instead of json_writer_free() here and there, how about free it in migrate_fd_cleanup() once and for all? -- Peter Xu