From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53157) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bzifR-0000TJ-0b for qemu-devel@nongnu.org; Thu, 27 Oct 2016 07:16:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bzifN-0008Ni-0H for qemu-devel@nongnu.org; Thu, 27 Oct 2016 07:16:17 -0400 Received: from mx0b-001b2d01.pphosted.com ([148.163.158.5]:35450 helo=mx0a-001b2d01.pphosted.com) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1bzifM-0008NL-Rb for qemu-devel@nongnu.org; Thu, 27 Oct 2016 07:16:12 -0400 Received: from pps.filterd (m0098416.ppops.net [127.0.0.1]) by mx0b-001b2d01.pphosted.com (8.16.0.17/8.16.0.17) with SMTP id u9RBE4qK007242 for ; Thu, 27 Oct 2016 07:16:12 -0400 Received: from e06smtp13.uk.ibm.com (e06smtp13.uk.ibm.com [195.75.94.109]) by mx0b-001b2d01.pphosted.com with ESMTP id 26bfgwcfs7-1 (version=TLSv1.2 cipher=AES256-SHA bits=256 verify=NOT) for ; Thu, 27 Oct 2016 07:16:11 -0400 Received: from localhost by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 27 Oct 2016 12:16:10 +0100 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> From: Halil Pasic Date: Thu, 27 Oct 2016 13:16:04 +0200 MIME-Version: 1.0 In-Reply-To: <46f2cb18-564c-b64f-a9ca-929df3f282c0@linux.vnet.ibm.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Message-Id: <3f457449-b23e-dd9c-1ca7-711e6b828144@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [QEMU PATCH v8 2/3] migration: migrate QTAILQ List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jianjun Duan , "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/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? Halil