Re: [Qemu-devel] [RFC] New Migration Protocol using Visitor Interface

[Anthony Liguori]

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

>