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. 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. Since in vmstate_load/put_state we don't have any type information about the QTAILQ instance, we cannot use offset() in QTAILQ_RAW_*** macros. May have to stick the constants there for first/last/next/prev in QTAILQ_RAW_***. Not sure if it will work for all arches. >> 3 At this moment, vmstate_load_state/vmstate_put_state couldn't handle a >> queue, or a list, or another recursive structure. To make it >> extensible, I think a metadata is needed. The idea is for any >> structure which needs special handling, customized metadata/put/get >> should provide enough flexibility to hack around. > > I think your solution is a bit overengineered. If the metadata can > fit in the VMStateField, you can use VMStateField. > > Thanks, > > Paolo > Thanks, Jianjun
