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

[Steven Sistare <steven.sistare@oracle.com>]

On 10/21/2024 5:05 PM, Fabiano Rosas wrote:
> Steve Sistare <steven.sistare@oracle.com> writes:
> 
>> 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.
>>
>> 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 */
>> +    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);
>> +        }
>> +    }
> 
> Doesn't this break preconfig? Now migrate_incoming happens without user
> input so there's no time to set migration options before incoming code
> starts using them.

This branch is never taken for preconfig, because preconfig requires defer:

qemu_validate_options()
     if (incoming && preconfig_requested && strcmp(incoming, "defer") != 0) {
         error_report("'preconfig' supports '-incoming defer' only");

- Steve

>> +
>>       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).
>> -     */
>>       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) {