Re: [Qemu-devel] [PATCH v2 01/10] qapi: add Visitor interfaces for uint*_t and int*_t

[Anthony Liguori <anthony@codemonkey.ws>]

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?  I'm not sure 
if I'm just being dense, but I'm having a really hard time understanding what 
you're suggesting.  How I see this evolving with Mike's series is:

struct DeviceStateClass {
    ObjectClass parent;

    void (*load_state)(DeviceState *obj, Visitor *v, const char *name,
                       Error **errp);
    void (*save_state)(DeviceState *obj, Visitor *v, const char *name,
                       Error **errp);
};

The PCIDevice base class would expose:

/* protected */
void pci_load_state(PCIDevice *obj, Visitor *v, const char *name,
                     Error **errp)
{
     visit_start_struct(v, "PCIDevice", name, errp);
     visit_type_int8(v, &obj->reg[0], "reg[0]", errp);
     ..
     visit_end_struct(v, errp);
}

Or:

static VMStateDescription pci_device_desc = {
   .name = "PCIDevice",
   .fields = {
      VMSTATE_UINT8(PCIDevice, reg[0]),
      ...
   }
};

void pci_load_state(PCIDevice *obj, Visitor *v, const char *name,
                     Error **errp)
{
     visit_with_vmstate(v, obj, &pci_device_desc, name, errp);
}

A subclass would do:

static void my_device_load_state(DeviceState *obj, Visitor *v, const char *name,
                                  Error **errp)
{
     MyDevice *s = MY_DEVICE(obj);
     visit_start_struct(v, "MyDevice", name, errp);
     pci_load_state(PCI_DEVICE(obj), v, "super", errp);
     visit_end_struct(v, errp);
}

static void my_device_class_init(ObjectClass *klass, void *data)
{
    DeviceClass *dc = DEVICE_CLASS(klass);

    dc->load_state = my_device_load_state;
    ...
}

There's no reference at all to QEMUFile.  The load_state/save_state methods can 
be exposed as a "state" property too.

Once the series 2/4 lands and Mike's series, implementing this should be very 
straight forward.

Regards,

Anthony Liguori