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

[Jianjun Duan]

On 05/26/2016 12:11 AM, Paolo Bonzini wrote:
> 
> 
> On 25/05/2016 22:17, Jianjun Duan wrote:
>>
>>
>> On 05/25/2016 12:22 PM, Paolo Bonzini wrote:
>>>> 1 QTAILQ should only be accessed using the interfaces defined in
>>>> queue.h. Its structs should not be directly used. So I created
>>>> interfaces in queue.h to query about its layout. If the implementation
>>>> is changed, these interfaces should be changed accordingly. Code using
>>>> these interfaces should not break.
>>>
>>> You don't need to query the layout, as long as the knowledge
>>> remains hidden in QTAILQ_RAW_* macros.  And because QTAILQ_*_OFFSET
>>> returns constant values, you can just put the knowledge of the offsets
>>> directly in QTAILQ_RAW_FOREACH and QTAILQ_RAW_INSERT_TAIL.
>>>
>>>> 2 Based on point 1, vmstate_load_state/vmstate_put_state in vmstate.c
>>>> doesn't exactly know the structs of QTAILAQ head and entry. So pointer
>>>> arithmetic is needed to put/get a QTAILQ. To do it, we need those 6
>>>> parameters to be passed in. So it is not redundant if we only want to
>>>> only use the interfaces.
>>>
>>> No, you only need two.  The other four are internal to qemu/queue.h.
>>> Just like QTAILQ users do not know about tqh_* and tqe_*, they need not
>>> know about their offsets, only the fields that contain them.
>>
>> In vmstate_load/put_state usually we use a VMSD to describe the type for
>> dump/load purpose. The metadata for the QTAILQ is for the same purpose.
>> Of course we can encode the information in a VMSD or VMStateField if
>> we don't have to change much.
> 
> A VMSD describes the "sub-structure" of the field.  Here, the
> substructure of a QTAILQ is the structure of each entry.
> 
>> The user may only care the position of head and entry. But to
>> implement QTAILQ_RAW_***, we need more offset information than that.
>> If we don't query the offsets using something like offset() and store
>> it in a metadata, we have to make the assumption that all the pointer
>> types have the same size.
> 
> We make this assumption elsewhere anyway.  _You_ are making it by doing
> loads and stores through void** in QTAILQ_RAW_*.
I thought every data pointer can be converted to and from void type of
pointer. Anyway we can make that assumption here.

How about alignment? Is it OK to assume pointers are always aligned at
4byte for 32 bit and 8byte for 64 bit? If yes then we can just add the
size of a void pointer to first to get last in QTAILQ head. We will get
something like:
first=0
last = sizeof(void *)
next=0
prev=sizeof(void *)

Thanks,
Jianjun



> 
> Thanks,
> 
> Paolo
>