Re: [PATCH v4] migration: Support QLIST migration

[Peter Xu]

On Fri, Oct 18, 2019 at 11:21:36AM +0200, Eric Auger wrote:
> Support QLIST migration using the same principle as QTAILQ:
> 94869d5c52 ("migration: migrate QTAILQ").
> 
> The VMSTATE_QLIST_V macro has the same proto as VMSTATE_QTAILQ_V.
> The change mainly resides in QLIST RAW macros: QLIST_RAW_INSERT_HEAD
> and QLIST_RAW_REVERSE.
> 
> Tests also are provided.
> 
> Signed-off-by: Eric Auger <address@hidden>
> 
> ---
> 
> v3 -> v4:
> - replace QLIST_RAW_INSERT_TAIL by QLIST_RAW_INSERT_HEAD and
>   QLIST_RAW_REVERSE as suggested by Juan
> ---
>  include/migration/vmstate.h |  21 ++++++
>  include/qemu/queue.h        |  40 +++++++++++
>  migration/trace-events      |   5 ++
>  migration/vmstate-types.c   |  70 +++++++++++++++++++
>  tests/test-vmstate.c        | 133 ++++++++++++++++++++++++++++++++++++
>  5 files changed, 269 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index b9ee563aa4..ea2f1f4749 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -225,6 +225,7 @@ extern const VMStateInfo vmstate_info_tmp;
>  extern const VMStateInfo vmstate_info_bitmap;
>  extern const VMStateInfo vmstate_info_qtailq;
>  extern const VMStateInfo vmstate_info_gtree;
> +extern const VMStateInfo vmstate_info_qlist;
>  
>  #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
>  /*
> @@ -794,6 +795,26 @@ extern const VMStateInfo vmstate_info_gtree;
>      .offset       = offsetof(_state, _field),                                
>   \
>  }
>  
> +/*
> + * For migrating a QLIST
> + * Target QLIST needs be properly initialized.
> + * _type: type of QLIST element
> + * _next: name of QLIST_ENTRY entry field in QLIST element
> + * _vmsd: VMSD for QLIST element
> + * size: size of QLIST element
> + * start: offset of QLIST_ENTRY in QTAILQ element
> + */
> +#define VMSTATE_QLIST_V(_field, _state, _version, _vmsd, _type, _next)  \
> +{                                                                        \
> +    .name         = (stringify(_field)),                                 \
> +    .version_id   = (_version),                                          \
> +    .vmsd         = &(_vmsd),                                            \
> +    .size         = sizeof(_type),                                       \
> +    .info         = &vmstate_info_qlist,                                 \
> +    .offset       = offsetof(_state, _field),                            \
> +    .start        = offsetof(_type, _next),                              \
> +}
> +
>  /* _f : field name
>     _f_n : num of elements field_name
>     _n : num of elements
> diff --git a/include/qemu/queue.h b/include/qemu/queue.h
> index 73bf4a984d..cd8ad4f386 100644
> --- a/include/qemu/queue.h
> +++ b/include/qemu/queue.h
> @@ -491,4 +491,44 @@ union {                                                  
>                \
>          QTAILQ_RAW_TQH_CIRC(head)->tql_prev = QTAILQ_RAW_TQE_CIRC(elm, 
> entry);  \
>  } while (/*CONSTCOND*/0)
>  
> +#define QLIST_RAW_FIRST(head)                                                
>   \
> +        field_at_offset(head, 0, void *)
> +
> +#define QLIST_RAW_NEXT(elm, entry)                                           
>   \
> +        field_at_offset(elm, entry, void *)
> +
> +#define QLIST_RAW_PREVIOUS(elm, entry)                                       
>   \
> +        field_at_offset(elm, entry + sizeof(void *), void *)
> +
> +#define QLIST_RAW_FOREACH(elm, head, entry)                                  
>   \
> +        for ((elm) = *QLIST_RAW_FIRST(head);                                 
>   \
> +             (elm);                                                          
>   \
> +             (elm) = *QLIST_RAW_NEXT(elm, entry))
> +
> +#define QLIST_RAW_INSERT_HEAD(head, elm, entry) do {                         
>   \
> +        void *first = *QLIST_RAW_FIRST(head);                                
>   \
> +        *QLIST_RAW_FIRST(head) = elm;                                        
>   \
> +        *QLIST_RAW_PREVIOUS(elm, entry) = head;                              
>   \
> +        if (first) {                                                         
>   \
> +            *QLIST_RAW_PREVIOUS(first, entry) = first;                       
>   \
                                                   ^^^^^
                                         should this be "elm"?

> +            *QLIST_RAW_NEXT(elm, entry) = first;                             
>   \
> +        } else {                                                             
>   \
> +            *QLIST_RAW_NEXT(elm, entry) = NULL;                              
>   \
> +        }                                                                    
>   \
> +} while (0)
> +
> +#define QLIST_RAW_REVERSE(head, elm, entry) do {                             
>   \
> +        void *iter = *QLIST_RAW_FIRST(head), *prev = NULL, *next;            
>   \
> +        while (iter)                                                         
>   \
> +            {                                                                
>   \

(indent issue)

> +                next = *QLIST_RAW_NEXT(iter, entry);                         
>   \
> +                *QLIST_RAW_PREVIOUS(iter, entry) = next;                     
>   \
> +                *QLIST_RAW_NEXT(iter, entry) = prev;                         
>   \
> +                prev = iter;                                                 
>   \
> +                iter = next;                                                 
>   \
> +        }                                                                    
>   \
> +        *QLIST_RAW_FIRST(head) = prev;                                       
>   \
> +        *QLIST_RAW_PREVIOUS(prev, entry) = head;                             
>   \
> +} while (0)

[...]

Otherwise looks good to me.

Thanks,

-- 
Peter Xu