* [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
@ 2018-06-04 9:55 Lidong Chen
2018-07-05 11:05 ` Dr. David Alan Gilbert
2018-07-06 9:53 ` Dr. David Alan Gilbert
0 siblings, 2 replies; 6+ messages in thread
From: Lidong Chen @ 2018-06-04 9:55 UTC (permalink / raw)
To: quintela, dgilbert; +Cc: qemu-devel, Lidong Chen
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(+)
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
2018-06-04 9:55 [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize Lidong Chen
@ 2018-07-05 11:05 ` Dr. David Alan Gilbert
2018-07-06 9:53 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2018-07-05 11:05 UTC (permalink / raw)
To: Lidong Chen; +Cc: quintela, qemu-devel, Lidong Chen
* 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
2018-06-04 9:55 [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize Lidong Chen
2018-07-05 11:05 ` Dr. David Alan Gilbert
@ 2018-07-06 9:53 ` Dr. David Alan Gilbert
2018-07-06 10:41 ` Dr. David Alan Gilbert
1 sibling, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2018-07-06 9:53 UTC (permalink / raw)
To: Lidong Chen; +Cc: quintela, qemu-devel, Lidong Chen
* 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
2018-07-06 9:53 ` Dr. David Alan Gilbert
@ 2018-07-06 10:41 ` Dr. David Alan Gilbert
2018-07-12 4:08 ` 858585 jemmy
0 siblings, 1 reply; 6+ messages in thread
From: Dr. David Alan Gilbert @ 2018-07-06 10:41 UTC (permalink / raw)
To: Lidong Chen; +Cc: quintela, qemu-devel, Lidong Chen
* 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
2018-07-06 10:41 ` Dr. David Alan Gilbert
@ 2018-07-12 4:08 ` 858585 jemmy
2018-07-17 14:55 ` 858585 jemmy
0 siblings, 1 reply; 6+ messages in thread
From: 858585 jemmy @ 2018-07-12 4:08 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Juan Quintela, qemu-devel, Lidong Chen
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize
2018-07-12 4:08 ` 858585 jemmy
@ 2018-07-17 14:55 ` 858585 jemmy
0 siblings, 0 replies; 6+ messages in thread
From: 858585 jemmy @ 2018-07-17 14:55 UTC (permalink / raw)
To: Dr. David Alan Gilbert; +Cc: Juan Quintela, qemu-devel, Lidong Chen
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-07-17 14:55 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-04 9:55 [Qemu-devel] [PATCH] migration: release MigrationIncomingState in migration_object_finalize Lidong Chen
2018-07-05 11:05 ` Dr. David Alan Gilbert
2018-07-06 9:53 ` Dr. David Alan Gilbert
2018-07-06 10:41 ` Dr. David Alan Gilbert
2018-07-12 4:08 ` 858585 jemmy
2018-07-17 14:55 ` 858585 jemmy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.