| Message ID | 1476394254-7987-3-git-send-email-duanj@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
* Jianjun Duan (duanj@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 <duanj@linux.vnet.ibm.com> > --- > include/migration/vmstate.h | 20 +++++++++++++++ > include/qemu/queue.h | 32 ++++++++++++++++++++++++ > migration/trace-events | 4 +++ > migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 115 insertions(+) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index d0e37b5..4dd0aed 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 > + * _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, \ > + .flags = VMS_LINKED, \ Hang on - you killed VMS_LINKED off in the previous patch; so this doesn't work. (Adding a test to tests/test-vmstate.c would have found that) > + .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..d672ae0 100644 > --- a/include/qemu/queue.h > +++ b/include/qemu/queue.h > @@ -438,4 +438,36 @@ struct { \ > #define QTAILQ_PREV(elm, headname, field) \ > (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last)) > > +/* > + * Offsets of layout of a tail queue head. > + */ > +#define QTAILQ_FIRST_OFFSET 0 > +#define QTAILQ_LAST_OFFSET (sizeof(void *)) > + > +/* > + * Offsets of layout of a tail queue element. > + */ > +#define QTAILQ_NEXT_OFFSET 0 > +#define QTAILQ_PREV_OFFSET (sizeof(void *)) > + > +/* > + * Tail queue tranversal using pointer arithmetic. > + */ > +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ > + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ > + (elm); \ > + (elm) = \ > + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) > +/* > + * Tail queue insertion using pointer arithmetic. > + */ > +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ > + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ > + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ > + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ > + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ > + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ > + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ > +} while (/*CONSTCOND*/0) I wonder if there's a simpler way to do this; I'm not sure this works, but something like: struct QTAILQDummy { char dummy; }; QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; #define QTAILQ_RAW_FOREACH(elm, head, entry) \ for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ (elm); \ (elm) = \ (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next and then I think elm gets declared as a struct QTAILQDummy. But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. Would that work? > #endif /* QEMU_SYS_QUEUE_H */ > diff --git a/migration/trace-events b/migration/trace-events > index dfee75a..9a6ec59 100644 > --- a/migration/trace-events > +++ b/migration/trace-events > @@ -52,6 +52,10 @@ vmstate_n_elems(const char *name, int n_elems) "%s: %d" > vmstate_subsection_load(const char *parent) "%s" > vmstate_subsection_load_bad(const char *parent, const char *sub, const char *sub2) "%s: %s/%s" > vmstate_subsection_load_good(const char *parent) "%s" > +get_qtailq(const char *name, int version_id) "%s v%d" > +get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d" > +put_qtailq(const char *name, int version_id) "%s v%d" > +put_qtailq_end(const char *name, const char *reason) "%s %s" > > # migration/qemu-file.c > qemu_file_fclose(void) "" > diff --git a/migration/vmstate.c b/migration/vmstate.c > index e8eff3f..eb68fc6 100644 > --- a/migration/vmstate.c > +++ b/migration/vmstate.c > @@ -5,6 +5,7 @@ > #include "migration/vmstate.h" > #include "qemu/bitops.h" > #include "qemu/error-report.h" > +#include "qemu/queue.h" > #include "trace.h" > #include "migration/qjson.h" > > @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = { > .get = get_bitmap, > .put = put_bitmap, > }; > + > +/*get for QTAILQ */ > +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, > + VMStateField *field) > +{ > + int ret = 0; > + const VMStateDescription *vmsd = field->vmsd; > + size_t size = field->size; > + size_t entry = field->start; Please comment these two here; I'd prefer entry_offset or something as a name to make clear what it is. > + int version_id = field->version_id; > + void *elm; > + > + trace_get_qtailq(vmsd->name, version_id); > + if (version_id > vmsd->version_id) { > + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL); > + return -EINVAL; > + } > + if (version_id < vmsd->minimum_version_id) { > + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL); > + return -EINVAL; > + } Please make those error_report's - if anything fails migration I like to see it in the stderr log. > + while (qemu_get_byte(f)) { > + elm = g_malloc(size); > + ret = vmstate_load_state(f, vmsd, elm, version_id); > + if (ret) { > + return ret; > + } > + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry); > + } > + > + trace_get_qtailq_end(vmsd->name, "end", ret); > + return ret; > +} > + > +/* put for QTAILQ */ > +static void put_qtailq(QEMUFile *f, void *pv, size_t unused_size, > + VMStateField *field, QJSON *vmdesc) > +{ > + const VMStateDescription *vmsd = field->vmsd; > + size_t entry = field->start; > + void *elm; > + > + trace_put_qtailq(vmsd->name, vmsd->version_id); > + > + QTAILQ_RAW_FOREACH(elm, pv, entry) { > + qemu_put_byte(f, true); > + vmstate_save_state(f, vmsd, elm, vmdesc); > + } > + qemu_put_byte(f, false); > + > + trace_put_qtailq_end(vmsd->name, "end"); > +} > +const VMStateInfo vmstate_info_qtailq = { > + .name = "qtailq", > + .get = get_qtailq, > + .put = put_qtailq, > +}; > -- > 1.9.1 Dave > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 14/10/2016 12:44, Dr. David Alan Gilbert wrote: >> > +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >> > + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ >> > + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ >> > + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ >> > + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ >> > + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ >> > + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ >> > +} while (/*CONSTCOND*/0) > I wonder if there's a simpler way to do this; I'm not sure this works, but something like: > > struct QTAILQDummy { > char dummy; > }; > > QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) > typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; > > #define QTAILQ_RAW_FOREACH(elm, head, entry) \ > for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ > (elm); \ > (elm) = \ > (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next > > and then I think elm gets declared as a struct QTAILQDummy. > But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. Another possibility is a macro like #define field_at_offset(base, offset, type) \ ((type) (((char *) (base)) + (offset))) so that you can do *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL; *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) = *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET); **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm); *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET); or something like that (note that I've always used the same type for next and last, by the way). Paolo
I need to double check my code. My build passed and migration test also succeeded. On 10/14/2016 04:07 AM, Paolo Bonzini wrote: > > > On 14/10/2016 12:44, Dr. David Alan Gilbert wrote: >>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ >>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ >>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ >>>> +} while (/*CONSTCOND*/0) >> I wonder if there's a simpler way to do this; I'm not sure this works, but something like: >> >> struct QTAILQDummy { >> char dummy; >> }; >> >> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) >> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; >> >> #define QTAILQ_RAW_FOREACH(elm, head, entry) \ >> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ >> (elm); \ >> (elm) = \ >> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next >> >> and then I think elm gets declared as a struct QTAILQDummy. >> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. > > Another possibility is a macro like > > #define field_at_offset(base, offset, type) \ > ((type) (((char *) (base)) + (offset))) > > so that you can do > > *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL; > *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) = > *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET); > **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm); > *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = > field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET); The thing is that we don't know type at all. So only the offset values is used. This is intended for QTAILQ of any type. Thanks, Jianjun > > or something like that (note that I've always used the same type for > next and last, by the way). > > Paolo >
On 10/14/2016 03:44 AM, Dr. David Alan Gilbert wrote: > * Jianjun Duan (duanj@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 <duanj@linux.vnet.ibm.com> >> --- >> include/migration/vmstate.h | 20 +++++++++++++++ >> include/qemu/queue.h | 32 ++++++++++++++++++++++++ >> migration/trace-events | 4 +++ >> migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ >> 4 files changed, 115 insertions(+) >> >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >> index d0e37b5..4dd0aed 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 >> + * _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, \ >> + .flags = VMS_LINKED, \ > > Hang on - you killed VMS_LINKED off in the previous patch; so this > doesn't work. > (Adding a test to tests/test-vmstate.c would have found that) > Thanks for pointing it out. Since the actual application of this patch is split off, my test didn't go through this path. >> + .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..d672ae0 100644 >> --- a/include/qemu/queue.h >> +++ b/include/qemu/queue.h >> @@ -438,4 +438,36 @@ struct { \ >> #define QTAILQ_PREV(elm, headname, field) \ >> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last)) >> >> +/* >> + * Offsets of layout of a tail queue head. >> + */ >> +#define QTAILQ_FIRST_OFFSET 0 >> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) >> + >> +/* >> + * Offsets of layout of a tail queue element. >> + */ >> +#define QTAILQ_NEXT_OFFSET 0 >> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) >> + >> +/* >> + * Tail queue tranversal using pointer arithmetic. >> + */ >> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ >> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ >> + (elm); \ >> + (elm) = \ >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) >> +/* >> + * Tail queue insertion using pointer arithmetic. >> + */ >> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ >> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ >> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ >> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ >> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ >> +} while (/*CONSTCOND*/0) > > I wonder if there's a simpler way to do this; I'm not sure this works, but something like: > > struct QTAILQDummy { > char dummy; > }; > > QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) > typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; > > #define QTAILQ_RAW_FOREACH(elm, head, entry) \ > for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ > (elm); \ > (elm) = \ > (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next > > and then I think elm gets declared as a struct QTAILQDummy. > But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. > > Would that work? > It is intended for QTAILQ of any type. So type is not available. >> #endif /* QEMU_SYS_QUEUE_H */ >> diff --git a/migration/trace-events b/migration/trace-events >> index dfee75a..9a6ec59 100644 >> --- a/migration/trace-events >> +++ b/migration/trace-events >> @@ -52,6 +52,10 @@ vmstate_n_elems(const char *name, int n_elems) "%s: %d" >> vmstate_subsection_load(const char *parent) "%s" >> vmstate_subsection_load_bad(const char *parent, const char *sub, const char *sub2) "%s: %s/%s" >> vmstate_subsection_load_good(const char *parent) "%s" >> +get_qtailq(const char *name, int version_id) "%s v%d" >> +get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d" >> +put_qtailq(const char *name, int version_id) "%s v%d" >> +put_qtailq_end(const char *name, const char *reason) "%s %s" >> >> # migration/qemu-file.c >> qemu_file_fclose(void) "" >> diff --git a/migration/vmstate.c b/migration/vmstate.c >> index e8eff3f..eb68fc6 100644 >> --- a/migration/vmstate.c >> +++ b/migration/vmstate.c >> @@ -5,6 +5,7 @@ >> #include "migration/vmstate.h" >> #include "qemu/bitops.h" >> #include "qemu/error-report.h" >> +#include "qemu/queue.h" >> #include "trace.h" >> #include "migration/qjson.h" >> >> @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = { >> .get = get_bitmap, >> .put = put_bitmap, >> }; >> + >> +/*get for QTAILQ */ >> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, >> + VMStateField *field) >> +{ >> + int ret = 0; >> + const VMStateDescription *vmsd = field->vmsd; >> + size_t size = field->size; >> + size_t entry = field->start; > > Please comment these two here; I'd prefer entry_offset or something > as a name to make clear what it is. > OK. >> + int version_id = field->version_id; >> + void *elm; >> + >> + trace_get_qtailq(vmsd->name, version_id); >> + if (version_id > vmsd->version_id) { >> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL); >> + return -EINVAL; >> + } >> + if (version_id < vmsd->minimum_version_id) { >> + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL); >> + return -EINVAL; >> + } > > Please make those error_report's - if anything fails migration > I like to see it in the stderr log. > I know previously you wanted it to be error report. But in the migration code the same error is always just put into trace (vmastate_load_state). I just want to be consistent. Thanks, Jianjun >> + while (qemu_get_byte(f)) { >> + elm = g_malloc(size); >> + ret = vmstate_load_state(f, vmsd, elm, version_id); >> + if (ret) { >> + return ret; >> + } >> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry); >> + } >> + >> + trace_get_qtailq_end(vmsd->name, "end", ret); >> + return ret; >> +} >> + >> +/* put for QTAILQ */ >> +static void put_qtailq(QEMUFile *f, void *pv, size_t unused_size, >> + VMStateField *field, QJSON *vmdesc) >> +{ >> + const VMStateDescription *vmsd = field->vmsd; >> + size_t entry = field->start; >> + void *elm; >> + >> + trace_put_qtailq(vmsd->name, vmsd->version_id); >> + >> + QTAILQ_RAW_FOREACH(elm, pv, entry) { >> + qemu_put_byte(f, true); >> + vmstate_save_state(f, vmsd, elm, vmdesc); >> + } >> + qemu_put_byte(f, false); >> + >> + trace_put_qtailq_end(vmsd->name, "end"); >> +} >> +const VMStateInfo vmstate_info_qtailq = { >> + .name = "qtailq", >> + .get = get_qtailq, >> + .put = put_qtailq, >> +}; >> -- >> 1.9.1 > > Dave > >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote: > > > On 10/14/2016 03:44 AM, Dr. David Alan Gilbert wrote: > > * Jianjun Duan (duanj@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 <duanj@linux.vnet.ibm.com> > >> --- > >> include/migration/vmstate.h | 20 +++++++++++++++ > >> include/qemu/queue.h | 32 ++++++++++++++++++++++++ > >> migration/trace-events | 4 +++ > >> migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ > >> 4 files changed, 115 insertions(+) > >> > >> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > >> index d0e37b5..4dd0aed 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 > >> + * _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, \ > >> + .flags = VMS_LINKED, \ > > > > Hang on - you killed VMS_LINKED off in the previous patch; so this > > doesn't work. > > (Adding a test to tests/test-vmstate.c would have found that) > > > > Thanks for pointing it out. Since the actual application of this patch > is split off, my test didn't go through this path. Yes, but you could add a new entry into test-vmstate.c for this. > >> + .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..d672ae0 100644 > >> --- a/include/qemu/queue.h > >> +++ b/include/qemu/queue.h > >> @@ -438,4 +438,36 @@ struct { \ > >> #define QTAILQ_PREV(elm, headname, field) \ > >> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last)) > >> > >> +/* > >> + * Offsets of layout of a tail queue head. > >> + */ > >> +#define QTAILQ_FIRST_OFFSET 0 > >> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) > >> + > >> +/* > >> + * Offsets of layout of a tail queue element. > >> + */ > >> +#define QTAILQ_NEXT_OFFSET 0 > >> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) > >> + > >> +/* > >> + * Tail queue tranversal using pointer arithmetic. > >> + */ > >> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ > >> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ > >> + (elm); \ > >> + (elm) = \ > >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) > >> +/* > >> + * Tail queue insertion using pointer arithmetic. > >> + */ > >> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ > >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ > >> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ > >> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ > >> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ > >> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ > >> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ > >> +} while (/*CONSTCOND*/0) > > > > I wonder if there's a simpler way to do this; I'm not sure this works, but something like: > > > > struct QTAILQDummy { > > char dummy; > > }; > > > > QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) > > typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; > > > > #define QTAILQ_RAW_FOREACH(elm, head, entry) \ > > for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ > > (elm); \ > > (elm) = \ > > (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next > > > > and then I think elm gets declared as a struct QTAILQDummy. > > But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. > > > > Would that work? > > > It is intended for QTAILQ of any type. So type is not available. I think it might be possible to do it generally. > >> #endif /* QEMU_SYS_QUEUE_H */ > >> diff --git a/migration/trace-events b/migration/trace-events > >> index dfee75a..9a6ec59 100644 > >> --- a/migration/trace-events > >> +++ b/migration/trace-events > >> @@ -52,6 +52,10 @@ vmstate_n_elems(const char *name, int n_elems) "%s: %d" > >> vmstate_subsection_load(const char *parent) "%s" > >> vmstate_subsection_load_bad(const char *parent, const char *sub, const char *sub2) "%s: %s/%s" > >> vmstate_subsection_load_good(const char *parent) "%s" > >> +get_qtailq(const char *name, int version_id) "%s v%d" > >> +get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d" > >> +put_qtailq(const char *name, int version_id) "%s v%d" > >> +put_qtailq_end(const char *name, const char *reason) "%s %s" > >> > >> # migration/qemu-file.c > >> qemu_file_fclose(void) "" > >> diff --git a/migration/vmstate.c b/migration/vmstate.c > >> index e8eff3f..eb68fc6 100644 > >> --- a/migration/vmstate.c > >> +++ b/migration/vmstate.c > >> @@ -5,6 +5,7 @@ > >> #include "migration/vmstate.h" > >> #include "qemu/bitops.h" > >> #include "qemu/error-report.h" > >> +#include "qemu/queue.h" > >> #include "trace.h" > >> #include "migration/qjson.h" > >> > >> @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = { > >> .get = get_bitmap, > >> .put = put_bitmap, > >> }; > >> + > >> +/*get for QTAILQ */ > >> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, > >> + VMStateField *field) > >> +{ > >> + int ret = 0; > >> + const VMStateDescription *vmsd = field->vmsd; > >> + size_t size = field->size; > >> + size_t entry = field->start; > > > > Please comment these two here; I'd prefer entry_offset or something > > as a name to make clear what it is. > > > OK. > >> + int version_id = field->version_id; > >> + void *elm; > >> + > >> + trace_get_qtailq(vmsd->name, version_id); > >> + if (version_id > vmsd->version_id) { > >> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL); > >> + return -EINVAL; > >> + } > >> + if (version_id < vmsd->minimum_version_id) { > >> + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL); > >> + return -EINVAL; > >> + } > > > > Please make those error_report's - if anything fails migration > > I like to see it in the stderr log. > > > I know previously you wanted it to be error report. But in the migration > code the same error is always just put into trace (vmastate_load_state). > I just want to be consistent. I should fix those as well to add error_reports ! It's very difficult when we get an error back from a customer with just a migration failure when the only thing we know is an -EINVAL, so I keep adding error_report calls. Dave > Thanks, > Jianjun > >> + while (qemu_get_byte(f)) { > >> + elm = g_malloc(size); > >> + ret = vmstate_load_state(f, vmsd, elm, version_id); > >> + if (ret) { > >> + return ret; > >> + } > >> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry); > >> + } > >> + > >> + trace_get_qtailq_end(vmsd->name, "end", ret); > >> + return ret; > >> +} > >> + > >> +/* put for QTAILQ */ > >> +static void put_qtailq(QEMUFile *f, void *pv, size_t unused_size, > >> + VMStateField *field, QJSON *vmdesc) > >> +{ > >> + const VMStateDescription *vmsd = field->vmsd; > >> + size_t entry = field->start; > >> + void *elm; > >> + > >> + trace_put_qtailq(vmsd->name, vmsd->version_id); > >> + > >> + QTAILQ_RAW_FOREACH(elm, pv, entry) { > >> + qemu_put_byte(f, true); > >> + vmstate_save_state(f, vmsd, elm, vmdesc); > >> + } > >> + qemu_put_byte(f, false); > >> + > >> + trace_put_qtailq_end(vmsd->name, "end"); > >> +} > >> +const VMStateInfo vmstate_info_qtailq = { > >> + .name = "qtailq", > >> + .get = get_qtailq, > >> + .put = put_qtailq, > >> +}; > >> -- > >> 1.9.1 > > > > Dave > > > >> > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 10/14/2016 10:04 AM, Dr. David Alan Gilbert wrote: > * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote: >> >> >> On 10/14/2016 03:44 AM, Dr. David Alan Gilbert wrote: >>> * Jianjun Duan (duanj@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 <duanj@linux.vnet.ibm.com> >>>> --- >>>> include/migration/vmstate.h | 20 +++++++++++++++ >>>> include/qemu/queue.h | 32 ++++++++++++++++++++++++ >>>> migration/trace-events | 4 +++ >>>> migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 115 insertions(+) >>>> >>>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >>>> index d0e37b5..4dd0aed 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 >>>> + * _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, \ >>>> + .flags = VMS_LINKED, \ >>> >>> Hang on - you killed VMS_LINKED off in the previous patch; so this >>> doesn't work. >>> (Adding a test to tests/test-vmstate.c would have found that) >>> >> >> Thanks for pointing it out. Since the actual application of this patch >> is split off, my test didn't go through this path. > > Yes, but you could add a new entry into test-vmstate.c for this. OK. > >>>> + .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..d672ae0 100644 >>>> --- a/include/qemu/queue.h >>>> +++ b/include/qemu/queue.h >>>> @@ -438,4 +438,36 @@ struct { \ >>>> #define QTAILQ_PREV(elm, headname, field) \ >>>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last)) >>>> >>>> +/* >>>> + * Offsets of layout of a tail queue head. >>>> + */ >>>> +#define QTAILQ_FIRST_OFFSET 0 >>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) >>>> + >>>> +/* >>>> + * Offsets of layout of a tail queue element. >>>> + */ >>>> +#define QTAILQ_NEXT_OFFSET 0 >>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) >>>> + >>>> +/* >>>> + * Tail queue tranversal using pointer arithmetic. >>>> + */ >>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ >>>> + (elm); \ >>>> + (elm) = \ >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) >>>> +/* >>>> + * Tail queue insertion using pointer arithmetic. >>>> + */ >>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ >>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ >>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ >>>> +} while (/*CONSTCOND*/0) >>> >>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like: >>> >>> struct QTAILQDummy { >>> char dummy; >>> }; >>> >>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) >>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; >>> >>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ >>> (elm); \ >>> (elm) = \ >>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next >>> >>> and then I think elm gets declared as a struct QTAILQDummy. >>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. >>> >>> Would that work? >>> >> It is intended for QTAILQ of any type. So type is not available. > > I think it might be possible to do it generally. > If we have type, then we can use what is there already, and don't need a pointer arithmetic based approach. Inside put/get, we only get type layout info from vmsd, which is all about size and offset. This macro is used inside put/get, so I am not sure how we can directly use type here. >>>> #endif /* QEMU_SYS_QUEUE_H */ >>>> diff --git a/migration/trace-events b/migration/trace-events >>>> index dfee75a..9a6ec59 100644 >>>> --- a/migration/trace-events >>>> +++ b/migration/trace-events >>>> @@ -52,6 +52,10 @@ vmstate_n_elems(const char *name, int n_elems) "%s: %d" >>>> vmstate_subsection_load(const char *parent) "%s" >>>> vmstate_subsection_load_bad(const char *parent, const char *sub, const char *sub2) "%s: %s/%s" >>>> vmstate_subsection_load_good(const char *parent) "%s" >>>> +get_qtailq(const char *name, int version_id) "%s v%d" >>>> +get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d" >>>> +put_qtailq(const char *name, int version_id) "%s v%d" >>>> +put_qtailq_end(const char *name, const char *reason) "%s %s" >>>> >>>> # migration/qemu-file.c >>>> qemu_file_fclose(void) "" >>>> diff --git a/migration/vmstate.c b/migration/vmstate.c >>>> index e8eff3f..eb68fc6 100644 >>>> --- a/migration/vmstate.c >>>> +++ b/migration/vmstate.c >>>> @@ -5,6 +5,7 @@ >>>> #include "migration/vmstate.h" >>>> #include "qemu/bitops.h" >>>> #include "qemu/error-report.h" >>>> +#include "qemu/queue.h" >>>> #include "trace.h" >>>> #include "migration/qjson.h" >>>> >>>> @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = { >>>> .get = get_bitmap, >>>> .put = put_bitmap, >>>> }; >>>> + >>>> +/*get for QTAILQ */ >>>> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, >>>> + VMStateField *field) >>>> +{ >>>> + int ret = 0; >>>> + const VMStateDescription *vmsd = field->vmsd; >>>> + size_t size = field->size; >>>> + size_t entry = field->start; >>> >>> Please comment these two here; I'd prefer entry_offset or something >>> as a name to make clear what it is. >>> >> OK. >>>> + int version_id = field->version_id; >>>> + void *elm; >>>> + >>>> + trace_get_qtailq(vmsd->name, version_id); >>>> + if (version_id > vmsd->version_id) { >>>> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL); >>>> + return -EINVAL; >>>> + } >>>> + if (version_id < vmsd->minimum_version_id) { >>>> + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL); >>>> + return -EINVAL; >>>> + } >>> >>> Please make those error_report's - if anything fails migration >>> I like to see it in the stderr log. >>> >> I know previously you wanted it to be error report. But in the migration >> code the same error is always just put into trace (vmastate_load_state). >> I just want to be consistent. > > I should fix those as well to add error_reports ! > It's very difficult when we get an error back from a customer with just > a migration failure when the only thing we know is an -EINVAL, so I keep > adding error_report calls. > OK. I will add err reports in vmstate.c. Thanks, Jianjun > Dave > >> Thanks, >> Jianjun >>>> + while (qemu_get_byte(f)) { >>>> + elm = g_malloc(size); >>>> + ret = vmstate_load_state(f, vmsd, elm, version_id); >>>> + if (ret) { >>>> + return ret; >>>> + } >>>> + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry); >>>> + } >>>> + >>>> + trace_get_qtailq_end(vmsd->name, "end", ret); >>>> + return ret; >>>> +} >>>> + >>>> +/* put for QTAILQ */ >>>> +static void put_qtailq(QEMUFile *f, void *pv, size_t unused_size, >>>> + VMStateField *field, QJSON *vmdesc) >>>> +{ >>>> + const VMStateDescription *vmsd = field->vmsd; >>>> + size_t entry = field->start; >>>> + void *elm; >>>> + >>>> + trace_put_qtailq(vmsd->name, vmsd->version_id); >>>> + >>>> + QTAILQ_RAW_FOREACH(elm, pv, entry) { >>>> + qemu_put_byte(f, true); >>>> + vmstate_save_state(f, vmsd, elm, vmdesc); >>>> + } >>>> + qemu_put_byte(f, false); >>>> + >>>> + trace_put_qtailq_end(vmsd->name, "end"); >>>> +} >>>> +const VMStateInfo vmstate_info_qtailq = { >>>> + .name = "qtailq", >>>> + .get = get_qtailq, >>>> + .put = put_qtailq, >>>> +}; >>>> -- >>>> 1.9.1 >>> >>> Dave >>> >>>> >>> -- >>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >>> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
> > Another possibility is a macro like > > > > #define field_at_offset(base, offset, type) \ > > ((type) (((char *) (base)) + (offset))) > > > > so that you can do > > > > *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL; > > *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) = > > *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET); > > **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm); > > *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = > > field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET); > > The thing is that we don't know type at all. So only the offset values > is used. This is intended for QTAILQ of any type. Sure, my code is identical to yours---just with more macros for readability, and a little more type-safe. Another possibility: #define QTAILQ_RAW_NEXT(elm, entry) \ (*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET)) #define QTAILQ_RAW_PREV(elm, entry) \ (*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET)) #define QTAILQ_RAW_LAST(head) \ (*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET)) QTAILQ_RAW_NEXT(elm, entry) = NULL; QTAILQ_RAW_PREV(elm, entry) = QTAILQ_RAW_LAST(head); *QTAILQ_RAW_LAST(head) = (elm); QTAILQ_RAW_LAST(head) = &QTAILQ_RAW_NEXT(elm, entry); Thanks, Paolo > Thanks, > Jianjun > > > > > or something like that (note that I've always used the same type for > > next and last, by the way). > > > > Paolo > > > >
On 10/14/2016 11:39 AM, Paolo Bonzini wrote: > >>> Another possibility is a macro like >>> >>> #define field_at_offset(base, offset, type) \ >>> ((type) (((char *) (base)) + (offset))) >>> >>> so that you can do >>> >>> *field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET) = NULL; >>> *field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET) = >>> *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET); >>> **field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = (elm); >>> *field_at_offset(void ***, head, QTAILQ_LAST_OFFSET) = >>> field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET); >> >> The thing is that we don't know type at all. So only the offset values >> is used. This is intended for QTAILQ of any type. > > Sure, my code is identical to yours---just with more macros for readability, > and a little more type-safe. Another possibility: > > #define QTAILQ_RAW_NEXT(elm, entry) \ > (*field_at_offset(void **, elm, (entry) + QTAILQ_NEXT_OFFSET)) > #define QTAILQ_RAW_PREV(elm, entry) \ > (*field_at_offset(void ***, elm, (entry) + QTAILQ_PREV_OFFSET)) > #define QTAILQ_RAW_LAST(head) \ > (*field_at_offset(void ***, head, QTAILQ_LAST_OFFSET)) > > QTAILQ_RAW_NEXT(elm, entry) = NULL; > QTAILQ_RAW_PREV(elm, entry) = QTAILQ_RAW_LAST(head); > *QTAILQ_RAW_LAST(head) = (elm); > QTAILQ_RAW_LAST(head) = &QTAILQ_RAW_NEXT(elm, entry); > You are right. The readability can be approved. Thanks, Jianjun > Thanks, > > Paolo > >> Thanks, >> Jianjun >> >>> >>> or something like that (note that I've always used the same type for >>> next and last, by the way). >>> >>> Paolo >>> >> >> >
On 10/14/2016 07:18 PM, Jianjun Duan wrote: >>>>> +/* >>>>> >>>> + * Offsets of layout of a tail queue head. >>>>> >>>> + */ >>>>> >>>> +#define QTAILQ_FIRST_OFFSET 0 >>>>> >>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) >>>>> >>>> + >>>>> >>>> +/* >>>>> >>>> + * Offsets of layout of a tail queue element. >>>>> >>>> + */ >>>>> >>>> +#define QTAILQ_NEXT_OFFSET 0 >>>>> >>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) >>>>> >>>> + >>>>> >>>> +/* >>>>> >>>> + * Tail queue tranversal using pointer arithmetic. >>>>> >>>> + */ >>>>> >>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>>>> >>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ >>>>> >>>> + (elm); \ >>>>> >>>> + (elm) = \ >>>>> >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) >>>>> >>>> +/* >>>>> >>>> + * Tail queue insertion using pointer arithmetic. >>>>> >>>> + */ >>>>> >>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >>>>> >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ >>>>> >>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ >>>>> >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ >>>>> >>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ >>>>> >>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ >>>>> >>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ >>>>> >>>> +} while (/*CONSTCOND*/0) >>>> >>> >>>> >>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like: >>>> >>> >>>> >>> struct QTAILQDummy { >>>> >>> char dummy; >>>> >>> }; >>>> >>> >>>> >>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) >>>> >>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; >>>> >>> >>>> >>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>>> >>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ >>>> >>> (elm); \ >>>> >>> (elm) = \ >>>> >>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next >>>> >>> >>>> >>> and then I think elm gets declared as a struct QTAILQDummy. >>>> >>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. >>>> >>> >>>> >>> Would that work? >>>> >>> >>> >> It is intended for QTAILQ of any type. So type is not available. >> > >> > I think it might be possible to do it generally. >> > > If we have type, then we can use what is there already, and don't need a > pointer arithmetic based approach. Inside put/get, we only get type > layout info from vmsd, which is all about size and offset. This macro > is used inside put/get, so I am not sure how we can directly use type > here. > Dave's approach seems perfectly sane to me. Jianjun have you actually tried to make it work before writing this? Your argument does not work, because what you need from vmsd for QTAILQ_RAW_FOREACH is only .start which corresponds to the entry parameter of the macro. Dave still does the pointer arithmetic to get a pointer (char*) to the anonymous struct holding tqe_next and tqe_prev. Now since no arithmetic is done wit tqe_next and tqe_prev, only dereferencing, their pointer type does not matter all that much so we can do the and follow the pointer. Same goes for the head. Actually the QTAILQDummy is not necessary in my opinion since we can probably (did not try it out myself) do: Q_TAILQ_HEAD(QTAILQRawHead, void,) typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry; Cheers, Halil
On 10/15/2016 05:48 AM, Halil Pasic wrote: > > > On 10/14/2016 07:18 PM, Jianjun Duan wrote: >>>>>> +/* >>>>>>>>>> + * Offsets of layout of a tail queue head. >>>>>>>>>> + */ >>>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0 >>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) >>>>>>>>>> + >>>>>>>>>> +/* >>>>>>>>>> + * Offsets of layout of a tail queue element. >>>>>>>>>> + */ >>>>>>>>>> +#define QTAILQ_NEXT_OFFSET 0 >>>>>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) >>>>>>>>>> + >>>>>>>>>> +/* >>>>>>>>>> + * Tail queue tranversal using pointer arithmetic. >>>>>>>>>> + */ >>>>>>>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>>>>>>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ >>>>>>>>>> + (elm); \ >>>>>>>>>> + (elm) = \ >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) >>>>>>>>>> +/* >>>>>>>>>> + * Tail queue insertion using pointer arithmetic. >>>>>>>>>> + */ >>>>>>>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ >>>>>>>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ >>>>>>>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ >>>>>>>>>> +} while (/*CONSTCOND*/0) >>>>>>>> >>>>>>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like: >>>>>>>> >>>>>>>> struct QTAILQDummy { >>>>>>>> char dummy; >>>>>>>> }; >>>>>>>> >>>>>>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) >>>>>>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; >>>>>>>> >>>>>>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>>>>>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ >>>>>>>> (elm); \ >>>>>>>> (elm) = \ >>>>>>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next >>>>>>>> >>>>>>>> and then I think elm gets declared as a struct QTAILQDummy. >>>>>>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. >>>>>>>> >>>>>>>> Would that work? >>>>>>>> >>>>>> It is intended for QTAILQ of any type. So type is not available. >>>> >>>> I think it might be possible to do it generally. >>>> >> If we have type, then we can use what is there already, and don't need a >> pointer arithmetic based approach. Inside put/get, we only get type >> layout info from vmsd, which is all about size and offset. This macro >> is used inside put/get, so I am not sure how we can directly use type >> here. >> > > Dave's approach seems perfectly sane to me. > > Jianjun have you actually tried to make it work before writing this? > Your argument does not work, because what you need from vmsd for > QTAILQ_RAW_FOREACH is only .start which corresponds to the entry > parameter of the macro. Dave still does the pointer arithmetic to > get a pointer (char*) to the anonymous struct holding tqe_next > and tqe_prev. Now since no arithmetic is done wit tqe_next > and tqe_prev, only dereferencing, their pointer type does not matter > all that much so we can do the and follow the pointer. Same goes > for the head. > > Actually the QTAILQDummy is not necessary in my opinion since we can > probably (did not try it out myself) do: > > Q_TAILQ_HEAD(QTAILQRawHead, void,) > typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry; > Now I see. I thought Dave was using QTAILQDummy as an example. Thanks, Jianjun > Cheers, > Halil >
* Jianjun Duan (duanj@linux.vnet.ibm.com) wrote: > > > On 10/15/2016 05:48 AM, Halil Pasic wrote: > > > > > > On 10/14/2016 07:18 PM, Jianjun Duan wrote: > >>>>>> +/* > >>>>>>>>>> + * Offsets of layout of a tail queue head. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0 > >>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) > >>>>>>>>>> + > >>>>>>>>>> +/* > >>>>>>>>>> + * Offsets of layout of a tail queue element. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_NEXT_OFFSET 0 > >>>>>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) > >>>>>>>>>> + > >>>>>>>>>> +/* > >>>>>>>>>> + * Tail queue tranversal using pointer arithmetic. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ > >>>>>>>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ > >>>>>>>>>> + (elm); \ > >>>>>>>>>> + (elm) = \ > >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) > >>>>>>>>>> +/* > >>>>>>>>>> + * Tail queue insertion using pointer arithmetic. > >>>>>>>>>> + */ > >>>>>>>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ > >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ > >>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ > >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ > >>>>>>>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ > >>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ > >>>>>>>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ > >>>>>>>>>> +} while (/*CONSTCOND*/0) > >>>>>>>> > >>>>>>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like: > >>>>>>>> > >>>>>>>> struct QTAILQDummy { > >>>>>>>> char dummy; > >>>>>>>> }; > >>>>>>>> > >>>>>>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) > >>>>>>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; > >>>>>>>> > >>>>>>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \ > >>>>>>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ > >>>>>>>> (elm); \ > >>>>>>>> (elm) = \ > >>>>>>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next > >>>>>>>> > >>>>>>>> and then I think elm gets declared as a struct QTAILQDummy. > >>>>>>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. > >>>>>>>> > >>>>>>>> Would that work? > >>>>>>>> > >>>>>> It is intended for QTAILQ of any type. So type is not available. > >>>> > >>>> I think it might be possible to do it generally. > >>>> > >> If we have type, then we can use what is there already, and don't need a > >> pointer arithmetic based approach. Inside put/get, we only get type > >> layout info from vmsd, which is all about size and offset. This macro > >> is used inside put/get, so I am not sure how we can directly use type > >> here. > >> > > > > Dave's approach seems perfectly sane to me. > > > > Jianjun have you actually tried to make it work before writing this? > > Your argument does not work, because what you need from vmsd for > > QTAILQ_RAW_FOREACH is only .start which corresponds to the entry > > parameter of the macro. Dave still does the pointer arithmetic to > > get a pointer (char*) to the anonymous struct holding tqe_next > > and tqe_prev. Now since no arithmetic is done wit tqe_next > > and tqe_prev, only dereferencing, their pointer type does not matter > > all that much so we can do the and follow the pointer. Same goes > > for the head. > > > > Actually the QTAILQDummy is not necessary in my opinion since we can > > probably (did not try it out myself) do: > > > > Q_TAILQ_HEAD(QTAILQRawHead, void,) > > typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry; > > > > Now I see. I thought Dave was using QTAILQDummy as an example. If you have a new version with either that or Paolo's suggestion, it would be good; I'd like to see this set go in because I've now got a small pile of patches built on top of it using my WITH_TMP macro. Dave > > Thanks, > Jianjun > > Cheers, > > Halil > > > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
On 10/21/2016 11:51 AM, Dr. David Alan Gilbert wrote: > * Jianjun Duan (duanj@linux.vnet.ibm.com) wrote: >> >> >> On 10/15/2016 05:48 AM, Halil Pasic wrote: >>> >>> >>> On 10/14/2016 07:18 PM, Jianjun Duan wrote: >>>>>>>> +/* >>>>>>>>>>>> + * Offsets of layout of a tail queue head. >>>>>>>>>>>> + */ >>>>>>>>>>>> +#define QTAILQ_FIRST_OFFSET 0 >>>>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) >>>>>>>>>>>> + >>>>>>>>>>>> +/* >>>>>>>>>>>> + * Offsets of layout of a tail queue element. >>>>>>>>>>>> + */ >>>>>>>>>>>> +#define QTAILQ_NEXT_OFFSET 0 >>>>>>>>>>>> +#define QTAILQ_PREV_OFFSET (sizeof(void *)) >>>>>>>>>>>> + >>>>>>>>>>>> +/* >>>>>>>>>>>> + * Tail queue tranversal using pointer arithmetic. >>>>>>>>>>>> + */ >>>>>>>>>>>> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>>>>>>>>>>> + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ >>>>>>>>>>>> + (elm); \ >>>>>>>>>>>> + (elm) = \ >>>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) >>>>>>>>>>>> +/* >>>>>>>>>>>> + * Tail queue insertion using pointer arithmetic. >>>>>>>>>>>> + */ >>>>>>>>>>>> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >>>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ >>>>>>>>>>>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ >>>>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ >>>>>>>>>>>> + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ >>>>>>>>>>>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ >>>>>>>>>>>> + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ >>>>>>>>>>>> +} while (/*CONSTCOND*/0) >>>>>>>>>> >>>>>>>>>> I wonder if there's a simpler way to do this; I'm not sure this works, but something like: >>>>>>>>>> >>>>>>>>>> struct QTAILQDummy { >>>>>>>>>> char dummy; >>>>>>>>>> }; >>>>>>>>>> >>>>>>>>>> QTAILQ_HEAD(QTAILQRawHead, struct QTAILQDummy) >>>>>>>>>> typedef QTAILQ_ENTRY(struct QTAILQDummy) QTAILQRawEntry; >>>>>>>>>> >>>>>>>>>> #define QTAILQ_RAW_FOREACH(elm, head, entry) \ >>>>>>>>>> for ((elm) = ((struct QTAILQRawHead *)head)->tqh_first) \ >>>>>>>>>> (elm); \ >>>>>>>>>> (elm) = \ >>>>>>>>>> (elm) = ((QTAILQRawEntry *)((char *) (elm) + (entry)))->tqh_next >>>>>>>>>> >>>>>>>>>> and then I think elm gets declared as a struct QTAILQDummy. >>>>>>>>>> But it does avoid those FIRST_OFFSET/LAST_OFFSET/NEXT_OFFSET/PREV_OFFSET calculations. >>>>>>>>>> >>>>>>>>>> Would that work? >>>>>>>>>> >>>>>>>> It is intended for QTAILQ of any type. So type is not available. >>>>>> >>>>>> I think it might be possible to do it generally. >>>>>> >>>> If we have type, then we can use what is there already, and don't need a >>>> pointer arithmetic based approach. Inside put/get, we only get type >>>> layout info from vmsd, which is all about size and offset. This macro >>>> is used inside put/get, so I am not sure how we can directly use type >>>> here. >>>> >>> >>> Dave's approach seems perfectly sane to me. >>> >>> Jianjun have you actually tried to make it work before writing this? >>> Your argument does not work, because what you need from vmsd for >>> QTAILQ_RAW_FOREACH is only .start which corresponds to the entry >>> parameter of the macro. Dave still does the pointer arithmetic to >>> get a pointer (char*) to the anonymous struct holding tqe_next >>> and tqe_prev. Now since no arithmetic is done wit tqe_next >>> and tqe_prev, only dereferencing, their pointer type does not matter >>> all that much so we can do the and follow the pointer. Same goes >>> for the head. >>> >>> Actually the QTAILQDummy is not necessary in my opinion since we can >>> probably (did not try it out myself) do: >>> >>> Q_TAILQ_HEAD(QTAILQRawHead, void,) >>> typedef Q_TAILQ_ENTRY(void,) QTAILQRawEntry; >>> >> >> Now I see. I thought Dave was using QTAILQDummy as an example. > > If you have a new version with either that or Paolo's suggestion, > it would be good; I'd like to see this set go in because I've > now got a small pile of patches built on top of it using > my WITH_TMP macro. > I am working on it. Thanks, Jianjun > Dave > >> >> Thanks, >> Jianjun >>> Cheers, >>> Halil >>> >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >
diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h index d0e37b5..4dd0aed 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 + * _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, \ + .flags = VMS_LINKED, \ + .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..d672ae0 100644 --- a/include/qemu/queue.h +++ b/include/qemu/queue.h @@ -438,4 +438,36 @@ struct { \ #define QTAILQ_PREV(elm, headname, field) \ (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last)) +/* + * Offsets of layout of a tail queue head. + */ +#define QTAILQ_FIRST_OFFSET 0 +#define QTAILQ_LAST_OFFSET (sizeof(void *)) + +/* + * Offsets of layout of a tail queue element. + */ +#define QTAILQ_NEXT_OFFSET 0 +#define QTAILQ_PREV_OFFSET (sizeof(void *)) + +/* + * Tail queue tranversal using pointer arithmetic. + */ +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ + for ((elm) = *((void **) ((char *) (head) + QTAILQ_FIRST_OFFSET)); \ + (elm); \ + (elm) = \ + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET))) +/* + * Tail queue insertion using pointer arithmetic. + */ +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ + *((void **) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET)) = NULL; \ + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET)) = \ + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)); \ + **((void ***)((char *) (head) + QTAILQ_LAST_OFFSET)) = (elm); \ + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) = \ + (void *) ((char *) (elm) + (entry) + QTAILQ_NEXT_OFFSET); \ +} while (/*CONSTCOND*/0) + #endif /* QEMU_SYS_QUEUE_H */ diff --git a/migration/trace-events b/migration/trace-events index dfee75a..9a6ec59 100644 --- a/migration/trace-events +++ b/migration/trace-events @@ -52,6 +52,10 @@ vmstate_n_elems(const char *name, int n_elems) "%s: %d" vmstate_subsection_load(const char *parent) "%s" vmstate_subsection_load_bad(const char *parent, const char *sub, const char *sub2) "%s: %s/%s" vmstate_subsection_load_good(const char *parent) "%s" +get_qtailq(const char *name, int version_id) "%s v%d" +get_qtailq_end(const char *name, const char *reason, int val) "%s %s/%d" +put_qtailq(const char *name, int version_id) "%s v%d" +put_qtailq_end(const char *name, const char *reason) "%s %s" # migration/qemu-file.c qemu_file_fclose(void) "" diff --git a/migration/vmstate.c b/migration/vmstate.c index e8eff3f..eb68fc6 100644 --- a/migration/vmstate.c +++ b/migration/vmstate.c @@ -5,6 +5,7 @@ #include "migration/vmstate.h" #include "qemu/bitops.h" #include "qemu/error-report.h" +#include "qemu/queue.h" #include "trace.h" #include "migration/qjson.h" @@ -946,3 +947,61 @@ const VMStateInfo vmstate_info_bitmap = { .get = get_bitmap, .put = put_bitmap, }; + +/*get for QTAILQ */ +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, + VMStateField *field) +{ + int ret = 0; + const VMStateDescription *vmsd = field->vmsd; + size_t size = field->size; + size_t entry = field->start; + int version_id = field->version_id; + void *elm; + + trace_get_qtailq(vmsd->name, version_id); + if (version_id > vmsd->version_id) { + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL); + return -EINVAL; + } + if (version_id < vmsd->minimum_version_id) { + trace_get_qtailq_end(vmsd->name, "too old", -EINVAL); + return -EINVAL; + } + + while (qemu_get_byte(f)) { + elm = g_malloc(size); + ret = vmstate_load_state(f, vmsd, elm, version_id); + if (ret) { + return ret; + } + QTAILQ_RAW_INSERT_TAIL(pv, elm, entry); + } + + trace_get_qtailq_end(vmsd->name, "end", ret); + return ret; +} + +/* put for QTAILQ */ +static void put_qtailq(QEMUFile *f, void *pv, size_t unused_size, + VMStateField *field, QJSON *vmdesc) +{ + const VMStateDescription *vmsd = field->vmsd; + size_t entry = field->start; + void *elm; + + trace_put_qtailq(vmsd->name, vmsd->version_id); + + QTAILQ_RAW_FOREACH(elm, pv, entry) { + qemu_put_byte(f, true); + vmstate_save_state(f, vmsd, elm, vmdesc); + } + qemu_put_byte(f, false); + + trace_put_qtailq_end(vmsd->name, "end"); +} +const VMStateInfo vmstate_info_qtailq = { + .name = "qtailq", + .get = get_qtailq, + .put = put_qtailq, +};
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 <duanj@linux.vnet.ibm.com> --- include/migration/vmstate.h | 20 +++++++++++++++ include/qemu/queue.h | 32 ++++++++++++++++++++++++ migration/trace-events | 4 +++ migration/vmstate.c | 59 +++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 115 insertions(+)