1. Patchew
  2. QEMU
  3. [PULL 00/23] Next patches
  4. View patch

[PULL 13/23] migration: Calculate expected downtime on demand

Peter Xu posted 23 patches 4 months, 1 week ago
Maintainers: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Weiwei Li <liwei1518@gmail.com>, Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Chao Liu <chao.liu.zevorn@gmail.com>, Halil Pasic <pasic@linux.ibm.com>, Christian Borntraeger <borntraeger@linux.ibm.com>, Jason Herne <jjherne@linux.ibm.com>, Cornelia Huck <cohuck@redhat.com>, Eric Farman <farman@linux.ibm.com>, Matthew Rosato <mjrosato@linux.ibm.com>, Richard Henderson <richard.henderson@linaro.org>, Ilya Leoshkevich <iii@linux.ibm.com>, David Hildenbrand <david@kernel.org>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, Max Filippov <jcmvbkbc@gmail.com>, Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>, John Snow <jsnow@redhat.com>, Li Zhijian <lizhijian@fujitsu.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Laurent Vivier <lvivier@redhat.com>
There is a newer version of this series
[PULL 13/23] migration: Calculate expected downtime on demand
Posted by Peter Xu 4 months, 1 week ago 
This value does not need to be calculated as frequent.  Only calculate it
on demand when query-migrate happened.  With that we can remove the
variable in MigrationState.

This paves way for fixing this value to include all modules (not only RAM
but others too).

Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
Link: https://lore.kernel.org/r/20260421202110.306051-12-peterx@redhat.com
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.h |  2 +-
 migration/migration.c | 25 ++++++++++++-------------
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/migration/migration.h b/migration/migration.h
index 9081e6a612..a5e064a1ac 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -359,7 +359,6 @@ struct MigrationState {
     /* Timestamp when VM is down (ms) to migrate the last stuff */
     int64_t downtime_start;
     int64_t downtime;
-    int64_t expected_downtime;
     bool capabilities[MIGRATION_CAPABILITY__MAX];
     int64_t setup_time;
 
@@ -585,6 +584,7 @@ void migration_cancel(void);
 void migration_populate_vfio_info(MigrationInfo *info);
 void migration_reset_vfio_bytes_transferred(void);
 void postcopy_temp_page_reset(PostcopyTmpPage *tmp_page);
+int64_t migration_downtime_calc_expected(MigrationState *s);
 
 /*
  * Migration thread waiting for return path thread.  Return non-zero if an
diff --git a/migration/migration.c b/migration/migration.c
index 4e19fe3409..d740d9df85 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1041,6 +1041,17 @@ static bool migrate_show_downtime(MigrationState *s)
     return (s->state == MIGRATION_STATUS_COMPLETED) || migration_in_postcopy();
 }
 
+/* Return expected downtime (unit: milliseconds) */
+int64_t migration_downtime_calc_expected(MigrationState *s)
+{
+    if (mig_stats.dirty_sync_count <= 1) {
+        return migrate_downtime_limit();
+    }
+
+    return mig_stats.dirty_bytes_last_sync /
+        migration_get_switchover_bw(s) * 1000;
+}
+
 static void populate_time_info(MigrationInfo *info, MigrationState *s)
 {
     info->has_status = true;
@@ -1061,7 +1072,7 @@ static void populate_time_info(MigrationInfo *info, MigrationState *s)
         info->downtime = s->downtime;
     } else {
         info->has_expected_downtime = true;
-        info->expected_downtime = s->expected_downtime;
+        info->expected_downtime = migration_downtime_calc_expected(s);
     }
 }
 
@@ -1649,7 +1660,6 @@ int migrate_init(MigrationState *s, Error **errp)
     s->mbps = 0.0;
     s->pages_per_second = 0.0;
     s->downtime = 0;
-    s->expected_downtime = 0;
     s->setup_time = 0;
     s->start_postcopy = false;
     s->migration_thread_running = false;
@@ -3166,15 +3176,6 @@ static void migration_update_counters(MigrationState *s,
     s->pages_per_second = (double) transferred_pages /
                              (((double) time_spent / 1000.0));
 
-    /*
-     * if we haven't sent anything, we don't want to
-     * recalculate. 10000 is a small enough number for our purposes
-     */
-    if (mig_stats.dirty_pages_rate && transferred > 10000) {
-        s->expected_downtime =
-            mig_stats.dirty_bytes_last_sync / switchover_bw_per_ms;
-    }
-
     migration_rate_reset();
 
     update_iteration_initial_status(s);
@@ -3841,8 +3842,6 @@ void migration_start_outgoing(MigrationState *s)
     bool resume = (s->state == MIGRATION_STATUS_POSTCOPY_RECOVER_SETUP);
     int ret;
 
-    s->expected_downtime = migrate_downtime_limit();
-
     if (resume) {
         /* This is a resumed migration */
         rate_limit = migrate_max_postcopy_bandwidth();
-- 
2.53.0
Re: [PULL 13/23] migration: Calculate expected downtime on demand
Posted by Peter Maydell 4 months ago 
On Tue, 5 May 2026 at 21:29, Peter Xu <peterx@redhat.com> wrote:
>
> This value does not need to be calculated as frequent.  Only calculate it
> on demand when query-migrate happened.  With that we can remove the
> variable in MigrationState.
>
> This paves way for fixing this value to include all modules (not only RAM
> but others too).
>
> Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
> Link: https://lore.kernel.org/r/20260421202110.306051-12-peterx@redhat.com
> Signed-off-by: Peter Xu <peterx@redhat.com>

Hi; I'm seeing a clang undefined-behaviour sanitizer failure
in the code introduced in this change when running the
aarch64 migration-test via "make check" on an x86-64 host.

It seems to happen fairly reliably when I do a "make check -j20",
but not when I run the test on its own, so it's probably load dependent.

Here's the backtrace:

../../migration/migration.c:1051:12: runtime error: inf is outside the
range of representable values of type 'long'
    #0 0x57b49d635c0d in migration_downtime_calc_expected
/home/pm215/qemu/build/arm-clang/../../migration/migration.c:1051:12
    #1 0x57b49d63e860 in populate_time_info
/home/pm215/qemu/build/arm-clang/../../migration/migration.c:1075:35
    #2 0x57b49d63617e in fill_source_migration_info
/home/pm215/qemu/build/arm-clang/../../migration/migration.c:1184:9
    #3 0x57b49d63617e in qmp_query_migrate
/home/pm215/qemu/build/arm-clang/../../migration/migration.c:1264:5
    #4 0x57b49e4aed75 in qmp_marshal_query_migrate
/home/pm215/qemu/build/arm-clang/qapi/qapi-commands-migration.c:48:14
    #5 0x57b49e526814 in do_qmp_dispatch_bh
/home/pm215/qemu/build/arm-clang/../../qapi/qmp-dispatch.c:128:5
    #6 0x57b49e58c35a in aio_bh_call
/home/pm215/qemu/build/arm-clang/../../util/async.c:173:5
    #7 0x57b49e58c698 in aio_bh_poll
/home/pm215/qemu/build/arm-clang/../../util/async.c:220:13
    #8 0x57b49e542fc1 in aio_dispatch
/home/pm215/qemu/build/arm-clang/../../util/aio-posix.c:390:5
    #9 0x57b49e58f10a in aio_ctx_dispatch
/home/pm215/qemu/build/arm-clang/../../util/async.c:365:5
    #10 0x7c74a09b8584
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5d584) (BuildId:
116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #11 0x7c74a09b86cf in g_main_context_dispatch
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5d6cf) (BuildId:
116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
    #12 0x57b49e5901cb in glib_pollfds_poll
/home/pm215/qemu/build/arm-clang/../../util/main-loop.c:290:9
    #13 0x57b49e5901cb in os_host_main_loop_wait
/home/pm215/qemu/build/arm-clang/../../util/main-loop.c:313:5
    #14 0x57b49e5901cb in main_loop_wait
/home/pm215/qemu/build/arm-clang/../../util/main-loop.c:592:11
    #15 0x57b49d5f5486 in qemu_main_loop
/home/pm215/qemu/build/arm-clang/../../system/runstate.c:948:9
    #16 0x57b49e42cdfb in qemu_default_main
/home/pm215/qemu/build/arm-clang/../../system/main.c:50:14
    #17 0x57b49e42cdd3 in main
/home/pm215/qemu/build/arm-clang/../../system/main.c:93:9


> +/* Return expected downtime (unit: milliseconds) */
> +int64_t migration_downtime_calc_expected(MigrationState *s)
> +{
> +    if (mig_stats.dirty_sync_count <= 1) {
> +        return migrate_downtime_limit();
> +    }
> +
> +    return mig_stats.dirty_bytes_last_sync /
> +        migration_get_switchover_bw(s) * 1000;
> +}

Presumably in this function migration_get_switchover_bw() returns 0,
so the (floating-point) division results in Infinity. That's fine
until we have to convert it to int64_t to return it, which is the
UB that the sanitizer is complaining about...

thanks
-- PMM
Re: [PULL 13/23] migration: Calculate expected downtime on demand
Posted by Peter Xu 4 months ago 
On Thu, May 07, 2026 at 08:57:24PM +0100, Peter Maydell wrote:
> On Tue, 5 May 2026 at 21:29, Peter Xu <peterx@redhat.com> wrote:
> >
> > This value does not need to be calculated as frequent.  Only calculate it
> > on demand when query-migrate happened.  With that we can remove the
> > variable in MigrationState.
> >
> > This paves way for fixing this value to include all modules (not only RAM
> > but others too).
> >
> > Reviewed-by: Juraj Marcin <jmarcin@redhat.com>
> > Link: https://lore.kernel.org/r/20260421202110.306051-12-peterx@redhat.com
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> 
> Hi; I'm seeing a clang undefined-behaviour sanitizer failure
> in the code introduced in this change when running the
> aarch64 migration-test via "make check" on an x86-64 host.
> 
> It seems to happen fairly reliably when I do a "make check -j20",
> but not when I run the test on its own, so it's probably load dependent.
> 
> Here's the backtrace:
> 
> ../../migration/migration.c:1051:12: runtime error: inf is outside the
> range of representable values of type 'long'
>     #0 0x57b49d635c0d in migration_downtime_calc_expected
> /home/pm215/qemu/build/arm-clang/../../migration/migration.c:1051:12
>     #1 0x57b49d63e860 in populate_time_info
> /home/pm215/qemu/build/arm-clang/../../migration/migration.c:1075:35
>     #2 0x57b49d63617e in fill_source_migration_info
> /home/pm215/qemu/build/arm-clang/../../migration/migration.c:1184:9
>     #3 0x57b49d63617e in qmp_query_migrate
> /home/pm215/qemu/build/arm-clang/../../migration/migration.c:1264:5
>     #4 0x57b49e4aed75 in qmp_marshal_query_migrate
> /home/pm215/qemu/build/arm-clang/qapi/qapi-commands-migration.c:48:14
>     #5 0x57b49e526814 in do_qmp_dispatch_bh
> /home/pm215/qemu/build/arm-clang/../../qapi/qmp-dispatch.c:128:5
>     #6 0x57b49e58c35a in aio_bh_call
> /home/pm215/qemu/build/arm-clang/../../util/async.c:173:5
>     #7 0x57b49e58c698 in aio_bh_poll
> /home/pm215/qemu/build/arm-clang/../../util/async.c:220:13
>     #8 0x57b49e542fc1 in aio_dispatch
> /home/pm215/qemu/build/arm-clang/../../util/aio-posix.c:390:5
>     #9 0x57b49e58f10a in aio_ctx_dispatch
> /home/pm215/qemu/build/arm-clang/../../util/async.c:365:5
>     #10 0x7c74a09b8584
> (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5d584) (BuildId:
> 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>     #11 0x7c74a09b86cf in g_main_context_dispatch
> (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5d6cf) (BuildId:
> 116e142b9b52c8a4dfd403e759e71ab8f95d8bb3)
>     #12 0x57b49e5901cb in glib_pollfds_poll
> /home/pm215/qemu/build/arm-clang/../../util/main-loop.c:290:9
>     #13 0x57b49e5901cb in os_host_main_loop_wait
> /home/pm215/qemu/build/arm-clang/../../util/main-loop.c:313:5
>     #14 0x57b49e5901cb in main_loop_wait
> /home/pm215/qemu/build/arm-clang/../../util/main-loop.c:592:11
>     #15 0x57b49d5f5486 in qemu_main_loop
> /home/pm215/qemu/build/arm-clang/../../system/runstate.c:948:9
>     #16 0x57b49e42cdfb in qemu_default_main
> /home/pm215/qemu/build/arm-clang/../../system/main.c:50:14
>     #17 0x57b49e42cdd3 in main
> /home/pm215/qemu/build/arm-clang/../../system/main.c:93:9
> 
> 
> > +/* Return expected downtime (unit: milliseconds) */
> > +int64_t migration_downtime_calc_expected(MigrationState *s)
> > +{
> > +    if (mig_stats.dirty_sync_count <= 1) {
> > +        return migrate_downtime_limit();
> > +    }
> > +
> > +    return mig_stats.dirty_bytes_last_sync /
> > +        migration_get_switchover_bw(s) * 1000;
> > +}
> 
> Presumably in this function migration_get_switchover_bw() returns 0,
> so the (floating-point) division results in Infinity. That's fine
> until we have to convert it to int64_t to return it, which is the
> UB that the sanitizer is complaining about...

True, I can easily reproduce the warning too.  I'll send a patch.

Thanks,

-- 
Peter Xu

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.