Re: [Qemu-devel] [PATCH] augment info migrate with page status

[Dor Laor <dlaor@redhat.com>]

Glauber Costa wrote:
> This patch augments info migrate output with status about:
> * pages remaining
> * pages transferred
>
> This should be enough for management tools to realize
> whether or not there is progress in migration. We can
> add more information later on, if the need arrives
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  migration.c |    2 ++
>  sysemu.h    |    6 ++++++
>  vl.c        |   13 +++++++++++--
>  3 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index b9e3368..88d63c0 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -116,6 +116,8 @@ void do_info_migrate(Monitor *mon)
>          switch (s->get_status(s)) {
>          case MIG_STATE_ACTIVE:
>              monitor_printf(mon, "active\n");
> +            monitor_printf(mon, "remaining ram pages: %u\n", ram_save_remaining());
> +            monitor_printf(mon, "transferred ram pages: %u\n", ram_save_transferred());
>              break;
>          case MIG_STATE_COMPLETED:
>              monitor_printf(mon, "completed\n");
> diff --git a/sysemu.h b/sysemu.h
> index 7d65804..76ab84b 100644
> --- a/sysemu.h
> +++ b/sysemu.h
> @@ -28,6 +28,12 @@ void qemu_del_vm_change_state_handler(VMChangeStateEntry *e);
>  void vm_start(void);
>  void vm_stop(int reason);
>  
> +#ifdef NEED_CPU_H
> +#include "cpu-defs.h"
> +ram_addr_t ram_save_remaining(void);
> +ram_addr_t ram_save_transferred(void);
> +#endif
> +
>  int64_t cpu_get_ticks(void);
>  void cpu_enable_ticks(void);
>  void cpu_disable_ticks(void);
> diff --git a/vl.c b/vl.c
> index 9f25cd4..97a1bc6 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3236,8 +3236,9 @@ static int ram_save_block(QEMUFile *f)
>  }
>  
>  static ram_addr_t ram_save_threshold = 10;
> +static ram_addr_t pages_transferred = 0;
>   

It would be nice to zero pages_transferred each migration operation.
ram_save_threshold is really to small. From Uri's past measurements, as 
value of 50 is a
better suite. Alternately it can be parametrized by the monitor command.

In general there is small drawback in the current approach:
The way bandwidth is capped,  iirc,  in every second you start consuming 
migration
bandwidth. If the bandwidth allocation was consumed after 100msec, 
you'll wait 900msec.
In this period, mgmt app reading the ram_save_remaining will notice that 
migration does
not progress and might either increase bandwidth or stop the guest.
That's why #of no-progress-iteration has advantage.

>  
> -static ram_addr_t ram_save_remaining(void)
> +ram_addr_t ram_save_remaining(void)
>  {
>      ram_addr_t addr;
>      ram_addr_t count = 0;
> @@ -3250,6 +3251,11 @@ static ram_addr_t ram_save_remaining(void)
>      return count;
>  }
>  
> +ram_addr_t ram_save_transferred(void)
> +{
> +    return pages_transferred;
> +}
> +
>  static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>  {
>      ram_addr_t addr;
> @@ -3271,6 +3277,7 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>          int ret;
>  
>          ret = ram_save_block(f);
> +        pages_transferred += ret;
>          if (ret == 0) /* no more blocks */
>              break;
>      }
> @@ -3280,7 +3287,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>      if (stage == 3) {
>  
>          /* flush all remaining blocks regardless of rate limiting */
> -        while (ram_save_block(f) != 0);
> +        while (ram_save_block(f) != 0) {
> +            pages_transferred++;
> +        }
>          cpu_physical_memory_set_dirty_tracking(0);
>      }
>  
>