Re: [RFC V1 05/14] migration: init and listen during precreate

[Peter Xu <peterx@redhat.com>]

On Thu, Oct 17, 2024 at 08:14:06AM -0700, Steve Sistare wrote:
> Initialize the migration object as early as possible so that migration
> configuration commands may be sent during the precreate phase.  Also,
> start listening for the incoming migration connection during precreate,
> so that the listen port number is assigned (if dynamic), and the user
> can discover it during precreate via query-migrate.  The precreate phase
> will be delineated in a subsequent patch.
> 
> The code previously called migration_object_init after memory backends
> were created so that a subsequent migrate-set-capabilities call to set
> MIGRATION_CAPABILITY_POSTCOPY_RAM would verify all backends support
> postcopy.  See migrate_caps_check and postcopy_ram_supported_by_host.
> The new code calls migration_object_init before backends are created.
> However, migrate-set-capabilities will only be received during the
> precreate phase for CPR, and CPR does not support postcopy.  If the
> precreate phase is generalized in the future, then the ram compatibility
> check must be deferred to the start of migration.

This makes sense to me on its own, so maybe we can have a seperate patch.

We should probably always do the check at start of migration, to avoid
postcopy-ram cap set followed by an memory plug which doesn't support
postcopy.

> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  system/vl.c | 35 +++++++++++++----------------------
>  1 file changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/system/vl.c b/system/vl.c
> index bca2292..d32203c 100644
> --- a/system/vl.c
> +++ b/system/vl.c
> @@ -2753,17 +2753,7 @@ void qmp_x_exit_preconfig(Error **errp)
>          replay_vmstate_init();
>      }
>  
> -    if (incoming) {
> -        Error *local_err = NULL;
> -        if (strcmp(incoming, "defer") != 0) {
> -            qmp_migrate_incoming(incoming, false, NULL, true, true,
> -                                 &local_err);
> -            if (local_err) {
> -                error_reportf_err(local_err, "-incoming %s: ", incoming);
> -                exit(1);
> -            }
> -        }
> -    } else if (autostart) {
> +    if (!incoming && autostart) {
>          qmp_cont(NULL);
>      }
>  }
> @@ -3751,6 +3741,18 @@ void qemu_init(int argc, char **argv)
>       * called from do_configure_accelerator().
>       */
>  
> +    /* Creates a QOM object */

Shall we still keep the ordering notes for global/compat properties to be
set before this one?  "create QOM object" on its own isn't much of help as
a comment if the function has a proper name..

> +    migration_object_init();
> +
> +    if (incoming && !g_str_equal(incoming, "defer")) {
> +        Error *local_err = NULL;
> +        qmp_migrate_incoming(incoming, false, NULL, true, true, &local_err);
> +        if (local_err) {
> +            error_reportf_err(local_err, "-incoming %s: ", incoming);
> +            exit(1);
> +        }
> +    }
> +
>      suspend_mux_open();
>  
>      qemu_disable_default_devices();
> @@ -3773,20 +3775,9 @@ void qemu_init(int argc, char **argv)
>                       machine_class->name, machine_class->deprecation_reason);
>      }
>  
> -    /*
> -     * Create backends before creating migration objects, so that it can
> -     * check against compatibilities on the backend memories (e.g. postcopy
> -     * over memory-backend-file objects).
> -     */

(so if there'll be a separate patch to delay postcopy ram check on src,
 this removal can be part of it)

>      qemu_create_late_backends();
>      phase_advance(PHASE_LATE_BACKENDS_CREATED);
>  
> -    /*
> -     * Note: creates a QOM object, must run only after global and
> -     * compat properties have been set up.
> -     */
> -    migration_object_init();
> -
>      /* parse features once if machine provides default cpu_type */
>      current_machine->cpu_type = machine_class_default_cpu_type(machine_class);
>      if (cpu_option) {
> -- 
> 1.8.3.1
> 

-- 
Peter Xu