On Thu, Mar 31, 2022 at 11:08:42AM -0400, Peter Xu wrote:
> This variable, along with its helpers, is used to detect whether multiple
> channel will be supported for migration. In follow up patches, there'll be
> other capability that requires multi-channels. Hence move it outside multifd
> specific code and make it public. Meanwhile rename it from "multifd" to
> "multi_channels" to show its real meaning.
FWIW, I would generally suggest separating the rename from the code
movement into distinct patches.
>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> migration/migration.c | 22 +++++++++++++++++-----
> migration/migration.h | 3 +++
> migration/multifd.c | 19 ++++---------------
> migration/multifd.h | 2 --
> 4 files changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/migration/migration.c b/migration/migration.c
> index 281d33326b..596d3d30b4 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -180,6 +180,18 @@ static int migration_maybe_pause(MigrationState *s,
> int new_state);
> static void migrate_fd_cancel(MigrationState *s);
>
> +static bool migrate_allow_multi_channels = true;
This is a pre-existing thing, but I'm curious why we default this to
'true', when the first thing qemu_start_incoming_migration() and
qmp_migrate() do, is to set it to 'false' and then selectively
put it back to 'true'.
> static gint page_request_addr_cmp(gconstpointer ap, gconstpointer bp)
> {
> uintptr_t a = (uintptr_t) ap, b = (uintptr_t) bp;
> @@ -469,12 +481,12 @@ static void qemu_start_incoming_migration(const char
> *uri, Error **errp)
> {
> const char *p = NULL;
>
> - migrate_protocol_allow_multifd(false); /* reset it anyway */
> + migrate_protocol_allow_multi_channels(false); /* reset it anyway */
> qapi_event_send_migration(MIGRATION_STATUS_SETUP);
> if (strstart(uri, "tcp:", &p) ||
> strstart(uri, "unix:", NULL) ||
> strstart(uri, "vsock:", NULL)) {
> - migrate_protocol_allow_multifd(true);
> + migrate_protocol_allow_multi_channels(true);
> socket_start_incoming_migration(p ? p : uri, errp);
> @@ -2324,11 +2336,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool
> blk,
> }
> }
>
> - migrate_protocol_allow_multifd(false);
> + migrate_protocol_allow_multi_channels(false);
> if (strstart(uri, "tcp:", &p) ||
> strstart(uri, "unix:", NULL) ||
> strstart(uri, "vsock:", NULL)) {
> - migrate_protocol_allow_multifd(true);
> + migrate_protocol_allow_multi_channels(true);
> socket_start_outgoing_migration(s, p ? p : uri, &local_err);
> #ifdef CONFIG_RDMA
> } else if (strstart(uri, "rdma:", &p)) {
Regardless of comments above
Reviewed-by: Daniel P. Berrangé <berra...@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
