Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize

[858585 jemmy]

On Thu, Jul 12, 2018 at 12:08 PM, 858585 jemmy <address@hidden> wrote:
> On Fri, Jul 6, 2018 at 6:41 PM, Dr. David Alan Gilbert
> <address@hidden> wrote:
>> * Dr. David Alan Gilbert (address@hidden) wrote:
>>> * Lidong Chen (address@hidden) wrote:
>>> > Qemu initialize the MigrationIncomingState structure in 
>>> > migration_object_init,
>>> > but not release it. this patch release it in migration_object_finalize.
>>> >
>>> > Signed-off-by: Lidong Chen <address@hidden>
>>>
>>> Queued
>>
>> I've had to unqueue this, see below:
>>
>>>
>>> > ---
>>> >  migration/migration.c | 7 +++++++
>>> >  1 file changed, 7 insertions(+)
>>> >
>>> > diff --git a/migration/migration.c b/migration/migration.c
>>> > index 05aec2c..e009a05 100644
>>> > --- a/migration/migration.c
>>> > +++ b/migration/migration.c
>>> > @@ -156,6 +156,13 @@ void migration_object_init(void)
>>> >  void migration_object_finalize(void)
>>> >  {
>>> >      object_unref(OBJECT(current_migration));
>>> > +
>>> > +    qemu_sem_destroy(&current_incoming->postcopy_pause_sem_fault);
>>> > +    qemu_sem_destroy(&current_incoming->postcopy_pause_sem_dst);
>>> > +    qemu_event_destroy(&current_incoming->main_thread_load_event);
>>> > +    qemu_mutex_destroy(&current_incoming->rp_mutex);
>>> > +    g_array_free(current_incoming->postcopy_remote_fds, true);
>>
>> That array is already free'd in migration_incoming_state_destroy,
>> so I see reliable glib assert's from this array free.
>
> The migration_incoming_state_destroy only invoked in destination qemu.
> The source qemu will not free this memory.
> So I think free current_incoming->postcopy_remote_fds is not good way.
>
> and migration_object_init and migration_object_finalize should not be
> invoked in main
> function. It's better to  alloc memory when start migration and
> release it when migration finished.
>
> I will submit a new version patch to fix it.

I find many function use current_incoming and current_migration,
if we alloc these when migration start, and release these when
migration finished, it need change many function.
so I will just remove
g_array_free(current_incoming->postcopy_remote_fds, true) from the
patch.

Thanks

>
>>
>> Dave
>>
>>> > +    g_free(current_incoming);
>>> >  }
>>> >
>>> >  /* For outgoing */
>>> > --
>>> > 1.8.3.1
>>> >
>>> --
>>> Dr. David Alan Gilbert / address@hidden / Manchester, UK
>> --
>> Dr. David Alan Gilbert / address@hidden / Manchester, UK