From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56211) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WX5yA-0002j9-A7 for qemu-devel@nongnu.org; Mon, 07 Apr 2014 05:36:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WX5y2-00019s-Gi for qemu-devel@nongnu.org; Mon, 07 Apr 2014 05:35:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:48770) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WX5y2-00019h-9V for qemu-devel@nongnu.org; Mon, 07 Apr 2014 05:35:50 -0400 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s379ZmKe019457 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Mon, 7 Apr 2014 05:35:49 -0400 Date: Mon, 7 Apr 2014 10:35:44 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20140407093543.GI2400@work-vm> References: <1396840915-10384-1-git-send-email-quintela@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1396840915-10384-1-git-send-email-quintela@redhat.com> Subject: Re: [Qemu-devel] [PATCH for 2.1 00/97] VMState simplification (massive) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org * Juan Quintela (quintela@redhat.com) wrote: > Hi > > Look at the diffstat. Almost all the additions are at > test-vmstate.c. That is the reason why it is called a simplification. > > What this series does: > - peter removal of version_minimum_id_old field when not needed (Peter) > - cleanup: based on the previous one, I removed all the unneeded > the uses on the tree. This should make your compiles > a couple of nanoseconds faster. > - once there, fixed the indentation of the .fields line, to a canonical > .fields = (VMStateField[]) > - mst simplifications for vmstate engine > > And now, the big cleanup. > - Patches only do one thing, to make easy the review. > > - Added test for all VMSTATE_FOO() definitions > (well, I am lying, VMSTATE_STRUCT* are still missing, will come soon) > - We had two ways to make a field optional > VMSTATE_INT64_V(field, state, version) > and > VMSTATE_INT64_TEST(field, state, test) > > We can do the version one with one test like: > > static inline bool vmstate_5_plus(void *opaque, int version_id) > { > return version_id >= 5; > } > > and then change: > VMSTATE_INT64_V(field, state, 5); > > into > VMSTATE_INT64_TEST(field, state, vmstate_5_plus); I'm not sure if I like this; while I'm OK with the idea of changing the implementation of VMSTATE_INT64_V to use that function trick internally, it seems like we're discouraging providing easy to parse/record versionining info out of the tree. Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK