[Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[arei.gonglei]

From: ChenLiang <chenliang88@huawei.com>

The static variables in migration_bitmap_sync will not be reset in
the case of a second attempted migration.

Signed-off-by: ChenLiang <chenliang88@huawei.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 arch_init.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 60c975d..10516cb 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -468,15 +468,23 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
 
 
 /* Needs iothread lock! */
+/* Fix me: there are too many global variables used in migration process. */
+static int64_t start_time;
+static int64_t bytes_xfer_prev;
+static int64_t num_dirty_pages_period;
+
+static void migration_bitmap_sync_init(void)
+{
+    start_time = 0;
+    bytes_xfer_prev = 0;
+    num_dirty_pages_period = 0;
+}
 
 static void migration_bitmap_sync(void)
 {
     RAMBlock *block;
     uint64_t num_dirty_pages_init = migration_dirty_pages;
     MigrationState *s = migrate_get_current();
-    static int64_t start_time;
-    static int64_t bytes_xfer_prev;
-    static int64_t num_dirty_pages_period;
     int64_t end_time;
     int64_t bytes_xfer_now;
 
@@ -733,6 +741,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
     migration_dirty_pages = ram_pages;
     mig_throttle_on = false;
     dirty_rate_high_cnt = 0;
+    migration_bitmap_sync_init();
 
     if (migrate_use_xbzrle()) {
         qemu_mutex_lock_iothread();
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> The static variables in migration_bitmap_sync will not be reset in
> the case of a second attempted migration.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Good catch.  Applied..

> ---
>  arch_init.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 60c975d..10516cb 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -468,15 +468,23 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
>  
>  
>  /* Needs iothread lock! */
> +/* Fix me: there are too many global variables used in migration process. */
> +static int64_t start_time;
> +static int64_t bytes_xfer_prev;
> +static int64_t num_dirty_pages_period;
> +
> +static void migration_bitmap_sync_init(void)
> +{
> +    start_time = 0;
> +    bytes_xfer_prev = 0;
> +    num_dirty_pages_period = 0;
> +}
>  
>  static void migration_bitmap_sync(void)
>  {
>      RAMBlock *block;
>      uint64_t num_dirty_pages_init = migration_dirty_pages;
>      MigrationState *s = migrate_get_current();
> -    static int64_t start_time;
> -    static int64_t bytes_xfer_prev;
> -    static int64_t num_dirty_pages_period;
>      int64_t end_time;
>      int64_t bytes_xfer_now;
>  
> @@ -733,6 +741,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      migration_dirty_pages = ram_pages;
>      mig_throttle_on = false;
>      dirty_rate_high_cnt = 0;
> +    migration_bitmap_sync_init();
>  
>      if (migrate_use_xbzrle()) {
>          qemu_mutex_lock_iothread();

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Juan Quintela]

<arei.gonglei@huawei.com> wrote:
> From: ChenLiang <chenliang88@huawei.com>
>
> The static variables in migration_bitmap_sync will not be reset in
> the case of a second attempted migration.
>
> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>

Good catch.  Applied..

> ---
>  arch_init.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/arch_init.c b/arch_init.c
> index 60c975d..10516cb 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -468,15 +468,23 @@ static void migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
>  
>  
>  /* Needs iothread lock! */
> +/* Fix me: there are too many global variables used in migration process. */
> +static int64_t start_time;
> +static int64_t bytes_xfer_prev;
> +static int64_t num_dirty_pages_period;
> +
> +static void migration_bitmap_sync_init(void)
> +{
> +    start_time = 0;
> +    bytes_xfer_prev = 0;
> +    num_dirty_pages_period = 0;
> +}
>  
>  static void migration_bitmap_sync(void)
>  {
>      RAMBlock *block;
>      uint64_t num_dirty_pages_init = migration_dirty_pages;
>      MigrationState *s = migrate_get_current();
> -    static int64_t start_time;
> -    static int64_t bytes_xfer_prev;
> -    static int64_t num_dirty_pages_period;
>      int64_t end_time;
>      int64_t bytes_xfer_now;
>  
> @@ -733,6 +741,7 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
>      migration_dirty_pages = ram_pages;
>      mig_throttle_on = false;
>      dirty_rate_high_cnt = 0;
> +    migration_bitmap_sync_init();
>  
>      if (migrate_use_xbzrle()) {
>          qemu_mutex_lock_iothread();

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Gonglei (Arei)]

Hi, Juan

Ping...

Maybe you forgot to pull this patch, right? Thanks.



Best regards,
-Gonglei


> -----Original Message-----
> From: Juan Quintela [mailto:quintela@redhat.com]
> Sent: Friday, March 21, 2014 9:26 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; owasserm@redhat.com; pbonzini@redhat.com;
> eblake@redhat.com; dgilbert@redhat.com; chenliang (T)
> Subject: Re: [PATCH] migration: static variables will not be reset at second
> migration
> 
> <arei.gonglei@huawei.com> wrote:
> > From: ChenLiang <chenliang88@huawei.com>
> >
> > The static variables in migration_bitmap_sync will not be reset in
> > the case of a second attempted migration.
> >
> > Signed-off-by: ChenLiang <chenliang88@huawei.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> 
> Good catch.  Applied..
> 
> > ---
> >  arch_init.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 60c975d..10516cb 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -468,15 +468,23 @@ static void
> migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
> >
> >
> >  /* Needs iothread lock! */
> > +/* Fix me: there are too many global variables used in migration process. */
> > +static int64_t start_time;
> > +static int64_t bytes_xfer_prev;
> > +static int64_t num_dirty_pages_period;
> > +
> > +static void migration_bitmap_sync_init(void)
> > +{
> > +    start_time = 0;
> > +    bytes_xfer_prev = 0;
> > +    num_dirty_pages_period = 0;
> > +}
> >
> >  static void migration_bitmap_sync(void)
> >  {
> >      RAMBlock *block;
> >      uint64_t num_dirty_pages_init = migration_dirty_pages;
> >      MigrationState *s = migrate_get_current();
> > -    static int64_t start_time;
> > -    static int64_t bytes_xfer_prev;
> > -    static int64_t num_dirty_pages_period;
> >      int64_t end_time;
> >      int64_t bytes_xfer_now;
> >
> > @@ -733,6 +741,7 @@ static int ram_save_setup(QEMUFile *f, void
> *opaque)
> >      migration_dirty_pages = ram_pages;
> >      mig_throttle_on = false;
> >      dirty_rate_high_cnt = 0;
> > +    migration_bitmap_sync_init();
> >
> >      if (migrate_use_xbzrle()) {
> >          qemu_mutex_lock_iothread();

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Gonglei (Arei)]

> -----Original Message-----
> From: Juan Quintela [mailto:quintela@redhat.com]
> Sent: Friday, March 21, 2014 9:26 PM
> To: Gonglei (Arei)
> Cc: qemu-devel@nongnu.org; owasserm@redhat.com; pbonzini@redhat.com;
> eblake@redhat.com; dgilbert@redhat.com; chenliang (T)
> Subject: Re: [PATCH] migration: static variables will not be reset at second
> migration
> 
> <arei.gonglei@huawei.com> wrote:
> > From: ChenLiang <chenliang88@huawei.com>
> >
> > The static variables in migration_bitmap_sync will not be reset in
> > the case of a second attempted migration.
> >
> > Signed-off-by: ChenLiang <chenliang88@huawei.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> 
> Good catch.  Applied..
> 

Hi, Juan? Ping... please :)

Best regards,
-Gonglei
> > ---
> >  arch_init.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch_init.c b/arch_init.c
> > index 60c975d..10516cb 100644
> > --- a/arch_init.c
> > +++ b/arch_init.c
> > @@ -468,15 +468,23 @@ static void
> migration_bitmap_sync_range(ram_addr_t start, ram_addr_t length)
> >
> >
> >  /* Needs iothread lock! */
> > +/* Fix me: there are too many global variables used in migration process. */
> > +static int64_t start_time;
> > +static int64_t bytes_xfer_prev;
> > +static int64_t num_dirty_pages_period;
> > +
> > +static void migration_bitmap_sync_init(void)
> > +{
> > +    start_time = 0;
> > +    bytes_xfer_prev = 0;
> > +    num_dirty_pages_period = 0;
> > +}
> >
> >  static void migration_bitmap_sync(void)
> >  {
> >      RAMBlock *block;
> >      uint64_t num_dirty_pages_init = migration_dirty_pages;
> >      MigrationState *s = migrate_get_current();
> > -    static int64_t start_time;
> > -    static int64_t bytes_xfer_prev;
> > -    static int64_t num_dirty_pages_period;
> >      int64_t end_time;
> >      int64_t bytes_xfer_now;
> >
> > @@ -733,6 +741,7 @@ static int ram_save_setup(QEMUFile *f, void
> *opaque)
> >      migration_dirty_pages = ram_pages;
> >      mig_throttle_on = false;
> >      dirty_rate_high_cnt = 0;
> > +    migration_bitmap_sync_init();
> >
> >      if (migrate_use_xbzrle()) {
> >          qemu_mutex_lock_iothread();

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Paolo Bonzini]

On 24/06/2014 08:23, Gonglei (Arei) wrote:
>> -----Original Message-----
>> From: Juan Quintela [mailto:quintela@redhat.com]
>> Sent: Friday, March 21, 2014 9:26 PM
>> To: Gonglei (Arei)
>> Cc: qemu-devel@nongnu.org; owasserm@redhat.com; pbonzini@redhat.com;
>> eblake@redhat.com; dgilbert@redhat.com; chenliang (T)
>> Subject: Re: [PATCH] migration: static variables will not be reset at second
>> migration
>>
>> <arei.gonglei@huawei.com> wrote:
>>> From: ChenLiang <chenliang88@huawei.com>
>>>
>>> The static variables in migration_bitmap_sync will not be reset in
>>> the case of a second attempted migration.
>>>
>>> Signed-off-by: ChenLiang <chenliang88@huawei.com>
>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>>
>> Good catch.  Applied..
>>
> 
> Hi, Juan? Ping... please :)

Juan, what happened to this patch?

Paolo

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Gonglei]

On 2014/11/20 19:30, Paolo Bonzini wrote:

> 
> 
> On 24/06/2014 08:23, Gonglei (Arei) wrote:
>>> -----Original Message-----
>>> From: Juan Quintela [mailto:quintela@redhat.com]
>>> Sent: Friday, March 21, 2014 9:26 PM
>>> To: Gonglei (Arei)
>>> Cc: qemu-devel@nongnu.org; owasserm@redhat.com; pbonzini@redhat.com;
>>> eblake@redhat.com; dgilbert@redhat.com; chenliang (T)
>>> Subject: Re: [PATCH] migration: static variables will not be reset at second
>>> migration
>>>
>>> <arei.gonglei@huawei.com> wrote:
>>>> From: ChenLiang <chenliang88@huawei.com>
>>>>
>>>> The static variables in migration_bitmap_sync will not be reset in
>>>> the case of a second attempted migration.
>>>>
>>>> Signed-off-by: ChenLiang <chenliang88@huawei.com>
>>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>>>
>>> Good catch.  Applied..
>>>
>>
>> Hi, Juan? Ping... please :)
> 
> Juan, what happened to this patch?
> 
> Paolo

Nearly for half a year, I forgot it completely :(
Thanks for your prompt, Paolo.

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Amit Shah]

On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote:
> >>>> The static variables in migration_bitmap_sync will not be reset in
> >>>> the case of a second attempted migration.
> >>>>
> >>>> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> >>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> >>>
> >>> Good catch.  Applied..
> >>>
> >>
> >> Hi, Juan? Ping... please :)
> > 
> > Juan, what happened to this patch?
>
> Nearly for half a year, I forgot it completely :(
> Thanks for your prompt, Paolo.

Yeah; unfortunate.

I feel the patch could've been done in a better way, but since it's
been a while...

Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2.

Thanks,

		Amit

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Dr. David Alan Gilbert]

* Amit Shah (amit.shah@redhat.com) wrote:
> On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote:
> > >>>> The static variables in migration_bitmap_sync will not be reset in
> > >>>> the case of a second attempted migration.
> > >>>>
> > >>>> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> > >>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > >>>
> > >>> Good catch.  Applied..
> > >>>
> > >>
> > >> Hi, Juan? Ping... please :)
> > > 
> > > Juan, what happened to this patch?
> >
> > Nearly for half a year, I forgot it completely :(
> > Thanks for your prompt, Paolo.
> 
> Yeah; unfortunate.
> 
> I feel the patch could've been done in a better way, but since it's
> been a while...
> 
> Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2.

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Note that there are some more statics that have ended up in migration_bitmap_sync
since that patch landed, but still it would be good to get this fix going.

Dave

> 
> Thanks,
> 
> 		Amit
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Amit Shah]

On (Thu) 20 Nov 2014 [12:35:54], Dr. David Alan Gilbert wrote:
> * Amit Shah (amit.shah@redhat.com) wrote:
> > On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote:
> > > >>>> The static variables in migration_bitmap_sync will not be reset in
> > > >>>> the case of a second attempted migration.
> > > >>>>
> > > >>>> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> > > >>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > >>>
> > > >>> Good catch.  Applied..
> > > >>>
> > > >>
> > > >> Hi, Juan? Ping... please :)
> > > > 
> > > > Juan, what happened to this patch?
> > >
> > > Nearly for half a year, I forgot it completely :(
> > > Thanks for your prompt, Paolo.
> > 
> > Yeah; unfortunate.
> > 
> > I feel the patch could've been done in a better way, but since it's
> > been a while...
> > 
> > Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2.
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Thanks; pull req sent.

		Amit

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Gonglei]

On 2014/11/20 21:00, Amit Shah wrote:

> On (Thu) 20 Nov 2014 [12:35:54], Dr. David Alan Gilbert wrote:
>> * Amit Shah (amit.shah@redhat.com) wrote:
>>> On (Thu) 20 Nov 2014 [19:39:11], Gonglei wrote:
>>>>>>>> The static variables in migration_bitmap_sync will not be reset in
>>>>>>>> the case of a second attempted migration.
>>>>>>>>
>>>>>>>> Signed-off-by: ChenLiang <chenliang88@huawei.com>
>>>>>>>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
>>>>>>>
>>>>>>> Good catch.  Applied..
>>>>>>>
>>>>>>
>>>>>> Hi, Juan? Ping... please :)
>>>>>
>>>>> Juan, what happened to this patch?
>>>>
>>>> Nearly for half a year, I forgot it completely :(
>>>> Thanks for your prompt, Paolo.
>>>
>>> Yeah; unfortunate.
>>>
>>> I feel the patch could've been done in a better way, but since it's
>>> been a while...
>>>
>>> Paolo, Dave, can you please give me r-b and I'll pick it up for 2.2.
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> Thanks; pull req sent.
> 
> 		Amit

Thanks :)

Best regards,
-Gonglei

Re: [Qemu-devel] [PATCH] migration: static variables will not be reset at second migration

[Dr. David Alan Gilbert]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 
> 
> On 24/06/2014 08:23, Gonglei (Arei) wrote:
> >> -----Original Message-----
> >> From: Juan Quintela [mailto:quintela@redhat.com]
> >> Sent: Friday, March 21, 2014 9:26 PM
> >> To: Gonglei (Arei)
> >> Cc: qemu-devel@nongnu.org; owasserm@redhat.com; pbonzini@redhat.com;
> >> eblake@redhat.com; dgilbert@redhat.com; chenliang (T)
> >> Subject: Re: [PATCH] migration: static variables will not be reset at second
> >> migration
> >>
> >> <arei.gonglei@huawei.com> wrote:
> >>> From: ChenLiang <chenliang88@huawei.com>
> >>>
> >>> The static variables in migration_bitmap_sync will not be reset in
> >>> the case of a second attempted migration.
> >>>
> >>> Signed-off-by: ChenLiang <chenliang88@huawei.com>
> >>> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> >>
> >> Good catch.  Applied..
> >>
> > 
> > Hi, Juan? Ping... please :)
> 
> Juan, what happened to this patch?

I think we should put this in now; it's obvious it was intended
to go in.

Dave

> 
> Paolo
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK