diff mbox series

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

Message ID 1729178055-207271-6-git-send-email-steven.sistare@oracle.com
State New
Headers show
Series precreate phase | expand

Commit Message

Steven Sistare Oct. 17, 2024, 3:14 p.m. UTC 
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(-)

Comments

Peter Xu Oct. 21, 2024, 4:41 p.m. UTC | #1 
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
>
Fabiano Rosas Oct. 21, 2024, 9:05 p.m. UTC | #2 
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.

> +
>      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) {
Steven Sistare Oct. 23, 2024, 4:01 p.m. UTC | #3 
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) {
diff mbox series

Patch

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);
+        }
+    }
+
     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) {