From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34346) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b885F-0004zL-J9 for qemu-devel@nongnu.org; Wed, 01 Jun 2016 11:29:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b885B-0000MG-Ak for qemu-devel@nongnu.org; Wed, 01 Jun 2016 11:29:24 -0400 References: <1464717764-9040-1-git-send-email-duanj@linux.vnet.ibm.com> <1464717764-9040-5-git-send-email-duanj@linux.vnet.ibm.com> <142124939.18585117.1464724475643.JavaMail.zimbra@redhat.com> <574E07CD.1030908@linux.vnet.ibm.com> From: Paolo Bonzini Message-ID: <67193839-a00a-9a2e-e60a-b85dd1c6dfb5@redhat.com> Date: Wed, 1 Jun 2016 17:29:15 +0200 MIME-Version: 1.0 In-Reply-To: <574E07CD.1030908@linux.vnet.ibm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [QEMU RFC PATCH v3 4/6] Migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jianjun Duan Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org, dmitry@daynix.com, peter maydell , kraxel@redhat.com, mst@redhat.com, david@gibson.dropbear.id.au, veroniabahaa@gmail.com, quintela@redhat.com, amit shah , mreitz@redhat.com, kwolf@redhat.com, rth@twiddle.net, aurelien@aurel32.net, leon alrae , blauwirbel@gmail.com, mark cave-ayland , mdroth@linux.vnet.ibm.com On 31/05/2016 23:53, Jianjun Duan wrote: >=20 >=20 > On 05/31/2016 12:54 PM, Paolo Bonzini wrote: >> >> >> ----- Original Message ----- >>> From: "Jianjun Duan" >>> To: qemu-devel@nongnu.org >>> Cc: qemu-ppc@nongnu.org, duanj@linux.vnet.ibm.com, dmitry@daynix.com,= "peter maydell" , >>> kraxel@redhat.com, mst@redhat.com, david@gibson.dropbear.id.au, pbonz= ini@redhat.com, veroniabahaa@gmail.com, >>> quintela@redhat.com, "amit shah" , mreitz@redha= t.com, kwolf@redhat.com, rth@twiddle.net, >>> aurelien@aurel32.net, "leon alrae" , blauwirbe= l@gmail.com, "mark cave-ayland" >>> , mdroth@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 th= e >>> 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_s= tate >>> so that when VMS_CSTM is encountered, put and get from VMStateInfo ar= e >>> called respectively. This approach will be used to transfer pending_e= vents >>> and ccs_list in spapr state. >>> >>> We also create some macros in qemu/queue.h to access a QTAILQ using p= ointer >>> 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 | 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 i= n >>> * combination with one of VMS_VARRAY*. */ >>> VMS_MULTIPLY_ELEMENTS =3D 0x4000, >>> + /* For fields which need customized handling, such as QTAILQ in >>> queue.h*/ >>> + VMS_CSTM =3D 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. >> >>> }; >>> =20 >>> 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; >>> =20 >>> #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 =3D offsetof(_state, _field), = \ >>> } >>> =20 >>> +/* 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, _ne= xt) \ >>> +{ = \ >>> + .name =3D (stringify(_field)), = \ >>> + .version_id =3D (_version), = \ >>> + .vmsd =3D &(_vmsd), = \ >>> + .size =3D sizeof(_type), = \ >>> + .info =3D &vmstate_info_qtailq, = \ >>> + .flags =3D VMS_CSTM, = \ >>> + .offset =3D offsetof(_state, _field), = \ >>> + .start =3D 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)) >>> =20 >>> #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) =3D *((void **) ((char *) (head) + QTAILQ_FIRST_O= FFSET)); >>> \ >>> + (elm); >>> \ >>> + (elm) =3D >>> \ >>> + *((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))= =3D NULL; >>> \ >>> + *((void **) ((char *) (elm) + (entry) + QTAILQ_PREV_OFFSET))= =3D >>> \ >>> + *((void **) ((char *) (head) +QTAILQ_LAST_OFFSET)); >>> \ >>> + **((void ***)((char *) (head) +QTAILQ_LAST_OFFSET)) =3D (elm= ); >>> \ >>> + *((void **) ((char *) (head) + QTAILQ_LAST_OFFSET)) =3D >>> \ >>> + (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" >>> =20 >>> static void vmstate_subsection_save(QEMUFile *f, const VMStateDescri= ption >>> *vmsd, >>> void *opaque, QJSON *vmdesc); >>> @@ -120,6 +122,8 @@ int vmstate_load_state(QEMUFile *f, const >>> VMStateDescription *vmsd, >>> if (field->flags & VMS_STRUCT) { >>> ret =3D vmstate_load_state(f, field->vmsd, addr, >>> field->vmsd->version_id= ); >>> + } else if (field->flags & VMS_CSTM) { >>> + ret =3D field->info->get(f, addr, size, field); >>> } else { >>> ret =3D field->info->get(f, addr, size, NULL); >>> =20 >>> @@ -192,6 +196,8 @@ static const char *vmfield_get_type_name(VMStateF= ield >>> *field) >>> =20 >>> if (field->flags & VMS_STRUCT) { >>> type =3D "struct"; >>> + } else if (field->flags & VMS_CSTM) { >>> + type =3D "customized"; >>> } else if (field->info->name) { >>> type =3D 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_lo= op); >>> } else { >>> field->info->put(f, addr, size, NULL, NULL); >>> } >>> @@ -938,3 +946,74 @@ const VMStateInfo vmstate_info_bitmap =3D { >>> .get =3D get_bitmap, >>> .put =3D put_bitmap, >>> }; >>> + >>> +/*get for QTAILQ */ >>> +static int get_qtailq(QEMUFile *f, void *pv, size_t unused_size, >>> + VMStateField *field) >>> +{ >>> + bool link; >>> + int ret =3D 0; >>> + const VMStateDescription *vmsd =3D field->vmsd; >>> + size_t size =3D field->size; >>> + size_t entry =3D field->start; >>> + int version_id =3D 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. >=20 > qemu_get/put is indeed better choice. >> >>> + if (!link) { >>> + break; >>> + } >>> + >>> + elm =3D g_malloc(size); >>> + ret =3D 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 =3D true; >>> + const VMStateDescription *vmsd =3D field->vmsd; >>> + size_t entry =3D 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