[19/41] virtio: use the right types for VirtQueue elements

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 hw/virtio.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

On Wed, Dec 02, 2009 at 01:04:17PM +0100, Juan Quintela wrote:
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hw/virtio.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/virtio.c b/hw/virtio.c
> index fd617ff..2b36cad 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -646,8 +646,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>      qemu_put_sbe32s(f, &vdev->num_pci_queues);
> 
>      for (i = 0; i < vdev->num_pci_queues; i++) {
> -        qemu_put_be32(f, vdev->vq[i].vring.num);
> -        qemu_put_be64(f, vdev->vq[i].pa);
> +        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)) {
> @@ -703,8 +703,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>      qemu_get_sbe32s(f, &vdev->num_pci_queues);
> 
>      for (i = 0; i < vdev->num_pci_queues; i++) {
> -        vdev->vq[i].vring.num = qemu_get_be32(f);
> -        vdev->vq[i].pa = qemu_get_be64(f);
> +        qemu_get_be32s(f, &vdev->vq[i].vring.num);
> +        qemu_get_be64s(f, &vdev->vq[i].pa);
>          qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
> 
>          if (vdev->type == VIRTIO_PCI &&

Why are these the right types?
I see:
static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
{
    qemu_put_be64(f, *pv);
}

so passing a pointer to qemu_get_be64s seems exactly equivalent to
qemu_put_be64 on value.

What am I missing?

> -- 
> 1.6.5.2

On Wed, Dec 02, 2009 at 07:24:12PM +0100, Juan Quintela wrote:
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > On Wed, Dec 02, 2009 at 01:04:17PM +0100, Juan Quintela wrote:
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  hw/virtio.c |    8 ++++----
> >>  1 files changed, 4 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/hw/virtio.c b/hw/virtio.c
> >> index fd617ff..2b36cad 100644
> >> --- a/hw/virtio.c
> >> +++ b/hw/virtio.c
> >> @@ -646,8 +646,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
> >>      qemu_put_sbe32s(f, &vdev->num_pci_queues);
> >> 
> >>      for (i = 0; i < vdev->num_pci_queues; i++) {
> >> -        qemu_put_be32(f, vdev->vq[i].vring.num);
> >> -        qemu_put_be64(f, vdev->vq[i].pa);
> >> +        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)) {
> >> @@ -703,8 +703,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
> >>      qemu_get_sbe32s(f, &vdev->num_pci_queues);
> >> 
> >>      for (i = 0; i < vdev->num_pci_queues; i++) {
> >> -        vdev->vq[i].vring.num = qemu_get_be32(f);
> >> -        vdev->vq[i].pa = qemu_get_be64(f);
> >> +        qemu_get_be32s(f, &vdev->vq[i].vring.num);
> >> +        qemu_get_be64s(f, &vdev->vq[i].pa);
> >>          qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
> >> 
> >>          if (vdev->type == VIRTIO_PCI &&
> >
> > Why are these the right types?
> > I see:
> > static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
> > {
> >     qemu_put_be64(f, *pv);
> > }
> >
> > so passing a pointer to qemu_get_be64s seems exactly equivalent to
> > qemu_put_be64 on value.
> >
> > What am I missing?
> 
> While I was porting this to vmstate, it was difficult.  vmstate always
> use the pointer functions (the other ones shouldn't have been defined at
> all, but that is another war).
> 
> It just made the change to vmstate obvious.  I had to redo it several
> times until it worked :(
> 
> Later, Juan.

Maybe try explaining this in commit log.

"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Wed, Dec 02, 2009 at 01:04:17PM +0100, Juan Quintela wrote:
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  hw/virtio.c |    8 ++++----
>>  1 files changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/hw/virtio.c b/hw/virtio.c
>> index fd617ff..2b36cad 100644
>> --- a/hw/virtio.c
>> +++ b/hw/virtio.c
>> @@ -646,8 +646,8 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>>      qemu_put_sbe32s(f, &vdev->num_pci_queues);
>> 
>>      for (i = 0; i < vdev->num_pci_queues; i++) {
>> -        qemu_put_be32(f, vdev->vq[i].vring.num);
>> -        qemu_put_be64(f, vdev->vq[i].pa);
>> +        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)) {
>> @@ -703,8 +703,8 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>>      qemu_get_sbe32s(f, &vdev->num_pci_queues);
>> 
>>      for (i = 0; i < vdev->num_pci_queues; i++) {
>> -        vdev->vq[i].vring.num = qemu_get_be32(f);
>> -        vdev->vq[i].pa = qemu_get_be64(f);
>> +        qemu_get_be32s(f, &vdev->vq[i].vring.num);
>> +        qemu_get_be64s(f, &vdev->vq[i].pa);
>>          qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
>> 
>>          if (vdev->type == VIRTIO_PCI &&
>
> Why are these the right types?
> I see:
> static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv)
> {
>     qemu_put_be64(f, *pv);
> }
>
> so passing a pointer to qemu_get_be64s seems exactly equivalent to
> qemu_put_be64 on value.
>
> What am I missing?

While I was porting this to vmstate, it was difficult.  vmstate always
use the pointer functions (the other ones shouldn't have been defined at
all, but that is another war).

It just made the change to vmstate obvious.  I had to redo it several
times until it worked :(

Later, Juan.