From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:55237) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RAkhJ-0001ac-1y for qemu-devel@nongnu.org; Mon, 03 Oct 2011 11:44:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RAkhH-0000YC-Ix for qemu-devel@nongnu.org; Mon, 03 Oct 2011 11:44:53 -0400 Received: from mail-qy0-f180.google.com ([209.85.216.180]:52824) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RAkhH-0000Y5-9D for qemu-devel@nongnu.org; Mon, 03 Oct 2011 11:44:51 -0400 Received: by qyc1 with SMTP id 1so4316072qyc.4 for ; Mon, 03 Oct 2011 08:44:50 -0700 (PDT) Message-ID: <4E89D86D.2010201@codemonkey.ws> Date: Mon, 03 Oct 2011 10:44:45 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1316443309-23843-1-git-send-email-mdroth@linux.vnet.ibm.com> <20111003064653.GA15380@redhat.com> <4E89AFB4.8000103@us.ibm.com> <20111003132445.GB18920@redhat.com> <4E89BC1A.30208@codemonkey.ws> <20111003141140.GB19689@redhat.com> <4E89C9BA.8070404@us.ibm.com> <20111003152950.GB20141@redhat.com> In-Reply-To: <20111003152950.GB20141@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC] New Migration Protocol using Visitor Interface List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Michael Roth , qemu-devel@nongnu.org On 10/03/2011 10:29 AM, Michael S. Tsirkin wrote: > On Mon, Oct 03, 2011 at 09:42:02AM -0500, Anthony Liguori wrote: >> On 10/03/2011 09:11 AM, Michael S. Tsirkin wrote: >>> On Mon, Oct 03, 2011 at 08:43:54AM -0500, Anthony Liguori wrote: >>>>>> visit_start_array(v, "entries", errp); >>>>>> for (int i = 0; i< s->size; i++) { >>>>>> visit_type_int(v, NULL,&s->entry[i], errp); >>>>>> } >>>>>> visit_end_array(v, errp); >>>>> >>>>> Sequences can encode structures not just arrays. >>>>> How would you encode this for example: >>>>> >>>>> SEQUENCE OF { VQN: INTEGER, SEQUENCE { OPTIONAL VECTOR: INTEGER} } >>>> >>>> visit_start_array(v, "vqs", errp); >>>> for (i = 0; i< s->n_vqs; i++) { >>>> // Array elements never have a name, hence NULL name >>>> visit_start_struct(v, "VirtQueue", NULL, errp); >>>> visit_type_int(v,&s->vq[i].num, "vqn", errp); >>>> >>>> // Given this sub-struct an arbitrary name. It could also be anonymous. >>>> visit_start_struct(v, "MsixInfo", "msix_info", errp); >>>> if (s->vq[i].msix_enabled) { >>>> visit_type_int(v,&s->vq[i].vector, "vector", errp); >>> >>> Why is this a pointer to vector, btw? >> >> So you can write a single visit function that works for input or output. >> >> Think of the simple case like: >> >> void visit_simple_type(Visitor *v, SimpleType *t, const char *name, Error **errp) >> { >> visit_start_struct(v, "SimpleType", name, errp); >> visit_type_int(v,&t->a, "a", errp); >> visit_type_int(v,&t->b, "b", errp); >> visit_end_struct(v, errp); >> } > > Okay, so this actually stores the pointer to the integer somewhere? > So what is the lifetime requirement for this memory? typedef struct SimpleType { int a; int b; } SimpleType; So the Input visitor (reading off of the wire), essentially is: type_int(Visitor *v, int *value, Error **errp) { *value = read_int_off_wire; } The Output visitor (writing to the wire), is: type_int(Visitor *v, int *value, Error **errp) { write_int_to_write(*value); } > For how long must it stay around? There are two interest cases where life cycle matters: pointers to structs and lists. Right now, Visitors model all structures as pointers to structs. I've been thinking of having two separate interfaces, but for now, it takes: start_struct(Visitor *v, void **value, size_t size, const char *name, Error **errp) { } The output visitor ignores size, but the input visitor essentially does: { *value = g_malloc0(size); } For QAPI, we actually provide automatic free functions that can free structures that were allocated by a Visitor. >> For complex types like Virtio, you need to do a bit more. You >> wouldn't do a simple for () {} loop but instead use the Visitor list >> mechanism. That would eliminate the need to have to marshal n_vqs. > > Example? For a list, you need to make your list type compatible with GenericList. Once you've done that, you can do: GenericList *i; visit_start_list(m, name, errp); for (i = visit_next_list(m, (GenericList **)obj, errp); i; i = visit_next_list(m, &i, errp)) { TestStructList *native_i = (TestStructList *)i; visit_type_TestStruct(m, &native_i->value, NULL, errp); } visit_end_list(m, errp); I won't rewrite the virtio code, but hopefully it's clear how you would modify it to work this way. > >>> >>>> } >>>> visit_end_struct(v, errp); >>>> >>>> visit_end_struct(v, errp); >>>> } >>>> visit_end_array(v, errp); >>>> >>>> This would also generate JSON of: >>>> >>>> 'vqs': [ { 'vqn': 2, 'msix_info': { 'vector': 3 } } ] >>> >>> How would optional fields be handled? >> >> As far as the Visitor goes, if something is optional you just don't >> encode it. If you need to key off the presence of a field, >> presumably you could just check to see whether it succeeded or >> failed to visit that field. > > It would typically depend on the value. > > >> I'm not 100% sure if you can do a >> single input/output visitor when you have optional fields. >> >> My rough thinking is that each device would have a input/output >> visitor callback that took the same signature. That gives the >> flexibility of having two separate interfaces but in the common >> case, you just pass the same function for both. >> >>> Specifically >>> the case where first field in a sequence tells >>> you the meaning of the following ones? >> >> Can you give me the example in ASN.1? >> >> Regards, >> >> Anthony Liguori > > That would be a selection from CHOICE. > Note that CHOICE doesn't affect encoding on the wire: > BER just uses the underlying type. Ah, so that's a union value. I think we would have to decide how we wanted to model unions. Presumably, the selected CHOICE is indicated by some other field? I would think it's a good idea to use the type information to identify which selection of a CHOICE was chosen. I had implemented a union type in the old glib branch but that was based on a struct with an enum element and then the appropriate value element. Regards, Anthony Liguori > > >