On Mon, Nov 27, 2023 at 05:26:01PM -0300, Fabiano Rosas wrote:
> Some functionalities of multifd are incompatible with the 'fixed-ram'
> migration format.
>
> The MULTIFD_FLUSH flag in particular is not used because in fixed-ram
> there is no sinchronicity between migration source and destination so
> there is not need for a sync packet. In fact, fixed-ram disables
> packets in multifd as a whole.
>
> However, we still need to sync the migration thread with the multifd
> channels at key moments:
>
> - between iterations, to avoid a slow channel being overrun by a fast
> channel in the subsequent iteration;
>
> - at ram_save_complete, to make sure all data has been transferred
> before finishing migration;
[1]
>
> Make sure RAM_SAVE_FLAG_MULTIFD_FLUSH is only emitted for fixed-ram at
> those key moments.
>
> Signed-off-by: Fabiano Rosas <faro...@suse.de>
> ---
> migration/ram.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 08604222f2..ad6abd1761 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -1363,7 +1363,7 @@ static int find_dirty_block(RAMState *rs,
> PageSearchStatus *pss)
> pss->page = 0;
> pss->block = QLIST_NEXT_RCU(pss->block, next);
> if (!pss->block) {
> - if (migrate_multifd() &&
> + if (migrate_multifd() && !migrate_fixed_ram() &&
> !migrate_multifd_flush_after_each_section()) {
> QEMUFile *f = rs->pss[RAM_CHANNEL_PRECOPY].pss_channel;
> int ret = multifd_send_sync_main(f);
> @@ -3112,7 +3112,8 @@ static int ram_save_setup(QEMUFile *f, void *opaque)
> return ret;
> }
>
> - if (migrate_multifd() && !migrate_multifd_flush_after_each_section()) {
> + if (migrate_multifd() && !migrate_multifd_flush_after_each_section()
> + && !migrate_fixed_ram()) {
> qemu_put_be64(f, RAM_SAVE_FLAG_MULTIFD_FLUSH);
> }
>
> @@ -3242,8 +3243,11 @@ static int ram_save_iterate(QEMUFile *f, void *opaque)
> out:
> if (ret >= 0
> && migration_is_setup_or_active(migrate_get_current()->state)) {
> - if (migrate_multifd() && migrate_multifd_flush_after_each_section())
> {
> - ret =
> multifd_send_sync_main(rs->pss[RAM_CHANNEL_PRECOPY].pss_channel);
> + if (migrate_multifd() &&
> + (migrate_multifd_flush_after_each_section() ||
> + migrate_fixed_ram())) {
> + ret = multifd_send_sync_main(
> + rs->pss[RAM_CHANNEL_PRECOPY].pss_channel);
Why you want this one? ram_save_iterate() can be called tens for each
second iiuc.
There's one more? ram_save_complete():
if (migrate_multifd() && !migrate_multifd_flush_after_each_section()) {
qemu_put_be64(f, RAM_SAVE_FLAG_MULTIFD_FLUSH);
}
IIUC that's the one you referred to at [1] above, not sure why you modified
the code in ram_save_iterate() instead.
> if (ret < 0) {
> return ret;
> }
> --
> 2.35.3
>
Since the file migration added its whole new code in
multifd_send_sync_main(), now I'm hesitating whether we should just provide
multifd_file_sync_threads(), put file sync there, and call explicitly,
like:
if (migrate_multifd()) {
if (migrate_is_file()) {
multifd_file_sync_threads();
} else if (migrate_multifd_flush_after_each_section()) {
multifd_send_sync_main();
}
}
It'll be much clearer that file goes into its own path and we don't need to
worry on fat eyes of those if clauses. diff should be similar.
--
Peter Xu
