Re: [Qemu-devel] [PATCH 1/1] migration: fix expected_downtime

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Denis V. Lunev (den@openvz.org) wrote:
> From: Igor Redko <redkoi@virtuozzo.com>
> 
> To get this estimation we must divide pending_size by bandwidth
> according to description of expected-downtime ("qmp-commands.hx:3246"):
>   "expected-downtime": only present while migration is active
>               total amount in ms for downtime that was calculated on
>               the last bitmap round (json-int)
> 
> Previous version was just wrong because dirty_bytes_rate and bandwidth
> are measured in Bytes/ms, so dividing first by second we get some
> dimensionless quantity.
> As it said in description above this value is showed during active
> migration phase and recalculated only after transferring all memory
> and if this process took more than 1 sec. So maybe just nobody noticed
> that bug.

While I agree the existing code looks wrong, I don't see how this is
any more correct.

 I think 'pending_size' is an estimate of the number of bytes left
to transfer, the intention being that most of those are transferred
prior to pausing the machine, if those are transferred before pausing
then they aren't part of the downtime.

It feels that:
   * If the guest wasn't dirtying pages, then you wouldn't have to
     pause the guest; if it was just dirtying them a little then you
     wouldn't have much to transfer after the pages you'd already
     sent; so if the guest dirty pages fast then the estimate should be
     larger; so 'dirty_bytes_rate' being on top of the fraction feels right.
     
   * If the bandwidth is higher then the estimate should be smaller; so
     'bandwidth' being on the bottom of the fraction feels right.

Dave

> Signed-off-by: Igor Redko <redkoi@virtuozzo.com>
> Reviewed-by: Anna Melekhova <annam@virtuozzo.com>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Juan Quintela <quintela@redhat.com>
> CC: Amit Shah <amit.shah@redhat.com>
> ---
>  migration/migration.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index 662e77e..d55d545 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -994,7 +994,7 @@ static void *migration_thread(void *opaque)
>              /* if we haven't sent anything, we don't want to recalculate
>                 10000 is a small enough number for our purposes */
>              if (s->dirty_bytes_rate && transferred_bytes > 10000) {
> -                s->expected_downtime = s->dirty_bytes_rate / bandwidth;
> +                s->expected_downtime = pending_size / bandwidth;
>              }
>  
>              qemu_file_reset_rate_limit(s->file);
> -- 
> 2.1.4
> 
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK