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

[Anthony Liguori]

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