From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58942) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5bp0-0000MT-CL for qemu-devel@nongnu.org; Wed, 25 May 2016 12:38:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b5bov-00035g-AW for qemu-devel@nongnu.org; Wed, 25 May 2016 12:38:14 -0400 Received: from e31.co.us.ibm.com ([32.97.110.149]:44140) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b5bou-00035G-Vq for qemu-devel@nongnu.org; Wed, 25 May 2016 12:38:09 -0400 Received: from localhost by e31.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 25 May 2016 10:38:05 -0600 References: <1464112509-21806-1-git-send-email-duanj@linux.vnet.ibm.com> <1464112509-21806-5-git-send-email-duanj@linux.vnet.ibm.com> From: Jianjun Duan Message-ID: <5745D4E7.2000003@linux.vnet.ibm.com> Date: Wed, 25 May 2016 09:37:59 -0700 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [QEMU RFC PATCH v2 4/6] Migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini , qemu-devel@nongnu.org Cc: quintela@redhat.com, mdroth@linux.vnet.ibm.com, qemu-ppc@nongnu.org, amit.shah@redhat.com, david@gibson.dropbear.id.au On 05/25/2016 01:14 AM, Paolo Bonzini wrote: > > > On 24/05/2016 19:55, Jianjun Duan wrote: >> +/* >> + * Offsets of layout of a tail queue head. >> + */ >> +#define QTAILQ_FIRST_OFFSET(head_type) \ >> + ((size_t) ((char *) &((head_type *)0)->tqh_first - (char *)0)) >> + >> +#define QTAILQ_LAST_OFFSET(head_type) \ >> + ((size_t) ((char *) &((head_type *)0)->tqh_last - (char *)0)) >> + >> +/* >> + * Offsets of layout of a tail queue element. >> + */ >> +#define QTAILQ_NEXT_OFFSET(ele_type, field) \ >> + ((size_t) ((char *) &((ele_type *)0)->field.tqe_next - \ >> + (char *) &((ele_type *)0)->field)) >> + >> +#define QTAILQ_PREV_OFFSET(ele_type, field) \ >> + ((size_t) ((char *) &((ele_type *)0)->field.tqe_prev - \ >> + (char *) &((ele_type *)0)->field)) > > Please use offsetof. OK. > >> + /* >> + * Following 3 fields are for VMStateField which needs customized handling, >> + * such as QTAILQ in qemu/queue.h, lists, and tree. >> + */ >> + const void *meta_data; >> + int (*extend_get)(QEMUFile *f, const void *metadata, void *opaque); >> + void (*extend_put)(QEMUFile *f, const void *metadata, void *opaque, >> + QJSON *vmdesc); >> } VMStateField; > > Do not add these two function pointers to VMStateField, instead add > QJSON* and VMStateField* arguments as needed to VMStateInfo's get and put. > That is definitely the ideal way. However, VMStateInfo's get/put are already used extensively. If I change them, I need change all the calling sites of them. Not very sure about whether it will be welcomed. >> +#define VMSTATE_QTAILQ_METADATA(_field, _state, _type, _next, _vmsd) { \ >> + .first = QTAILQ_FIRST_OFFSET(typeof_field(_state, _field)), \ >> + .last = QTAILQ_LAST_OFFSET(typeof_field(_state, _field)), \ >> + .next = QTAILQ_NEXT_OFFSET(_type, _next), \ >> + .prev = QTAILQ_PREV_OFFSET(_type, _next), \ >> + .entry = offsetof(_type, _next), \ >> + .size = sizeof(_type), \ >> + .vmsd = &(_vmsd), \ >> +} > > .last and .prev are unnecessary, since they come just after .first and > .next (and their use is hidden behind QTAILQ_RAW_*). .first and .next > can be placed in .offset and .num_offset respectively. So you don't > really need an extra metadata struct. > > If you prefer you could have something like > > union { > size_t num_offset; /* VMS_VARRAY */ > size_t size_offset; /* VMS_VBUFFER */ > size_t next_offset; /* VMS_TAILQ */ > } offsets; > Actually I explored the approach in which I use a VMSD to encode all the information. But a VMSD describes actual memory layout. Interpreting it another way could be confusing. One of the assumption about QTAILQ is that we can only use the interfaces defined in queue.h to access it. I intend not to depend on its actually layout. From this perspective, these 6 parameters are needed. > Thanks, > > Paolo > Thanks, Jianjun