[Qemu-devel] Re: [PATCH 6/9] virtio-pci: Remove duplicate test

["Michael S. Tsirkin" <mst@redhat.com>]

On Tue, Mar 16, 2010 at 07:51:22PM +0100, Juan Quintela wrote:
> We already do the test for msix on the caller, just use that test
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>

NAK

I think we are better off not making assumptions
about caller behaviour in msix.c, virtio
will not be the only user forever.

> ---
>  hw/msix.c       |    8 --------
>  hw/virtio-pci.c |    7 ++++---
>  2 files changed, 4 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 2ca0900..867f0b1 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -323,10 +323,6 @@ void msix_save(PCIDevice *dev, QEMUFile *f)
>  {
>      unsigned n = dev->msix_entries_nr;
> 
> -    if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) {
> -        return;
> -    }
> -
>      qemu_put_buffer(f, dev->msix_table_page, n * MSIX_ENTRY_SIZE);
>      qemu_put_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / 8);
>  }
> @@ -336,10 +332,6 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
>  {
>      unsigned n = dev->msix_entries_nr;
> 
> -    if (!(dev->cap_present & QEMU_PCI_CAP_MSIX)) {
> -        return;
> -    }
> -
>      msix_free_irq_entries(dev);
>      qemu_get_buffer(f, dev->msix_table_page, n * MSIX_ENTRY_SIZE);
>      qemu_get_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, (n + 7) / 8);
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index 799f664..ce48ddc 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -115,9 +115,10 @@ static void virtio_pci_save_config(void * opaque, QEMUFile *f)
>  {
>      VirtIOPCIProxy *proxy = opaque;
>      pci_device_save(&proxy->pci_dev, f);
> -    msix_save(&proxy->pci_dev, f);
> -    if (msix_present(&proxy->pci_dev))
> +    if (msix_present(&proxy->pci_dev)) {
> +        msix_save(&proxy->pci_dev, f);
>          qemu_put_be16(f, proxy->vdev->config_vector);
> +    }
>  }
> 
>  static void virtio_pci_save_queue(void * opaque, int n, QEMUFile *f)
> @@ -135,8 +136,8 @@ static int virtio_pci_load_config(void * opaque, QEMUFile *f)
>      if (ret) {
>          return ret;
>      }
> -    msix_load(&proxy->pci_dev, f);
>      if (msix_present(&proxy->pci_dev)) {
> +        msix_load(&proxy->pci_dev, f);
>          qemu_get_be16s(f, &proxy->vdev->config_vector);
>      } else {
>          proxy->vdev->config_vector = VIRTIO_NO_VECTOR;
> -- 
> 1.6.6.1
> 
>