[Qemu-devel] Re: [PATCH 08/41] msix: Store sizes that we send/receive

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

On Wed, Dec 02, 2009 at 01:04:06PM +0100, Juan Quintela wrote:
> VMstate send buffers in bytes ammonts, not bits or MSIX_ENTRY_SIZE
> multiples
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/msix.c |   13 +++++++++----
>  hw/pci.h  |    2 ++
>  2 files changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index 8dca9fd..62865d0 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -294,9 +294,11 @@ int msix_uninit(PCIDevice *dev)
>  void msix_save(PCIDevice *dev, QEMUFile *f)
>  {
>      unsigned n = dev->msix_entries_nr;
> +    dev->msix_entries_size = n * MSIX_ENTRY_SIZE;
> +    dev->msix_pending_size = (n + 7) / 8;
> 
> -    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);
> +    qemu_put_buffer(f, dev->msix_table_page, dev->msix_entries_size);
> +    qemu_put_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, dev->msix_pending_size);
>  }
> 
>  /* Should be called after restoring the config space. */
> @@ -304,9 +306,12 @@ void msix_load(PCIDevice *dev, QEMUFile *f)
>  {
>      unsigned n = dev->msix_entries_nr;
> 
> +    dev->msix_entries_size = n * MSIX_ENTRY_SIZE;
> +    dev->msix_pending_size = (n + 7) / 8;
> +
>      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);
> +    qemu_get_buffer(f, dev->msix_table_page, dev->msix_entries_size);
> +    qemu_get_buffer(f, dev->msix_table_page + MSIX_PAGE_PENDING, dev->msix_pending_size);
>  }
> 
>  /* Does device support MSI-X? */
> diff --git a/hw/pci.h b/hw/pci.h
> index 0baf69b..c67cc70 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -241,6 +241,8 @@ struct PCIDevice {
>      uint32_t msix_bar_size;
>      /* Version id needed for VMState */
>      int32_t version_id;
> +    int32_t msix_entries_size;
> +    int32_t msix_pending_size;
>  };
> 
>  PCIDevice *pci_register_device(PCIBus *bus, const char *name,

So, I think this is another example of a problem where
vmstate macros derive format from internal
datastructures used.

We have a similar problem with pci interrupt states now.
I do not think making data structures used at runtime
match savevm format is a good solution.

A possible cleaner solution would be to add a variant
of _pre/_post callbacks that get an intermediate
structure and convert e.g. PCIDevice to PCIDeviceVMstate.
Vmstate macros would then operate on that intermediate
structure.

-- 
MST