Re: [Qemu-devel] [PATCH 01/12] virtio: add VIRTIO_DEF_DEVICE_VMSD macro

[Paolo Bonzini]

On 30/09/2016 16:19, Halil Pasic wrote:
> In most cases the functions passed to VMSTATE_VIRTIO_DEVICE
> only call the virtio_load and virtio_save wrappers. Some include some
> pre- and post- massaging too. The massaging is better expressed
> as such in the VMStateDescription.
> 
> Let us introduce a new macro called VIRTIO_DEF_DEVICE_VMSD and replace
> VMSTATE_VIRTIO_DEVICE with it gradually.
> 
> Signed-off-by: Halil Pasic <address@hidden>
> ---
>  hw/virtio/virtio.c         | 15 +++++++++++++++
>  include/hw/virtio/virtio.h | 25 +++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 18ce333..ca0a780 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1622,6 +1622,21 @@ void virtio_vmstate_save(QEMUFile *f, void *opaque, 
> size_t size)
>      virtio_save(VIRTIO_DEVICE(opaque), f);
>  }
>  
> +/* A wrapper for use as a VMState .put function */
> +void virtio_save_as_vmsi_put(QEMUFile *f, void *opaque, size_t size)
> +{
> +    virtio_save(VIRTIO_DEVICE(opaque), f);
> +}
> +
> +/* A wrapper for use as a VMState .get function */
> +int virtio_load_as_vmsi_get(QEMUFile *f, void *opaque, size_t size)
> +{
> +    VirtIODevice *vdev = VIRTIO_DEVICE(opaque);
> +    DeviceClass *dc = DEVICE_CLASS(VIRTIO_DEVICE_GET_CLASS(vdev));
> +
> +    return virtio_load(vdev, f, dc->vmsd->version_id);
> +}
> +
>  static int virtio_set_features_nocheck(VirtIODevice *vdev, uint64_t val)
>  {
>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 888c8de..01de49b 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -176,6 +176,31 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq);
>  
>  void virtio_save(VirtIODevice *vdev, QEMUFile *f);
>  void virtio_vmstate_save(QEMUFile *f, void *opaque, size_t size);
> +void virtio_save_as_vmsi_put(QEMUFile *f, void *opaque, size_t size);
> +int virtio_load_as_vmsi_get(QEMUFile *f, void *opaque, size_t size);

These can be called virtio_device_get and virtio_device_put, and can be
static if...

> +
> +#define VMSTATE_VIRTIO_FIELD \
> +    {                                         \
> +        .name = "virtio",                     \
> +        .info = &(const VMStateInfo) {        \
> +            .name = "virtio",                 \
> +            .get = virtio_load_as_vmsi_get,   \
> +            .put = virtio_save_as_vmsi_put,   \
> +        },                                    \
> +        .flags = VMS_SINGLE,                  \
> +    }

... you only export the VMStateInfo.

Also please define the macro like the rest of the VMSTATE macros, i.e.

#define VMSTATE_PCI_DEVICE(_field, _state) {                           \
    .name       = (stringify(_field)),                                 \
    .size       = sizeof(VirtIODevice),                                \
    .vmsd       = &vmstate_virtio_device,                              \
    .flags      = VMS_SINGLE,                                          \
    .offset     = vmstate_offset_value(_state, _field, VirtIODevice),  \
}


> +#define VIRTIO_DEF_DEVICE_VMSD(devname, v, ...) \
> +    static const VMStateDescription vmstate_virtio_ ## devname = { \
> +        .name = "virtio-" #devname ,          \
> +        .minimum_version_id = v,              \
> +        .version_id = v,                      \
> +        .fields = (VMStateField[]) {          \
> +            VMSTATE_VIRTIO_FIELD,             \
> +            VMSTATE_END_OF_LIST()             \
> +        },                                    \
> +        __VA_ARGS__                           \
> +    };

and here, don't use the macro, using its expansion everywhere instead.

Paolo

>  #define VMSTATE_VIRTIO_DEVICE(devname, v, getf, putf) \
>      static const VMStateDescription vmstate_virtio_ ## devname = { \
>