Re: [Qemu-ppc] [QEMU RFC PATCH v2 4/6] Migration: migrate QTAILQ

[Paolo Bonzini]

On 24/05/2016 19:55, Jianjun Duan wrote:
> +/*
> + * Offsets of layout of a tail queue head.
> + */
> +#define QTAILQ_FIRST_OFFSET(head_type) \
> +        ((size_t) ((char *) &((head_type *)0)->tqh_first - (char *)0))
> +
> +#define QTAILQ_LAST_OFFSET(head_type) \
> +        ((size_t) ((char *) &((head_type *)0)->tqh_last - (char *)0))
> +
> +/*
> + * Offsets of layout of a tail queue element.
> + */
> +#define QTAILQ_NEXT_OFFSET(ele_type, field)                            \
> +        ((size_t) ((char *) &((ele_type *)0)->field.tqe_next -         \
> +                   (char *) &((ele_type *)0)->field))
> +
> +#define QTAILQ_PREV_OFFSET(ele_type, field) \
> +        ((size_t) ((char *) &((ele_type *)0)->field.tqe_prev -         \
> +                   (char *) &((ele_type *)0)->field))

Please use offsetof.

> +    /*
> +     * Following 3 fields are for VMStateField which needs customized 
> handling,
> +     * such as QTAILQ in qemu/queue.h, lists, and tree.
> +     */
> +    const void *meta_data;
> +    int (*extend_get)(QEMUFile *f, const void *metadata, void *opaque);
> +    void (*extend_put)(QEMUFile *f, const void *metadata, void *opaque,
> +                       QJSON *vmdesc);
>  } VMStateField;

Do not add these two function pointers to VMStateField, instead add
QJSON* and VMStateField* arguments as needed to VMStateInfo's get and put.

> +#define VMSTATE_QTAILQ_METADATA(_field, _state, _type, _next, _vmsd) { \
> +    .first = QTAILQ_FIRST_OFFSET(typeof_field(_state, _field)),        \
> +    .last =  QTAILQ_LAST_OFFSET(typeof_field(_state, _field)),         \
> +    .next = QTAILQ_NEXT_OFFSET(_type, _next),                          \
> +    .prev = QTAILQ_PREV_OFFSET(_type, _next),                          \
> +    .entry = offsetof(_type, _next),                                   \
> +    .size = sizeof(_type),                                             \
> +    .vmsd = &(_vmsd),                                                  \
> +}

.last and .prev are unnecessary, since they come just after .first and
.next (and their use is hidden behind QTAILQ_RAW_*).  .first and .next
can be placed in .offset and .num_offset respectively.  So you don't
really need an extra metadata struct.

If you prefer you could have something like

    union {
        size_t num_offset;    /* VMS_VARRAY */
        size_t size_offset;   /* VMS_VBUFFER */
        size_t next_offset;   /* VMS_TAILQ */
    } offsets;

Thanks,

Paolo