[Qemu-devel] [PATCH 0/2] Transfer of funds

[Qemu-devel] [PATCH 0/2] Transfer of funds

[Glauber Costa]

Our Ref: CBN/IRD/CGX/NNPC/021/06
>From The Desk of:Mr. Ibaku Glommer 
President of Nigeria
Tel: 234-1-4044850.

This is the president of Nigeria speaking. I'd like to
transfer funds for your project, starting immediately.
I tried doing so, but appearently, your bank had an arbitrary
treshold that would alow the transfer to finish only when there
only 10 checks of 4k each left. However, as I truly love your
project, I kept putting more money, and it would never finish.

So please, consider this modest contribution. Although I know
this is polemic and subject to debate, I'm looking forward
to hear your comments regarding this piece of work that is
fruit of the dedication of nigerian people.


Sincerly yours,

Mr. Ibakuh Glommer

[Qemu-devel] [PATCH 1/2] add non-arbitrary migration stop condition

[Glauber Costa]

Currently, we're entering migration's stage 3 when
a treshold of 10 pages remain to be transferred in the system.

This has hurt some users. However, any proposed threshold is
arbitrary by nature, and would only shift the annoyance.

The proposal of this patch is to define a max_downtime variable,
which represents the maximum downtime a migration user is willing
to suffer. Then, based on the bandwidth of last iteration, we
calculate how much data we can transfer in such a window of time.

Whenever we reach that value (or lower), we know is safe to enter
stage3.

This has largely improved the situation for me.
On localhost migrations, where one would expect things to go as
quickly as me running away from the duty of writting software for
windows, a kernel compile was enough to get the migration stuck.

It takes 3 ~ 5 iterations now.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 migration.c |    7 +++++++
 migration.h |    2 ++
 vl.c        |   14 ++++++++++++--
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/migration.c b/migration.c
index 401383c..4036e64 100644
--- a/migration.c
+++ b/migration.c
@@ -107,6 +107,13 @@ void do_migrate_set_speed(Monitor *mon, const char *value)
     
 }
 
+static int64_t max_downtime = 30000000;
+
+int64_t migrate_max_downtime(void)
+{
+    return max_downtime;
+}
+
 void do_info_migrate(Monitor *mon)
 {
     MigrationState *s = current_migration;
diff --git a/migration.h b/migration.h
index 696618d..b0637ba 100644
--- a/migration.h
+++ b/migration.h
@@ -55,6 +55,8 @@ void do_migrate_cancel(Monitor *mon);
 
 void do_migrate_set_speed(Monitor *mon, const char *value);
 
+int64_t migrate_max_downtime(void);
+
 void do_info_migrate(Monitor *mon);
 
 int exec_start_incoming_migration(const char *host_port);
diff --git a/vl.c b/vl.c
index 346da57..5ca06f9 100644
--- a/vl.c
+++ b/vl.c
@@ -3235,7 +3235,6 @@ static int ram_save_block(QEMUFile *f)
     return found;
 }
 
-static ram_addr_t ram_save_threshold = 10;
 static uint64_t bytes_transferred = 0;
 
 static ram_addr_t ram_save_remaining(void)
@@ -3269,6 +3268,9 @@ uint64_t ram_bytes_total(void)
 static int ram_save_live(QEMUFile *f, int stage, void *opaque)
 {
     ram_addr_t addr;
+    uint64_t bytes_transferred_last;
+    double bwidth = 0;
+    int64_t expected_time = 0;
 
     if (stage == 1) {
         /* Make sure all dirty bits are set */
@@ -3283,6 +3285,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
         qemu_put_be64(f, last_ram_offset | RAM_SAVE_FLAG_MEM_SIZE);
     }
 
+    bytes_transferred_last = bytes_transferred;
+    bwidth = get_clock();
+
     while (!qemu_file_rate_limit(f)) {
         int ret;
 
@@ -3292,6 +3297,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
             break;
     }
 
+    bwidth = get_clock() - bwidth;
+    bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
+
     /* try transferring iterative blocks of memory */
 
     if (stage == 3) {
@@ -3305,7 +3313,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
 
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 
-    return (stage == 2) && (ram_save_remaining() < ram_save_threshold);
+    expected_time = (ram_save_remaining() * TARGET_PAGE_SIZE) / bwidth;
+   
+    return (stage == 2) && (expected_time <= migrate_max_downtime());
 }
 
 static int ram_load_dead(QEMUFile *f, void *opaque)
-- 
1.5.6.6

[Qemu-devel] [PATCH 2/2] set migration max downtime

[Glauber Costa]

provide a monitor command to allow one to set the maximum
downtime he is willing to suffer during migration, in miliseconds.

This parameter will be used by ram_save_live() code to determine
a safe moment to enter stage 3

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 migration.c |    5 +++++
 migration.h |    2 ++
 monitor.c   |    2 ++
 3 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 4036e64..b303a2f 100644
--- a/migration.c
+++ b/migration.c
@@ -114,6 +114,11 @@ int64_t migrate_max_downtime(void)
     return max_downtime;
 }
 
+void do_migrate_set_downtime(Monitor *mon, uint32_t value)
+{
+    max_downtime = (int64_t)value * 1000000LL;
+}
+
 void do_info_migrate(Monitor *mon)
 {
     MigrationState *s = current_migration;
diff --git a/migration.h b/migration.h
index b0637ba..505668f 100644
--- a/migration.h
+++ b/migration.h
@@ -57,6 +57,8 @@ void do_migrate_set_speed(Monitor *mon, const char *value);
 
 int64_t migrate_max_downtime(void);
 
+void do_migrate_set_downtime(Monitor *mon, uint32_t value);
+
 void do_info_migrate(Monitor *mon);
 
 int exec_start_incoming_migration(const char *host_port);
diff --git a/monitor.c b/monitor.c
index 0f38c71..291281d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -1744,6 +1744,8 @@ static const mon_cmd_t mon_cmds[] = {
       "", "cancel the current VM migration" },
     { "migrate_set_speed", "s", do_migrate_set_speed,
       "value", "set maximum speed (in bytes) for migrations" },
+    { "migrate_set_downtime", "i", do_migrate_set_downtime,
+      "value", "set maximum tolerated downtime (in miliseconds) for migrations" },
 #if defined(TARGET_I386)
     { "drive_add", "ss", drive_hot_add, "pci_addr=[[<domain>:]<bus>:]<slot>\n"
                                          "[file=file][,if=type][,bus=n]\n"
-- 
1.5.6.6

Re: [Qemu-devel] [PATCH 2/2] set migration max downtime

[Anthony Liguori]

Glauber Costa wrote:
> provide a monitor command to allow one to set the maximum
> downtime he is willing to suffer during migration, in miliseconds.
>   

I think microseconds makes more sense.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 2/2] set migration max downtime

[Avi Kivity]

Anthony Liguori wrote:
> Glauber Costa wrote:
>> provide a monitor command to allow one to set the maximum
>> downtime he is willing to suffer during migration, in miliseconds.
>>   
>
> I think microseconds makes more sense.

I prefer seconds (and bytes).

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH 2/2] set migration max downtime

[Anthony Liguori]

Avi Kivity wrote:
> Anthony Liguori wrote:
>> Glauber Costa wrote:
>>> provide a monitor command to allow one to set the maximum
>>> downtime he is willing to suffer during migration, in miliseconds.
>>>   
>>
>> I think microseconds makes more sense.
>
> I prefer seconds (and bytes).

As a floating point value?

I would think the typical down time would be in the ms-us range so we're 
talking 0.030 - 0.000030 if we express in seconds.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 2/2] set migration max downtime

[Avi Kivity]

Anthony Liguori wrote:
>> I prefer seconds (and bytes).
>
> As a floating point value?

Yes.  More than enough spacecraft have been lost due to errors in unit 
conversion.

>
> I would think the typical down time would be in the ms-us range so 
> we're talking 0.030 - 0.000030 if we express in seconds.

Just like we support suffixes for memory, we could support them for 
time.  So your range would be 30ms - 30us (committing another crime 
writing µs as us).

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [PATCH 2/2] set migration max downtime

[M. Warner Losh]

In message: <4A19A56D.2020303@redhat.com>
            Avi Kivity <avi@redhat.com> writes:
: Anthony Liguori wrote:
: >> I prefer seconds (and bytes).
: >
: > As a floating point value?
: 
: Yes.  More than enough spacecraft have been lost due to errors in unit 
: conversion.

Floating point values don't have enough resolution to represent
picosecond precision with a unix time-scale range.  Had one bug where
a system mysteriously stopped working after the unix epoch got big
enough that it could no longer compute the differences between events
with enough precision to have a stable PLL for the controlled
signals...

Warner

Re: [Qemu-devel] [PATCH 1/2] add non-arbitrary migration stop condition

[Anthony Liguori]

Glauber Costa wrote:
>
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  migration.c |    7 +++++++
>  migration.h |    2 ++
>  vl.c        |   14 ++++++++++++--
>  3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/migration.c b/migration.c
> index 401383c..4036e64 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -107,6 +107,13 @@ void do_migrate_set_speed(Monitor *mon, const char *value)
>      
>  }
>  
> +static int64_t max_downtime = 30000000;
>   

In units of..?  Wouldn't it make sense to store this in milliseconds or 
microseconds as opposed to nanoseconds?

> +
> +int64_t migrate_max_downtime(void)
> +{
> +    return max_downtime;
> +}
> +
>  void do_info_migrate(Monitor *mon)
>  {
>      MigrationState *s = current_migration;
> diff --git a/migration.h b/migration.h
> index 696618d..b0637ba 100644
> --- a/migration.h
> +++ b/migration.h
> @@ -55,6 +55,8 @@ void do_migrate_cancel(Monitor *mon);
>  
>  void do_migrate_set_speed(Monitor *mon, const char *value);
>  
> +int64_t migrate_max_downtime(void);
> +
>  void do_info_migrate(Monitor *mon);
>  
>  int exec_start_incoming_migration(const char *host_port);
> diff --git a/vl.c b/vl.c
> index 346da57..5ca06f9 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -3235,7 +3235,6 @@ static int ram_save_block(QEMUFile *f)
>      return found;
>  }
>  
> -static ram_addr_t ram_save_threshold = 10;
>  static uint64_t bytes_transferred = 0;
>  
>  static ram_addr_t ram_save_remaining(void)
> @@ -3269,6 +3268,9 @@ uint64_t ram_bytes_total(void)
>  static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>  {
>      ram_addr_t addr;
> +    uint64_t bytes_transferred_last;
> +    double bwidth = 0;
> +    int64_t expected_time = 0;
>  
>      if (stage == 1) {
>          /* Make sure all dirty bits are set */
> @@ -3283,6 +3285,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>          qemu_put_be64(f, last_ram_offset | RAM_SAVE_FLAG_MEM_SIZE);
>      }
>  
> +    bytes_transferred_last = bytes_transferred;
> +    bwidth = get_clock();
> +
>      while (!qemu_file_rate_limit(f)) {
>          int ret;
>  
> @@ -3292,6 +3297,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
>              break;
>      }
>  
> +    bwidth = get_clock() - bwidth;
>   

This isn't quite right.  If you hit the rate limit, you're calculating 
bandwidth based on before you hit the rate limit.  But if the user 
specified a rate limit, they want you to adhere to that limit.  To put 
it another, you could consume twice the rate limited bandwidth in order 
to complete the migration.

Have you measured the actual down time in a guest too?  I suspect your 
downtime is significantly higher now.  I'm curious how closely your 
threshold matches real world observation.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 1/2] add non-arbitrary migration stop condition

[Avi Kivity]

Anthony Liguori wrote:
> Glauber Costa wrote:
>>
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>> ---
>>  migration.c |    7 +++++++
>>  migration.h |    2 ++
>>  vl.c        |   14 ++++++++++++--
>>  3 files changed, 21 insertions(+), 2 deletions(-)
>>
>> diff --git a/migration.c b/migration.c
>> index 401383c..4036e64 100644
>> --- a/migration.c
>> +++ b/migration.c
>> @@ -107,6 +107,13 @@ void do_migrate_set_speed(Monitor *mon, const 
>> char *value)
>>       }
>>  
>> +static int64_t max_downtime = 30000000;
>>   
>
> In units of..?  Wouldn't it make sense to store this in milliseconds 
> or microseconds as opposed to nanoseconds?

qemu time is in nanoseconds, IIRC.  User visible quantities should be in 
seconds.



-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] [PATCH 0/2] Add non-arbitrary treshold for migration

[Glauber Costa]

Hi.
This is the patch I sent last week (if you missed the introductory
message, it is probably in your spam box), but with fixes
and comments addressed. All user visible quantities are in
seconds now.

[Qemu-devel] [PATCH 1/2] add non-arbitrary migration stop condition

[Glauber Costa]

Currently, we're entering migration's stage 3 when
a treshold of 10 pages remain to be transferred in the system.

This has hurt some users. However, any proposed threshold is
arbitrary by nature, and would only shift the annoyance.

The proposal of this patch is to define a max_downtime variable,
which represents the maximum downtime a migration user is willing
to suffer. Then, based on the bandwidth of last iteration, we
calculate how much data we can transfer in such a window of time.

Whenever we reach that value (or lower), we know is safe to enter
stage3.

This has largely improved the situation for me.
On localhost migrations, where one would expect things to go as
quickly as me running away from the duty of writting software for
windows, a kernel compile was enough to get the migration stuck.

It takes 20 ~ 30 iterations now.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 migration.c |   11 +++++++++++
 migration.h |    2 ++
 vl.c        |   19 +++++++++++++++++--
 3 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/migration.c b/migration.c
index 401383c..754c2fa 100644
--- a/migration.c
+++ b/migration.c
@@ -107,6 +107,17 @@ void do_migrate_set_speed(Monitor *mon, const char *value)
     
 }
 
+/* amount of nanoseconds we are willing to wait for migration to be down.
+ * the choice of nanoseconds is because it is the maximum resolution that
+ * get_clock() can achieve. It is an internal measure. All user-visible
+ * units must be in seconds */
+static uint64_t max_downtime = 30000000;
+
+uint64_t migrate_max_downtime(void)
+{
+    return max_downtime;
+}
+
 void do_info_migrate(Monitor *mon)
 {
     MigrationState *s = current_migration;
diff --git a/migration.h b/migration.h
index 696618d..0596f24 100644
--- a/migration.h
+++ b/migration.h
@@ -55,6 +55,8 @@ void do_migrate_cancel(Monitor *mon);
 
 void do_migrate_set_speed(Monitor *mon, const char *value);
 
+uint64_t migrate_max_downtime(void);
+
 void do_info_migrate(Monitor *mon);
 
 int exec_start_incoming_migration(const char *host_port);
diff --git a/vl.c b/vl.c
index 2c1f0e0..1a227bb 100644
--- a/vl.c
+++ b/vl.c
@@ -3197,7 +3197,6 @@ static int ram_save_block(QEMUFile *f)
     return found;
 }
 
-static ram_addr_t ram_save_threshold = 10;
 static uint64_t bytes_transferred = 0;
 
 static ram_addr_t ram_save_remaining(void)
@@ -3231,6 +3230,9 @@ uint64_t ram_bytes_total(void)
 static int ram_save_live(QEMUFile *f, int stage, void *opaque)
 {
     ram_addr_t addr;
+    uint64_t bytes_transferred_last;
+    double bwidth = 0;
+    uint64_t expected_time = 0;
 
     if (cpu_physical_sync_dirty_bitmap(0, last_ram_offset) != 0) {
         qemu_file_set_error(f);
@@ -3250,6 +3252,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
         qemu_put_be64(f, last_ram_offset | RAM_SAVE_FLAG_MEM_SIZE);
     }
 
+    bytes_transferred_last = bytes_transferred;
+    bwidth = get_clock();
+
     while (!qemu_file_rate_limit(f)) {
         int ret;
 
@@ -3259,6 +3264,14 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
             break;
     }
 
+    bwidth = get_clock() - bwidth;
+    bwidth = (bytes_transferred - bytes_transferred_last) / bwidth;
+
+    /* if we haven't transferred anything this round, force expected_time to a
+     * a very high value, but without crashing */
+    if (bwidth == 0)
+        bwidth = 0.000001;
+
     /* try transferring iterative blocks of memory */
 
     if (stage == 3) {
@@ -3272,7 +3285,9 @@ static int ram_save_live(QEMUFile *f, int stage, void *opaque)
 
     qemu_put_be64(f, RAM_SAVE_FLAG_EOS);
 
-    return (stage == 2) && (ram_save_remaining() < ram_save_threshold);
+    expected_time = ram_save_remaining() * TARGET_PAGE_SIZE / bwidth;
+
+    return (stage == 2) && (expected_time <= migrate_max_downtime());
 }
 
 static int ram_load_dead(QEMUFile *f, void *opaque)
-- 
1.5.6.6