From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48569) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c0D6z-0002k9-8X for qemu-devel@nongnu.org; Fri, 28 Oct 2016 15:46:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c0D6u-0001GH-9z for qemu-devel@nongnu.org; Fri, 28 Oct 2016 15:46:45 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:57166) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c0D6u-0001Fw-0y for qemu-devel@nongnu.org; Fri, 28 Oct 2016 15:46:40 -0400 Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9SJhxUZ040883 for ; Fri, 28 Oct 2016 15:46:38 -0400 Received: from e34.co.us.ibm.com (e34.co.us.ibm.com [32.97.110.152]) by mx0a-001b2d01.pphosted.com with ESMTP id 26catu97pe-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Fri, 28 Oct 2016 15:46:37 -0400 Received: from localhost by e34.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 28 Oct 2016 13:46:37 -0600 References: <1477607317-27817-1-git-send-email-duanj@linux.vnet.ibm.com> <1477607317-27817-3-git-send-email-duanj@linux.vnet.ibm.com> <20161028190657.GB2173@work-vm> From: Jianjun Duan Date: Fri, 28 Oct 2016 12:46:25 -0700 MIME-Version: 1.0 In-Reply-To: <20161028190657.GB2173@work-vm> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Message-Id: <0cd200a6-3ded-0e02-1b8b-fbbf5c0bee03@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [QEMU PATCH v9 2/3] migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert" Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, dmitry@daynix.com, peter.maydell@linaro.org, kraxel@redhat.com, mst@redhat.com, david@gibson.dropbear.id.au, pbonzini@redhat.com, veroniabahaa@gmail.com, quintela@redhat.com, amit.shah@redhat.com, mreitz@redhat.com, kwolf@redhat.com, rth@twiddle.net, aurelien@aurel32.net, leon.alrae@imgtec.com, blauwirbel@gmail.com, mark.cave-ayland@ilande.co.uk, mdroth@linux.vnet.ibm.com On 10/28/2016 12:06 PM, 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 >> --- >> 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. > >> +/* >> + * Raw access of elements of a tail entry >> + */ >> +#define QTAILQ_RAW_NEXT(elm, entry) \ >> + (*field_at_offset(elm, entry + QTAILQ_NEXT_OFFSET, void **)) >> +#define QTAILQ_RAW_PREV(elm, entry) \ >> + (*field_at_offset(elm, entry + QTAILQ_PREV_OFFSET, void ***)) >> +/* >> + * Tail queue tranversal using pointer arithmetic. >> + */ >> +#define QTAILQ_RAW_FOREACH(elm, head, entry) \ >> + for ((elm) = QTAILQ_RAW_FIRST(head); \ >> + (elm); \ >> + (elm) = QTAILQ_RAW_NEXT(elm, entry)) >> +/* >> + * Tail queue insertion using pointer arithmetic. >> + */ >> +#define QTAILQ_RAW_INSERT_TAIL(head, elm, entry) do { \ >> + 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); \ >> +} 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 d188afa..70e0472 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" >> >> @@ -948,3 +949,69 @@ const VMStateInfo vmstate_info_bitmap = { >> .get = get_bitmap, >> .put = put_bitmap, >> }; >> + >> +/* get for QTAILQ >> + * meta data about the QTAILQ is encoded in a VMStateField structure >> + */ >> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, >> + VMStateField *field) >> +{ >> + int ret = 0; >> + const VMStateDescription *vmsd = field->vmsd; >> + /* size of a QTAILQ element */ >> + size_t size = field->size; >> + /* offset of the QTAILQ entry in a QTAILQ element */ >> + size_t entry_offset = field->start; >> + int version_id = field->version_id; >> + void *elm; >> + >> + trace_get_qtailq(vmsd->name, version_id); >> + if (version_id > vmsd->version_id) { >> + error_report("%s %s", vmsd->name, "too new"); >> + trace_get_qtailq_end(vmsd->name, "too new", -EINVAL); >> + >> + return -EINVAL; >> + } >> + if (version_id < vmsd->minimum_version_id) { >> + error_report("%s %s", vmsd->name, "too old"); >> + 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_offset); >> + } >> + >> + 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; >> + /* offset of the QTAILQ entry in a QTAILQ element*/ >> + size_t entry_offset = field->start; >> + void *elm; >> + >> + trace_put_qtailq(vmsd->name, vmsd->version_id); >> + >> + QTAILQ_RAW_FOREACH(elm, pv, entry_offset) { >> + 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 >> > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >