From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bznsK-0002V7-Sa for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:49:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bznsH-0000RJ-Ks for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:49:56 -0400 Received: from mx0a-001b2d01.pphosted.com ([148.163.156.1]:47209) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bznsH-0000Qz-CU for qemu-devel@nongnu.org; Thu, 27 Oct 2016 12:49:53 -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 u9RGn1q5091661 for ; Thu, 27 Oct 2016 12:49:52 -0400 Received: from e36.co.us.ibm.com (e36.co.us.ibm.com [32.97.110.154]) by mx0a-001b2d01.pphosted.com with ESMTP id 26bmwxt6uw-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 27 Oct 2016 12:49:51 -0400 Received: from localhost by e36.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 27 Oct 2016 10:49:50 -0600 References: <1477428463-10569-1-git-send-email-duanj@linux.vnet.ibm.com> <1477428463-10569-3-git-send-email-duanj@linux.vnet.ibm.com> <20161026162906.GA6456@work-vm> <20161026165328.GC6456@work-vm> <26c90413-bc4f-e23b-3d9d-08dc665f60ee@linux.vnet.ibm.com> <20161026170926.GD6456@work-vm> <46f2cb18-564c-b64f-a9ca-929df3f282c0@linux.vnet.ibm.com> <3f457449-b23e-dd9c-1ca7-711e6b828144@linux.vnet.ibm.com> From: Jianjun Duan Date: Thu, 27 Oct 2016 09:49:42 -0700 MIME-Version: 1.0 In-Reply-To: <3f457449-b23e-dd9c-1ca7-711e6b828144@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Message-Id: Subject: Re: [Qemu-devel] [Qemu-ppc] [QEMU PATCH v8 2/3] migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Halil Pasic , "Dr. David Alan Gilbert" Cc: veroniabahaa@gmail.com, peter.maydell@linaro.org, mdroth@linux.vnet.ibm.com, mst@redhat.com, quintela@redhat.com, mark.cave-ayland@ilande.co.uk, qemu-devel@nongnu.org, mreitz@redhat.com, blauwirbel@gmail.com, amit.shah@redhat.com, qemu-ppc@nongnu.org, kraxel@redhat.com, kwolf@redhat.com, dmitry@daynix.com, pbonzini@redhat.com, rth@twiddle.net, leon.alrae@imgtec.com, aurelien@aurel32.net, david@gibson.dropbear.id.au On 10/27/2016 04:16 AM, Halil Pasic wrote: > > > On 10/26/2016 07:33 PM, Jianjun Duan wrote: >>>>>>>> +#define QTAILQ_FIRST_OFFSET 0 >>>>>>>>>>>>>> +#define QTAILQ_LAST_OFFSET (sizeof(void *)) >>>>>>>>>>>> >>>>>>>>>>>> OK, you might want to add some asserts somewhere in a .c just to catch if >>>>>>>>>>>> any of these offsets change. >>>>>>>>>>>> >>>>>>>>>> if the layout of QTAILQ changes, the version id for the vmsd should also >>>>>>>>>> be changed. It will break migration between different versions. However >>>>>>>>>> the operation doesn't depend on these values. >>>>>>>> >>>>>>>> No, changing layout of QTAILQ doesn't need to change the version id of vmsd; >>>>>>>> it's an internal change. But if someone does make the change and forgets >>>>>>>> to update your OFFSET macros it'll cause memory corruption. >>>>>>>> You could catch that with an assert (possibly build time). >>>>>>>> >>>>>> If we use these constant I would agree an assertion is necessary. By >>>>>> using a macro we leave the door open for change. and if someone changes >>>>>> the layout, he or she better change the macros and the version id. If an >>>>>> assertion is added, then that assertion needs to be changed to reflect >>>>>> the change, then in the unlikely situations we could have several >>>>>> version of layout/macro/assertions floating around. That is too much. SO >>>>>> version id is the best guard here. >>>> >>>> Version id's are irrelevant here. >>>> The macros are irrelevant here. >>>> I'm talking about a potential mismatch between the definition of QTAILQ_LAST_OFFSET >>>> and the definition of Q_TAILQ_HEAD. >>>> >>>> Dave >>>> >> I suppose anyone who changes the layout should also change the macro and >> version ID of the relevant vmsd. Similar issue was discussed before as >> the early version tried to generate all these offsets based on the >> element and head type. You can see in version 2 discussion. >> >> >> Thanks, >> Jianjun > > IMHO Dave is right, although a change in head/entry does not seem too > likely to me. Just want to point out that Dave's proposal (discussed here > https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg03770.html) > does not have these issues and it appears more elegant to me. Jianjun > why do you prefer doing the offsets manually? > It came to the current shape based on previous comments stretching back almost 6 months. I can change it to whatever reviewers want. Thanks, Jianjun > Halil > >