Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
On Thu, Jul 12, 2018 at 12:08 PM, 858585 jemmy wrote: > On Fri, Jul 6, 2018 at 6:41 PM, Dr. David Alan Gilbert > wrote: >> * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: >>> * Lidong Chen (jemmy858...@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 >>> >>> 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(_incoming->postcopy_pause_sem_fault); >>> > +qemu_sem_destroy(_incoming->postcopy_pause_sem_dst); >>> > +qemu_event_destroy(_incoming->main_thread_load_event); >>> > +qemu_mutex_destroy(_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 / dgilb...@redhat.com / Manchester, UK >> -- >> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
On Fri, Jul 6, 2018 at 6:41 PM, Dr. David Alan Gilbert wrote: > * Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: >> * Lidong Chen (jemmy858...@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 >> >> 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(_incoming->postcopy_pause_sem_fault); >> > +qemu_sem_destroy(_incoming->postcopy_pause_sem_dst); >> > +qemu_event_destroy(_incoming->main_thread_load_event); >> > +qemu_mutex_destroy(_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 / dgilb...@redhat.com / Manchester, UK > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
* Dr. David Alan Gilbert (dgilb...@redhat.com) wrote: > * Lidong Chen (jemmy858...@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 > > 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(_incoming->postcopy_pause_sem_fault); > > +qemu_sem_destroy(_incoming->postcopy_pause_sem_dst); > > +qemu_event_destroy(_incoming->main_thread_load_event); > > +qemu_mutex_destroy(_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 / dgilb...@redhat.com / Manchester, UK -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
* Lidong Chen (jemmy858...@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 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(_incoming->postcopy_pause_sem_fault); > +qemu_sem_destroy(_incoming->postcopy_pause_sem_dst); > +qemu_event_destroy(_incoming->main_thread_load_event); > +qemu_mutex_destroy(_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 / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
* Lidong Chen (jemmy858...@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 Reviewed-by: Dr. David Alan Gilbert > --- > 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(_incoming->postcopy_pause_sem_fault); > +qemu_sem_destroy(_incoming->postcopy_pause_sem_dst); > +qemu_event_destroy(_incoming->main_thread_load_event); > +qemu_mutex_destroy(_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 / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
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 --- 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(_incoming->postcopy_pause_sem_fault); +qemu_sem_destroy(_incoming->postcopy_pause_sem_dst); +qemu_event_destroy(_incoming->main_thread_load_event); +qemu_mutex_destroy(_incoming->rp_mutex); +g_array_free(current_incoming->postcopy_remote_fds, true); +g_free(current_incoming); } /* For outgoing */ -- 1.8.3.1