On 05/10/2016 21:00, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (pbonz...@redhat.com) wrote:
>>
>>
>> On 05/10/2016 16:29, Dr. David Alan Gilbert wrote:
>>> The virtio-input conversion especially is nice and simple.
>>> The only weird case is virtio-gpu, and well virtio-gpu is the one that's
>>> odd here (compared to the rest of virtio).
>>
>> Though virtio-gpu would be the one that could become nicer without the
>> macros. :)
>
> Yes, it would.
>
>> What I don't like about the macros is that they don't allow you to
>> extend the VMStateDescription.
>
> Hmm so would this work:
>
> add an 'extra_field' that we normally leave empty:
>
> #define VIRTIO_DEF_DEVICE_VMSD(devname, v, extra_field, ...) \
> static const VMStateDescription vmstate_virtio_ ## devname = { \
> .name = "virtio-" #devname , \
> .minimum_version_id = v, \
> .version_id = v, \
> .fields = (VMStateField[]) { \
> VMSTATE_VIRTIO_FIELD, \
> extra_field \
> VMSTATE_END_OF_LIST() \
> }, \
> __VA_ARGS__ \
> };
>
> so the normal case would gain messy commas:
>
> VIRTIO_DEF_DEVICE_VMSD(console,3, /* empty */)
> the case with pre/post would be:
> VIRTIO_DEF_DEVICE_VMSD(vhost_vsock, VHOST_VSOCK_SAVEVM_VERSION, /* empty
> */,
> .pre_save = vhost_vsock_pre_save, .post_load = vhost_vsock_post_load)
>
> .....
>
> Define a macro for this field so it's passed as one entry:
>
> #define VMSTATE_VIRTIO_GPU_FIELD \
> { \
> .name = "virtio-gpu", \
> .info = &(const VMStateInfo) { \
> .name = "virtio-gpu", \
> .get = virtio_gpu_load, \
> .put = virtio_gpu_save, \
> }, \
> .flags = VMS_SINGLE, \
> } /* device */,
>
> VIRTIO_DEF_DEVICE_VMSD(virtio-gpu, VIRTIO_GPU_VM_VERSION,
> VMSTATE_VIRTIO_GPU_FIELD)
I would just expand the macro in the virtio-gpu case. For now what
Halil did is fine.
Paolo
