On 31/05/2016 23:53, Jianjun Duan wrote: > > > On 05/31/2016 12:54 PM, Paolo Bonzini wrote: >> >> >> ----- Original Message ----- >>> From: "Jianjun Duan" <du...@linux.vnet.ibm.com> >>> To: qemu-devel@nongnu.org >>> Cc: qemu-...@nongnu.org, du...@linux.vnet.ibm.com, dmi...@daynix.com, >>> "peter maydell" <peter.mayd...@linaro.org>, >>> kra...@redhat.com, m...@redhat.com, da...@gibson.dropbear.id.au, >>> pbonz...@redhat.com, veroniaba...@gmail.com, >>> quint...@redhat.com, "amit shah" <amit.s...@redhat.com>, mre...@redhat.com, >>> kw...@redhat.com, r...@twiddle.net, >>> aurel...@aurel32.net, "leon alrae" <leon.al...@imgtec.com>, >>> blauwir...@gmail.com, "mark cave-ayland" >>> <mark.cave-ayl...@ilande.co.uk>, mdr...@linux.vnet.ibm.com >>> Sent: Tuesday, May 31, 2016 8:02:42 PM >>> Subject: [QEMU RFC PATCH v3 4/6] Migration: migrate 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. In our approach such a structure is tagged >>> with VMS_CSTM. We then modified vmstate_save_state and vmstate_load_state >>> so that when VMS_CSTM is encountered, put and get from VMStateInfo are >>> called respectively. 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 | 22 +++++++++++++ >>> include/qemu/queue.h | 32 ++++++++++++++++++ >>> migration/vmstate.c | 79 >>> +++++++++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 133 insertions(+) >>> >>> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h >>> index 56a4171..da4ef7f 100644 >>> --- a/include/migration/vmstate.h >>> +++ b/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, >> >> Please call this VMS_LINKED. It can be adapted to other data >> structures in qemu/queue.h if there is a need later on. >> >>> }; >>> >>> struct VMStateField { >>> @@ -245,6 +247,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) >>> @@ -656,6 +659,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_CSTM, \ >>> + .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 f781aa2..003e368 100644 >>> --- a/include/qemu/queue.h >>> +++ b/include/qemu/queue.h >>> @@ -437,3 +437,35 @@ struct { >>> \ >>> (*(((struct headname *)((elm)->field.tqe_prev))->tqh_last)) >>> >>> #endif /* !QEMU_SYS_QUEUE_H_ */ >>> + >>> +/* >>> + * 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) >>> diff --git a/migration/vmstate.c b/migration/vmstate.c >>> index 644ba1f..ff56650 100644 >>> --- a/migration/vmstate.c >>> +++ b/migration/vmstate.c >>> @@ -5,7 +5,9 @@ >>> #include "migration/vmstate.h" >>> #include "qemu/bitops.h" >>> #include "qemu/error-report.h" >>> +#include "qemu/queue.h" >>> #include "trace.h" >>> +#include "migration/qjson.h" >>> >>> static void vmstate_subsection_save(QEMUFile *f, const VMStateDescription >>> *vmsd, >>> void *opaque, QJSON *vmdesc); >>> @@ -120,6 +122,8 @@ int vmstate_load_state(QEMUFile *f, const >>> VMStateDescription *vmsd, >>> if (field->flags & VMS_STRUCT) { >>> ret = vmstate_load_state(f, field->vmsd, addr, >>> field->vmsd->version_id); >>> + } else if (field->flags & VMS_CSTM) { >>> + ret = field->info->get(f, addr, size, field); >>> } else { >>> ret = field->info->get(f, addr, size, NULL); >>> >>> @@ -192,6 +196,8 @@ static const char *vmfield_get_type_name(VMStateField >>> *field) >>> >>> if (field->flags & VMS_STRUCT) { >>> type = "struct"; >>> + } else if (field->flags & VMS_CSTM) { >>> + type = "customized"; >>> } else if (field->info->name) { >>> type = field->info->name; >>> } >>> @@ -326,6 +332,8 @@ void vmstate_save_state(QEMUFile *f, const >>> VMStateDescription *vmsd, >>> } >>> if (field->flags & VMS_STRUCT) { >>> vmstate_save_state(f, field->vmsd, addr, vmdesc_loop); >>> + } else if (field->flags & VMS_CSTM) { >>> + field->info->put(f, addr, size, field, vmdesc_loop); >>> } else { >>> field->info->put(f, addr, size, NULL, NULL); >>> } >>> @@ -938,3 +946,74 @@ 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) >>> +{ >>> + bool link; >>> + 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_vmstate_load_state(vmsd->name, version_id); >>> + if (version_id > vmsd->version_id) { >>> + trace_vmstate_load_state_end(vmsd->name, "too new", -EINVAL); >>> + return -EINVAL; >>> + } >>> + if (version_id < vmsd->minimum_version_id) { >>> + trace_vmstate_load_state_end(vmsd->name, "too old", -EINVAL); >>> + return -EINVAL; >>> + } >>> + >>> + while (true) { >>> + vmstate_info_bool.get(f, &link, sizeof(bool), NULL); >> >> You can use qemu_get_byte here, and likewise qemu_put_byte below in >> put_qtailq. > > qemu_get/put is indeed better choice. >> >>> + if (!link) { >>> + break; >>> + } >>> + >>> + 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_vmstate_load_state_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) >>> +{ >>> + bool link = true; >>> + const VMStateDescription *vmsd = field->vmsd; >>> + size_t entry = field->start; >>> + void *elm; >>> + >>> + if (vmdesc) { >>> + json_prop_str(vmdesc, "vmsd_name", vmsd->name); >>> + json_prop_int(vmdesc, "version", vmsd->version_id); >>> + json_start_array(vmdesc, "fields"); >> >> You need to store the fields exactly once here, even if there are >> 0 or >1 elements. >> > Do you mean the json entries? I think it is already doing that.
In the case of 0 entries we don't go through the loop, so the JSON entries are definitely missing in that case. I'm not sure if QJSON handles duplicates in the case of 2+ entries. Thanks, Paolo