[Qemu-devel] [PATCH] migration: fix divide-by-zero introduced by sleep time accounting

[Qemu-devel] [PATCH] migration: fix divide-by-zero introduced by sleep time accounting

[Anthony Liguori]

If current_time == (initial_time + sleep_time), since
sleep_time == BUFFER_DELAY, time_spent will be 0 resulting in a
divide-by-zero when computing bandwidth.

This was introduced by:

  commit 7161082c8d8cf167c508976887a0a63f4db92b51
  Author: Juan Quintela <quintela@redhat.com>
  Date:   Fri Feb 1 12:41:38 2013 +0100

      migration: don't account sleep time for calculating bandwidth

Cc: Juan Quintela <quintela@redhat.com>
Reported-by: Mike Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 migration.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/migration.c b/migration.c
index 3eb0fad..70ae7a9 100644
--- a/migration.c
+++ b/migration.c
@@ -539,7 +539,7 @@ static void *migration_thread(void *opaque)
             break;
         }
         current_time = qemu_get_clock_ms(rt_clock);
-        if (current_time >= initial_time + BUFFER_DELAY) {
+        if ((current_time - sleep_time) >= initial_time + BUFFER_DELAY) {
             uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
             uint64_t time_spent = current_time - initial_time - sleep_time;
             double bandwidth = transferred_bytes / time_spent;
-- 
1.8.0

Re: [Qemu-devel] [PATCH] migration: fix divide-by-zero introduced by sleep time accounting

[Anthony Liguori]

Anthony Liguori <aliguori@us.ibm.com> writes:

> If current_time == (initial_time + sleep_time), since
> sleep_time == BUFFER_DELAY, time_spent will be 0 resulting in a
> divide-by-zero when computing bandwidth.
>
> This was introduced by:
>
>   commit 7161082c8d8cf167c508976887a0a63f4db92b51
>   Author: Juan Quintela <quintela@redhat.com>
>   Date:   Fri Feb 1 12:41:38 2013 +0100
>
>       migration: don't account sleep time for calculating bandwidth
>
> Cc: Juan Quintela <quintela@redhat.com>
> Reported-by: Mike Roth <mdroth@linux.vnet.ibm.com>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

Nack

Mike reports that this breaks migration in a different way.  He's
sending a full revert of the offending commit.

Regards,

Anthony Liguori

> ---
>  migration.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/migration.c b/migration.c
> index 3eb0fad..70ae7a9 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -539,7 +539,7 @@ static void *migration_thread(void *opaque)
>              break;
>          }
>          current_time = qemu_get_clock_ms(rt_clock);
> -        if (current_time >= initial_time + BUFFER_DELAY) {
> +        if ((current_time - sleep_time) >= initial_time + BUFFER_DELAY) {
>              uint64_t transferred_bytes = qemu_ftell(s->file) - initial_bytes;
>              uint64_t time_spent = current_time - initial_time - sleep_time;
>              double bandwidth = transferred_bytes / time_spent;
> -- 
> 1.8.0