| Message ID | 1528106113-32692-1-git-send-email-lidongchen@tencent.com |
|---|---|
| State | New |
| Headers | show |
| Series | migration: release MigrationIncomingState in migration_object_finalize | expand |
* Lidong Chen (jemmy858585@gmail.com) 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 <lidongchen@tencent.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > --- > 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(¤t_incoming->postcopy_pause_sem_fault); > + qemu_sem_destroy(¤t_incoming->postcopy_pause_sem_dst); > + qemu_event_destroy(¤t_incoming->main_thread_load_event); > + qemu_mutex_destroy(¤t_incoming->rp_mutex); > + g_array_free(current_incoming->postcopy_remote_fds, true); > + g_free(current_incoming); > } > > /* For outgoing */ > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Lidong Chen (jemmy858585@gmail.com) 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 <lidongchen@tencent.com> Queued > --- > 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(¤t_incoming->postcopy_pause_sem_fault); > + qemu_sem_destroy(¤t_incoming->postcopy_pause_sem_dst); > + qemu_event_destroy(¤t_incoming->main_thread_load_event); > + qemu_mutex_destroy(¤t_incoming->rp_mutex); > + g_array_free(current_incoming->postcopy_remote_fds, true); > + g_free(current_incoming); > } > > /* For outgoing */ > -- > 1.8.3.1 > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote: > * Lidong Chen (jemmy858585@gmail.com) 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 <lidongchen@tencent.com> > > 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(¤t_incoming->postcopy_pause_sem_fault); > > + qemu_sem_destroy(¤t_incoming->postcopy_pause_sem_dst); > > + qemu_event_destroy(¤t_incoming->main_thread_load_event); > > + qemu_mutex_destroy(¤t_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. Dave > > + g_free(current_incoming); > > } > > > > /* For outgoing */ > > -- > > 1.8.3.1 > > > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Fri, Jul 6, 2018 at 6:41 PM, Dr. David Alan Gilbert <dgilbert@redhat.com> wrote: > * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote: >> * Lidong Chen (jemmy858585@gmail.com) 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 <lidongchen@tencent.com> >> >> 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(¤t_incoming->postcopy_pause_sem_fault); >> > + qemu_sem_destroy(¤t_incoming->postcopy_pause_sem_dst); >> > + qemu_event_destroy(¤t_incoming->main_thread_load_event); >> > + qemu_mutex_destroy(¤t_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. > > Dave > >> > + g_free(current_incoming); >> > } >> > >> > /* For outgoing */ >> > -- >> > 1.8.3.1 >> > >> -- >> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On Thu, Jul 12, 2018 at 12:08 PM, 858585 jemmy <jemmy858585@gmail.com> wrote: > On Fri, Jul 6, 2018 at 6:41 PM, Dr. David Alan Gilbert > <dgilbert@redhat.com> wrote: >> * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote: >>> * Lidong Chen (jemmy858585@gmail.com) 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 <lidongchen@tencent.com> >>> >>> 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(¤t_incoming->postcopy_pause_sem_fault); >>> > + qemu_sem_destroy(¤t_incoming->postcopy_pause_sem_dst); >>> > + qemu_event_destroy(¤t_incoming->main_thread_load_event); >>> > + qemu_mutex_destroy(¤t_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 / dgilbert@redhat.com / Manchester, UK >> -- >> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
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(¤t_incoming->postcopy_pause_sem_fault); + qemu_sem_destroy(¤t_incoming->postcopy_pause_sem_dst); + qemu_event_destroy(¤t_incoming->main_thread_load_event); + qemu_mutex_destroy(¤t_incoming->rp_mutex); + g_array_free(current_incoming->postcopy_remote_fds, true); + g_free(current_incoming); } /* For outgoing */
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 <lidongchen@tencent.com> --- migration/migration.c | 7 +++++++ 1 file changed, 7 insertions(+)