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

2025-02-06 Thread Steven Sistare 

On 2/3/2025 11:58 AM, Peter Xu wrote:

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 
---
  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 */


Sure, I'll add MIG_PRI_UNINITIALIZED.

- Steve


+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

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

2025-02-03 Thread Peter Xu 
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 
> ---
>  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

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

2025-02-03 Thread Fabiano Rosas 
Steve Sistare  writes:

> 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 

Reviewed-by: Fabiano Rosas

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

2025-01-29 Thread Steve Sistare 
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 
---
 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,
+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