* Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
> Hi Sascha,
>
> On 06/02/2016 08:01 AM, Sascha Silbe wrote:
> > Dear Jianjun,
> >
> > Jianjun Duan <du...@linux.vnet.ibm.com> writes:
> >
> > [include/migration/vmstate.h]
> >> @@ -185,6 +185,8 @@ enum VMStateFlags {
> >> * to determine the number of entries in the array. Only valid in
> >> * combination with one of VMS_VARRAY*. */
> >> VMS_MULTIPLY_ELEMENTS = 0x4000,
> >> + /* For fields which need customized handling, such as QTAILQ in
> >> queue.h*/
> >> + VMS_CSTM = 0x8000,
> >
> > Can you describe (in the comment) how this customised handling is
> > performed, please? I.e. describe what exactly happens if the flag is
> > set, from the point of view of an API consumer.
>
> I will add more comments. When this flag is set VMStateInfo.get/put will
> be used in vmstate_load/save_state instead of recursive call. And the
> user should implement VMStateInfo.get/put to handle the concerned data
> structure.
> > Also, why do you need this flag at all? The only change I can see is
> > that you pass additional information to VMStateInfo.get() / .put(),
> > using NULL if it's not set. Why don't you just always pass the
> > additional information? If the additional information is not needed by
> > get() / put() the parameter will be unused anyway.
> You can do it without creating this flag. Instead just to check if info
> is set in the field. However I think it is more readable and more robust
> to check this flag in vmstate_load/get_state.
Apologies for not getting around to this sooner; I was on holiday when you first
sent it.
But:
a) Is there a reason to use the 'bool' between each element; can't you count
the
length of the queue, send the length and then send the contents? In that
case
it should look a lot more like an ARRAY case on the wire.
b) I think you should really try and split it into two parts:
1) A VMSTATE_ARRAY_CUSTOM (?) - so it's an array of elements but we've got
a special
way of allocating/counting/reading the elements
2) A version of that which is for a QTAILQ.
It's important that whatever ends up on the migration stream doesn't
reflect
that it happens to be implemented as a QTAILQ; so if you decide to change
it later the migration compatibility doesn't break.
c) Use new trace_ names for get_qtailq rather than misusing
trace_vmstate_load_state*
d) Add a trace_ for put_qtailq as well - that way when it goes horribly wrong
we can
turn the tracing on on both sides :-)
e) Is there anyway to make QTAILQ_RAW_INSERT_TAIL any more readable?
I don't think I understand why you can't use the standard QTAILQ macros.
f) You might need to fix up Amit's scripts/vmstate-static-checker.py
Dave
> > Sascha
> >
>
> Thanks,
> Jianjun
>
>
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
