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

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

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

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