[Qemu-devel] [PATCH] migration: Fix postcopy bw for recovery

[Qemu-devel] [PATCH] migration: Fix postcopy bw for recovery

[Peter Xu]

We've got max-postcopy-bandwidth parameter but it's not applied
correctly after a postcopy recovery so the recovered migration stream
will still eat the whole net bandwidth.  Fix that up.

Reported-by: Xiaohui Li <xiaohli@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/migration.c b/migration/migration.c
index 8b9f2fe30a..b307813aa3 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -3327,7 +3327,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
 
     if (resume) {
         /* This is a resumed migration */
-        rate_limit = INT64_MAX;
+        rate_limit = s->parameters.max_postcopy_bandwidth /
+            XFER_LIMIT_RATIO;
     } else {
         /* This is a fresh new migration */
         rate_limit = s->parameters.max_bandwidth / XFER_LIMIT_RATIO;
-- 
2.21.0

Re: [Qemu-devel] [PATCH] migration: Fix postcopy bw for recovery

[Juan Quintela]

Peter Xu <peterx@redhat.com> wrote:
> We've got max-postcopy-bandwidth parameter but it's not applied
> correctly after a postcopy recovery so the recovered migration stream
> will still eat the whole net bandwidth.  Fix that up.
>
> Reported-by: Xiaohui Li <xiaohli@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Juan Quintela <quintela@redhat.com>

> diff --git a/migration/migration.c b/migration/migration.c
> index 8b9f2fe30a..b307813aa3 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3327,7 +3327,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>  
>      if (resume) {
>          /* This is a resumed migration */
> -        rate_limit = INT64_MAX;
> +        rate_limit = s->parameters.max_postcopy_bandwidth /
> +            XFER_LIMIT_RATIO;
>      } else {
>          /* This is a fresh new migration */
>          rate_limit = s->parameters.max_bandwidth / XFER_LIMIT_RATIO;

I was confused thinking that the two assignations were the same O:-)

Re: [Qemu-devel] [PATCH] migration: Fix postcopy bw for recovery

[Dr. David Alan Gilbert]

* Peter Xu (peterx@redhat.com) wrote:
> We've got max-postcopy-bandwidth parameter but it's not applied
> correctly after a postcopy recovery so the recovered migration stream
> will still eat the whole net bandwidth.  Fix that up.
> 
> Reported-by: Xiaohui Li <xiaohli@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>

Queued

> ---
>  migration/migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 8b9f2fe30a..b307813aa3 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3327,7 +3327,8 @@ void migrate_fd_connect(MigrationState *s, Error *error_in)
>  
>      if (resume) {
>          /* This is a resumed migration */
> -        rate_limit = INT64_MAX;
> +        rate_limit = s->parameters.max_postcopy_bandwidth /
> +            XFER_LIMIT_RATIO;
>      } else {
>          /* This is a fresh new migration */
>          rate_limit = s->parameters.max_bandwidth / XFER_LIMIT_RATIO;
> -- 
> 2.21.0
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] migration: Fix postcopy bw for recovery

[Markus Armbruster]

migration-tests hangs intermittently for me, and git bisect led me
here.  Test script:

i=0; while true; do let i++; echo "= $i ="; MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/migration-test -p /x86_64/migration/postcopy/recovery -k; done

Re: [Qemu-devel] [PATCH] migration: Fix postcopy bw for recovery

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> migration-tests hangs intermittently for me, and git bisect led me
> here.  Test script:
> 
> i=0; while true; do let i++; echo "= $i ="; MALLOC_PERTURB_=${MALLOC_PERTURB_:-$(( ${RANDOM:-0} % 255 + 1))}  QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 QTEST_QEMU_IMG=qemu-img tests/migration-test -p /x86_64/migration/postcopy/recovery -k; done

Which is fixed by the patch I posted yesterday:

migration/postcopy: Recognise the recovery states as 'in_postcopy'

Good working finding it using a bisect - it was way too unreliable
for me to find repeatably.

The 'Fix postcopy bw for recovery' patch is actually right - it just
exposes another existing race.

Dave

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