From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55479) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RdOxo-0006CP-5u for qemu-devel@nongnu.org; Wed, 21 Dec 2011 11:24:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RdOxi-0006hE-4j for qemu-devel@nongnu.org; Wed, 21 Dec 2011 11:24:20 -0500 Received: from mail-iy0-f173.google.com ([209.85.210.173]:37250) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RdOxh-0006hA-V1 for qemu-devel@nongnu.org; Wed, 21 Dec 2011 11:24:14 -0500 Received: by iagj37 with SMTP id j37so13699458iag.4 for ; Wed, 21 Dec 2011 08:24:13 -0800 (PST) Message-ID: <4EF20829.50303@codemonkey.ws> Date: Wed, 21 Dec 2011 10:24:09 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1319735193-4718-1-git-send-email-mdroth@linux.vnet.ibm.com> <1319735193-4718-2-git-send-email-mdroth@linux.vnet.ibm.com> <4EF06D87.9000809@redhat.com> <4EF092A5.3070108@codemonkey.ws> <4EF09C14.9070102@redhat.com> <4EF0F689.7090806@codemonkey.ws> <4EF1D291.8010203@redhat.com> <4EF1F0FE.4060402@codemonkey.ws> <4EF1FDB7.5040306@redhat.com> In-Reply-To: <4EF1FDB7.5040306@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Juan Quintela , qemu-devel , Michael Roth On 12/21/2011 09:39 AM, Paolo Bonzini wrote: > On 12/21/2011 03:45 PM, Anthony Liguori wrote: >> On 12/21/2011 06:35 AM, Paolo Bonzini wrote: >>> On 12/20/2011 09:56 PM, Anthony Liguori wrote: >>>>> As always, you can implement that in many ways. However, I think the >>>>> point of using Visitors is not to remove QEMUFile. >>>> >>>> Yes, it is. >>> >>> The point of using Visitors is to provide a standard representation of >>> device >>> state. QEMUFile is but one consumer of that representation, along with >>> any other >>> migration filter. >> >> Can you do a quick code mock up of how you'd expect this to work? > > void > vmstate_get(Device *obj, Visitor *v, void *opaque, const char *name) > { > /* Use the VMStateDescription in opaque to add fields to Visitor > from the struct. */ > } > > void > vmstate_set(Device *obj, Visitor *v, void *opaque, const char *name) > { > /* Use the VMStateDescription in opaque to retrieve fields from > Visitor and store them in the struct. */ > } > > void > vmstate_load_state(Device *obj, Visitor *v, QEMUFile ) > { > VMStateDescription *vmsd = ???; /* something from the DeviceClass */ > > /* Use the VMStateDescription in opaque to retrieve fields from > Visitor and store them in the struct. This is basically the > VMState interpreter currently in savevm.c, only fetching fields > from v rather than the file. */ > } > > void > vmstate_save_state(Device *obj, Visitor *v, QEMUFile *out) > { > VMStateDescription *vmsd = ???; /* something from the DeviceClass */ > > /* Use the VMStateDescription in opaque to fetch fields from > Visitor and store them in the file. This is basically the > other VMState interpreter currently in savevm.c, but fetching > fields from v rather than the struct. */ > } > > void > qdev_add_vmstate(Device *obj, VMStateDescription *vmsd) > { > qdev_add_property(obj, "vmstate", vmstate_get, vmstate_set, vmsd); > qdev_add_interface(obj, "QEMUFileSerializable", vmstate_load_state, > vmstate_save_state); Ok, I get what you're suggesting. You would like to continue to keep VMStateDescription describing both the QEMUFile format and the fields. I'm suggesting something fundamentally different. What I see us doing is breaking VMStateDescription apart into two different things. One would be a pure description of current fields. The other one would be the description of the old protocol. The later would be fed to a migration filter and the former would live off of DeviceState. By doing Mike's series, we can do this incrementally by splitting the description up, and invoking the filter post_load/pre_save. One of the reasons for this split up is because I would like to generate the first table by IDL and make the second table not dependent on structure members (so we don't end up with all the hacks we have with dummy struct fields). Regards, Anthony Liguori > } > > > savevm.c: > > Visitor *v = qmp_output_visitor_new(); > qdev_property_get(obj, "vmstate", v); > QObject *qobj = qmp_visitor_get_obj(v); > qmp_visitor_free(v); > > Visitor *v = qmp_input_visitor_new(qobj); > QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj); > s->save_state(obj, v, outfile); > qmp_visitor_free(v); > > ... > > Visitor *v = qmp_output_visitor_new(); > QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj); > s->load_state(obj, v, outfile); > QObject *qobj = qmp_visitor_get_obj(v); > qmp_visitor_free(v); > > Visitor *v = qmp_input_visitor_new(qobj); > QEMUFileSerializable *s = QEMU_FILE_SERIALIZABLE(obj); > qdev_property_set(obj, "vmstate", v); > qmp_visitor_free(v); > > --------------------- > > Yes, this makes QEMUFile serialization special. But that's because it's legacy, > and it needs to do strange things and store things beyond the contents of the > vmstate. > > Take for example "unused" fields. In Mike's implementation, if you try to pass a > QMPOutputVisitor to save_state you might get a "unused" entry that is an array > of zeros. I have no idea what happens if a single VMStateDescription has more > than one VMSTATE_UNUSED field. QEMUFileOutputVisitor completely ignores the > field names, so it probably works but would break with QMPOutputVisitor. > > Other serialization backends should not need any hooks in the devices beyond > save_state and load_state. > > Paolo >