[Qemu-devel] Re: [PATCH 20/41] virtio: abstract test for save/load values

[Michael S. Tsirkin]

This does not make a lot of sense separately:
you are changing routines that you yourself
then remove.  And there is another helper not_msix
which you add in the next patch.

Maybe just roll this patch in
with the next one and be done with it.



On Wed, Dec 02, 2009 at 01:04:18PM +0100, Juan Quintela wrote:
> 
> Signed-off-by: Juan Quintela <address@hidden>
> ---
>  hw/virtio.c |   27 ++++++++++++++++++++-------
>  1 files changed, 20 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio.c b/hw/virtio.c
> index 2b36cad..5497716 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -615,6 +615,20 @@ void virtio_notify_config(VirtIODevice *vdev)
>      virtio_notify_vector(vdev, vdev->config_vector);
>  }
> 
> +static bool is_virtio_pci(void *opaque, int version_id)
> +{
> +    VirtIODevice *vdev = opaque;
> +
> +    return vdev->type == VIRTIO_PCI;
> +}
> +
> +static bool is_virtio_msix(void *opaque, int version_id)
> +{
> +    VirtIODevice *vdev = opaque;
> +    return (vdev->type == VIRTIO_PCI) &&

Reuse is_virtio_msix here? Or at least do not
add () around ==.

> +        virtio_pci_msix_present(vdev->binding_opaque);
> +}
> +
>  static void virtio_pre_save(void *opaque)
>  {
>      VirtIODevice *vdev = opaque;
> @@ -633,7 +647,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
> 
>      virtio_pre_save(vdev);
> 
> -    if (vdev->type == VIRTIO_PCI)
> +    if (is_virtio_pci(vdev, 1))
>          vmstate_save_state(f, &vmstate_virtio_pci_config, 
> vdev->binding_opaque);
> 
>      qemu_put_8s(f, &vdev->status);
> @@ -649,8 +663,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>          qemu_put_be32s(f, &vdev->vq[i].vring.num);
>          qemu_put_be64s(f, &vdev->vq[i].pa);
>          qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
> -        if (vdev->type == VIRTIO_PCI &&
> -            virtio_pci_msix_present(vdev->binding_opaque)) {
> +        if (is_virtio_msix(vdev, 1)) {
>              qemu_put_be16s(f, &vdev->vq[i].vector);
>          }
>      }
> @@ -682,11 +695,12 @@ static int virtio_post_load(void *opaque, int 
> version_id)
>      return 0;
>  }
> 
> +

Not needed.

>  int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>  {
>      int i, ret;
> 
> -    if (vdev->type == VIRTIO_PCI) {
> +    if (is_virtio_pci(vdev, 1)) {
>          ret = vmstate_load_state(f, &vmstate_virtio_pci_config, 
> vdev->binding_opaque,
>                                   vmstate_virtio_pci_config.version_id);
>          if (ret)
> @@ -707,9 +721,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>          qemu_get_be64s(f, &vdev->vq[i].pa);
>          qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
> 
> -        if (vdev->type == VIRTIO_PCI &&
> -            virtio_pci_msix_present(vdev->binding_opaque)) {
> -                qemu_get_be16s(f, &vdev->vq[i].vector);
> +        if (is_virtio_msix(vdev, 1)) {
> +            qemu_get_be16s(f, &vdev->vq[i].vector);
>          }
>      }
>      virtio_post_load(vdev, 1);
> -- 
> 1.6.5.2