From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47821) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buL0b-00045D-4Z for qemu-devel@nongnu.org; Wed, 12 Oct 2016 10:59:54 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buL0W-0006eJ-VD for qemu-devel@nongnu.org; Wed, 12 Oct 2016 10:59:52 -0400 Date: Wed, 12 Oct 2016 15:59:37 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20161012145936.GB13343@work-vm> References: <1475519097-27611-1-git-send-email-duanj@linux.vnet.ibm.com> <1475519097-27611-4-git-send-email-duanj@linux.vnet.ibm.com> <60b19618-e725-710f-f512-3f6df471f6f2@linux.vnet.ibm.com> <8b8b373d-34e9-00f3-9657-0ac4b5ade586@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <8b8b373d-34e9-00f3-9657-0ac4b5ade586@linux.vnet.ibm.com> Subject: Re: [Qemu-devel] [QEMU PATCH v5 3/6] migration: extend VMStateInfo List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Halil Pasic Cc: Paolo Bonzini , Jianjun Duan , qemu-devel@nongnu.org, veroniabahaa@gmail.com, peter.maydell@linaro.org, mst@redhat.com, quintela@redhat.com, mark.cave-ayland@ilande.co.uk, mdroth@linux.vnet.ibm.com, mreitz@redhat.com, blauwirbel@gmail.com, amit.shah@redhat.com, qemu-ppc@nongnu.org, kraxel@redhat.com, kwolf@redhat.com, dmitry@daynix.com, rth@twiddle.net, leon.alrae@imgtec.com, aurelien@aurel32.net, david@gibson.dropbear.id.au * Halil Pasic (pasic@linux.vnet.ibm.com) wrote: > > > On 10/12/2016 02:07 PM, Paolo Bonzini wrote: > > > > On 12/10/2016 13:59, Halil Pasic wrote: > >> > IMHO this would: > >> > * allow us to keep the good old MVStateInfo objects unmodified and > >> > the semantic of VMStateInfo unchanged > >> > * make clear that VMStateLinked does not care about the calculated size > >> > and provide a new anchor for documentation > >> > * if overloading the semantic of VMStateField.start is not desired we > >> > could put it into VMStateLinked, if needed we could also put more > >> > stuff in there > >> > * have clearer separation between special handling for (linked/certain) > >> > data structures and the usual scenario with the DAG. > > No, I disagree. We shouldn't be worried about making intrusive changes > > to all invocations or declarations, if that leads to a simpler API. > > Paolo I agree on a theoretical level. It's just I do not see why this > particular change makes the API simpler. In my opinion this complicates > things because now all VMStateInfo's can do funky stuff. Having additional > state you can poke is rarely a simplification. Same goes for lots > of arguments especially if some of them are barely ever used. These > additional parameters contribute nothing for the large majority > of the cases (except maybe some head scratching when reading > the code). I think it might depend how many VMStateInfo's we have. My ideal rule would be there are no .get/.put implementations outside of migration/ and we can trust that they would never do anything silly (right?); so the extra parameters aren't going to be misused too badly. However, we're probably quite a way from pulling all of the weirder .get/.put implementations back in. Dave > No strong opinion here, it's just that I don't understand. I think one > trait of a simple API is that it is easy to document. Unfortunately > the documentation is quite sparse and in the patch the signature > change goes undocumented. > > Well as I said, just an idea, motivated by how I understood he role of > VMStateInfo form reading the code. > > Cheers, > Halil > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK