Re: [PATCH V1 02/26] migration: lower handler priority

[Peter Xu <peterx@redhat.com>]

On Wed, Jan 29, 2025 at 06:42:58AM -0800, Steve Sistare wrote:
> Define a vmstate priority that is lower than the default, so its handlers
> run after all default priority handlers.  Since 0 is no longer the default
> priority, translate an uninitialized priority of 0 to MIG_PRI_DEFAULT.
> 
> CPR for vfio will use this to install handlers for containers that run
> after handlers for the devices that they contain.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>  include/migration/vmstate.h | 3 ++-
>  migration/savevm.c          | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index a1dfab4..3055a46 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -155,7 +155,8 @@ enum VMStateFlags {
>  };
>  
>  typedef enum {
> -    MIG_PRI_DEFAULT = 0,

Shall we still keep a defintion for 0?  Or at least add a comment link to
save_state_priority() - it might be helpful for whoever jumps to this enum
defintion when reading.. and get confused how a default value is non-zero.

Or define it as something like:

       MIG_PRI_UNINITIALIZED = 0,  /* Most devices don't set a priority, it will
                                    * be routed to MIG_PRI_DEFAULT */

> +    MIG_PRI_LOW = 1,            /* Must happen after default */
> +    MIG_PRI_DEFAULT,
>      MIG_PRI_IOMMU,              /* Must happen before PCI devices */
>      MIG_PRI_PCI_BUS,            /* Must happen before IOMMU */
>      MIG_PRI_VIRTIO_MEM,         /* Must happen before IOMMU */
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 264bc06..5dd2dc4 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -232,7 +232,7 @@ typedef struct SaveState {
>  
>  static SaveState savevm_state = {
>      .handlers = QTAILQ_HEAD_INITIALIZER(savevm_state.handlers),
> -    .handler_pri_head = { [MIG_PRI_DEFAULT ... MIG_PRI_MAX] = NULL },
> +    .handler_pri_head = { [0 ... MIG_PRI_MAX] = NULL },
>      .global_section_id = 0,
>  };
>  
> @@ -704,7 +704,7 @@ static int calculate_compat_instance_id(const char *idstr)
>  
>  static inline MigrationPriority save_state_priority(SaveStateEntry *se)
>  {
> -    if (se->vmsd) {
> +    if (se->vmsd && se->vmsd->priority) {
>          return se->vmsd->priority;
>      }
>      return MIG_PRI_DEFAULT;
> -- 
> 1.8.3.1
> 

-- 
Peter Xu