On 10/28/2016 09:46 PM, Jianjun Duan wrote:
>
>
> On 10/28/2016 12:06 PM, Dr. David Alan Gilbert wrote:
>> * Jianjun Duan (du...@linux.vnet.ibm.com) wrote:
>>> Currently we cannot directly transfer a QTAILQ instance because of the
>>> limitation in the migration code. Here we introduce an approach to
>>> transfer such structures. We created VMStateInfo vmstate_info_qtailq
>>> for QTAILQ. Similar VMStateInfo can be created for other data structures
>>> such as list.
>>>
>>> This approach will be used to transfer pending_events and ccs_list in spapr
>>> state.
>>>
>>> We also create some macros in qemu/queue.h to access a QTAILQ using pointer
>>> arithmetic. This ensures that we do not depend on the implementation
>>> details about QTAILQ in the migration code.
>>>
>>> Signed-off-by: Jianjun Duan <du...@linux.vnet.ibm.com>
>>> ---
>>> include/migration/vmstate.h | 20 ++++++++++++++
>>> include/qemu/queue.h | 61 +++++++++++++++++++++++++++++++++++++++++
>>> migration/trace-events | 4 +++
>>> migration/vmstate.c | 67
>>> +++++++++++++++++++++++++++++++++++++++++++++
>>> 4 files changed, 152 insertions(+)
>>>
>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>>> index d0e37b5..318a6f1 100644
>>> --- a/include/migration/vmstate.h
>>> +++ b/include/migration/vmstate.h
>>> @@ -251,6 +251,7 @@ extern const VMStateInfo vmstate_info_timer;
>>> extern const VMStateInfo vmstate_info_buffer;
>>> extern const VMStateInfo vmstate_info_unused_buffer;
>>> extern const VMStateInfo vmstate_info_bitmap;
>>> +extern const VMStateInfo vmstate_info_qtailq;
>>>
>>> #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
>>> #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0)
>>> @@ -662,6 +663,25 @@ extern const VMStateInfo vmstate_info_bitmap;
>>> .offset = offsetof(_state, _field), \
>>> }
>>>
>>> +/* For QTAILQ that need customized handling.
>>> + * Target QTAILQ needs be properly initialized.
>>> + * _type: type of QTAILQ element
>>> + * _next: name of QTAILQ entry field in QTAILQ element
>>> + * _vmsd: VMSD for QTAILQ element
>>> + * size: size of QTAILQ element
>>> + * start: offset of QTAILQ entry in QTAILQ element
>>> + */
>>> +#define VMSTATE_QTAILQ_V(_field, _state, _version, _vmsd, _type, _next) \
>>> +{ \
>>> + .name = (stringify(_field)), \
>>> + .version_id = (_version), \
>>> + .vmsd = &(_vmsd), \
>>> + .size = sizeof(_type), \
>>> + .info = &vmstate_info_qtailq, \
>>> + .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 342073f..16af127 100644
>>> --- a/include/qemu/queue.h
>>> +++ b/include/qemu/queue.h
>>> @@ -438,4 +438,65 @@ struct {
>>> \
>>> #define QTAILQ_PREV(elm, headname, field) \
>>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last))
>>>
>>> +#define field_at_offset(base, offset, type)
>>> \
>>> + ((type) (((char *) (base)) + (offset)))
>>> +
>>> +typedef struct DUMB_Q_ENTRY DUMB_Q_ENTRY;
>>> +typedef struct DUMB_Q DUMB_Q;
>>> +
>>> +struct DUMB_Q_ENTRY {
>>> + QTAILQ_ENTRY(DUMB_Q_ENTRY) next;
>>> +};
>>> +
>>> +struct DUMB_Q {
>>> + QTAILQ_HEAD(DUMB_Q_HEAD, DUMB_Q_ENTRY) head;
>>> +};
>>
>> OK, good we've got rid of the hard coded offsets; thanks!
>>
>>> +#define dumb_q ((DUMB_Q *) 0)
>>> +#define dumb_qh ((typeof(dumb_q->head) *) 0)
>>> +#define dumb_qe ((DUMB_Q_ENTRY *) 0)
>>
>> Note that 'dumb' and 'dummy' are completely different words;
>> this is a dummy not a dumb.
>>
> OK.
>>> +/*
>>> + * Offsets of layout of a tail queue head.
>>> + */
>>> +#define QTAILQ_FIRST_OFFSET ((size_t) &(QTAILQ_FIRST(dumb_qh)))
>>
>> Isn't that just offsetof(dumb_qh, tqh_first) ?
> Yes. But I don't want to depend on the inside of the type if it is
> possible. QTAILQ_FIRST is a defined access macro.
>
>>
>>> +#define QTAILQ_LAST_OFFSET (offsetof(typeof(dumb_q->head), tqh_last))
>>
>> Similarly isnt't that just offsetof(DUMB_Q_HEAD, tqh_last) ?
>>
> Similarly, DUMB_Q_HEAD may not be a type name in the future.
>
> Thanks,
> Jianjun
>>> +/*
>>> + * Raw access of elements of a tail queue
>>> + */
>>> +#define QTAILQ_RAW_FIRST(head)
>>> \
>>> + (*field_at_offset(head, QTAILQ_FIRST_OFFSET, void **))
>>> +#define QTAILQ_RAW_LAST(head)
>>> \
>>> + (*field_at_offset(head, QTAILQ_LAST_OFFSET, void ***))
>>> +
>>> +/*
>>> + * Offsets of layout of a tail queue element.
>>> + */
>>> +#define QTAILQ_NEXT_OFFSET ((size_t) (&QTAILQ_NEXT(dumb_qe, next)))
>>> +#define QTAILQ_PREV_OFFSET (offsetof(typeof(dumb_qe->next), tqe_prev))
>>
>> Similar comments to the pair above.
>>
>> Dave
>> P.S. I'm out next week, so please someone else jump in.
>>
[..]
I think this got overly complicated. Here is a little patch on
top of your stuff which gets rid of 15 lines and IMHO simplifies
things quite a bit. What do you think?
It is based on/inspired by Dave's proposal with the dummy stuff.
Did not address the typos though.
Cheers,
Halil
----------------- 8< ----------------------------
From: Halil Pasic <pa...@linux.vnet.ibm.com>
Date: Mon, 31 Oct 2016 13:53:31 +0100
Subject: [PATCH] fixup: simplify QTAILQ raw access macros
Intended to be fixed up to [PATCH v9 2/3] migration: migrate QTAILQ.
Signed-off-by: Halil Pasic <pa...@linux.vnet.ibm.com>
---
include/qemu/queue.h | 33 +++++++++------------------------
1 files changed, 9 insertions(+), 24 deletions(-)
diff --git a/include/qemu/queue.h b/include/qemu/queue.h
index 16af127..d67cb4e 100644
--- a/include/qemu/queue.h
+++ b/include/qemu/queue.h
@@ -441,33 +441,17 @@ struct {
\
#define field_at_offset(base, offset, type)
\
((type) (((char *) (base)) + (offset)))
-typedef struct DUMB_Q_ENTRY DUMB_Q_ENTRY;
-typedef struct DUMB_Q DUMB_Q;
-
-struct DUMB_Q_ENTRY {
- QTAILQ_ENTRY(DUMB_Q_ENTRY) next;
-};
-
-struct DUMB_Q {
- QTAILQ_HEAD(DUMB_Q_HEAD, DUMB_Q_ENTRY) head;
-};
-
-#define dumb_q ((DUMB_Q *) 0)
-#define dumb_qh ((typeof(dumb_q->head) *) 0)
-#define dumb_qe ((DUMB_Q_ENTRY *) 0)
-
/*
- * Offsets of layout of a tail queue head.
+ * Raw access helpers
*/
-#define QTAILQ_FIRST_OFFSET ((size_t) &(QTAILQ_FIRST(dumb_qh)))
-#define QTAILQ_LAST_OFFSET (offsetof(typeof(dumb_q->head), tqh_last))
+typedef Q_TAILQ_HEAD(QTAILQRawHead, void,) QTAILQRawHead;
+typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry;
+
/*
* Raw access of elements of a tail queue
*/
-#define QTAILQ_RAW_FIRST(head)
\
- (*field_at_offset(head, QTAILQ_FIRST_OFFSET, void **))
-#define QTAILQ_RAW_LAST(head)
\
- (*field_at_offset(head, QTAILQ_LAST_OFFSET, void ***))
+#define QTAILQ_RAW_FIRST(head) (((QTAILQRawHead *)(head))->tqh_first)
+#define QTAILQ_RAW_LAST(head) (((QTAILQRawHead *)(head))->tqh_last)
/*
* Offsets of layout of a tail queue element.
@@ -479,9 +463,10 @@ struct DUMB_Q {
* Raw access of elements of a tail entry
*/
#define QTAILQ_RAW_NEXT(elm, entry)
\
- (*field_at_offset(elm, entry + QTAILQ_NEXT_OFFSET, void **))
+ ((field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_next))
#define QTAILQ_RAW_PREV(elm, entry)
\
- (*field_at_offset(elm, entry + QTAILQ_PREV_OFFSET, void ***))
+ (field_at_offset(elm, entry, QTAILQRawEntry *)->tqe_prev)
+
/*
* Tail queue tranversal using pointer arithmetic.
*/
--
1.7.1
