diff mbox

[2/4] virtio: add wrapper for saving/restoring virtqueue elements

Message ID 1355149790-8125-3-git-send-email-aliguori@us.ibm.com
State New
Headers show

Commit Message

Anthony Liguori Dec. 10, 2012, 2:29 p.m. UTC 
Putting raw structures on the wire is bad news.  Add a wrapper and use it.

Note that in virtio-serial-bus, we were mapping both the in and out vectors as
writable.  This is a bug that is fixed by this change.  I checked the revision
history, it has been there since the code was first added and does not appear
to be intentional.

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
 hw/virtio-blk.c        |  9 ++-------
 hw/virtio-serial-bus.c | 10 ++--------
 hw/virtio.c            | 13 +++++++++++++
 hw/virtio.h            |  4 ++++
 4 files changed, 21 insertions(+), 15 deletions(-)

Comments

Paolo Bonzini Dec. 14, 2012, 11:46 a.m. UTC | #1 
Il 10/12/2012 15:29, Anthony Liguori ha scritto:
> Putting raw structures on the wire is bad news.  Add a wrapper and use it.
> 
> Note that in virtio-serial-bus, we were mapping both the in and out vectors as
> writable.  This is a bug that is fixed by this change.  I checked the revision
> history, it has been there since the code was first added and does not appear
> to be intentional.
> 
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
>  hw/virtio-blk.c        |  9 ++-------
>  hw/virtio-serial-bus.c | 10 ++--------
>  hw/virtio.c            | 13 +++++++++++++
>  hw/virtio.h            |  4 ++++
>  4 files changed, 21 insertions(+), 15 deletions(-)

virtio-scsi is missing; see virtio_scsi_load_request and
virtio_scsi_save_request.

Paolo

> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index e25cc96..7ab174f 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -555,7 +555,7 @@ static void virtio_blk_save(QEMUFile *f, void *opaque)
>      
>      while (req) {
>          qemu_put_sbyte(f, 1);
> -        qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
> +        virtio_put_virt_queue_element(f, &req->elem);
>          req = req->next;
>      }
>      qemu_put_sbyte(f, 0);
> @@ -576,14 +576,9 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
>  
>      while (qemu_get_sbyte(f)) {
>          VirtIOBlockReq *req = virtio_blk_alloc_request(s);
> -        qemu_get_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
> +        virtio_get_virt_queue_element(f, &req->elem);
>          req->next = s->rq;
>          s->rq = req;
> -
> -        virtqueue_map_sg(req->elem.in_sg, req->elem.in_addr,
> -            req->elem.in_num, 1);
> -        virtqueue_map_sg(req->elem.out_sg, req->elem.out_addr,
> -            req->elem.out_num, 0);
>      }
>  
>      return 0;
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index 155da58..aa1ded0 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -629,8 +629,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
>              qemu_put_be32s(f, &port->iov_idx);
>              qemu_put_be64s(f, &port->iov_offset);
>  
> -            qemu_put_buffer(f, (unsigned char *)&port->elem,
> -                            sizeof(port->elem));
> +            virtio_put_virt_queue_element(f, &port->elem);
>          }
>      }
>  }
> @@ -731,12 +730,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>                  qemu_get_be32s(f, &port->iov_idx);
>                  qemu_get_be64s(f, &port->iov_offset);
>  
> -                qemu_get_buffer(f, (unsigned char *)&port->elem,
> -                                sizeof(port->elem));
> -                virtqueue_map_sg(port->elem.in_sg, port->elem.in_addr,
> -                                 port->elem.in_num, 1);
> -                virtqueue_map_sg(port->elem.out_sg, port->elem.out_addr,
> -                                 port->elem.out_num, 1);
> +                virtio_get_virt_queue_element(f, &port->elem);
>  
>                  /*
>                   *  Port was throttled on source machine.  Let's
> diff --git a/hw/virtio.c b/hw/virtio.c
> index f40a8c5..8eb8f69 100644
> --- a/hw/virtio.c
> +++ b/hw/virtio.c
> @@ -875,6 +875,19 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>      return 0;
>  }
>  
> +void virtio_put_virt_queue_element(QEMUFile *f, const VirtQueueElement *elem)
> +{
> +    qemu_put_buffer(f, (unsigned char*)elem, sizeof(*elem));
> +}
> +
> +void virtio_get_virt_queue_element(QEMUFile *f, VirtQueueElement *elem)
> +{
> +    qemu_get_buffer(f, (unsigned char *)elem, sizeof(*elem));
> +
> +    virtqueue_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1);
> +    virtqueue_map_sg(elem->out_sg, elem->out_addr, elem->out_num, 0);
> +}
> +
>  void virtio_cleanup(VirtIODevice *vdev)
>  {
>      qemu_del_vm_change_state_handler(vdev->vmstate);
> diff --git a/hw/virtio.h b/hw/virtio.h
> index 7c17f7b..4af8239 100644
> --- a/hw/virtio.h
> +++ b/hw/virtio.h
> @@ -159,6 +159,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f);
>  
>  int virtio_load(VirtIODevice *vdev, QEMUFile *f);
>  
> +void virtio_put_virt_queue_element(QEMUFile *f, const VirtQueueElement *elem);
> +
> +void virtio_get_virt_queue_element(QEMUFile *f, VirtQueueElement *elem);
> +
>  void virtio_cleanup(VirtIODevice *vdev);
>  
>  void virtio_notify_config(VirtIODevice *vdev);
>
Anthony Liguori Dec. 14, 2012, 1:54 p.m. UTC | #2 
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 10/12/2012 15:29, Anthony Liguori ha scritto:
>> Putting raw structures on the wire is bad news.  Add a wrapper and use it.
>> 
>> Note that in virtio-serial-bus, we were mapping both the in and out vectors as
>> writable.  This is a bug that is fixed by this change.  I checked the revision
>> history, it has been there since the code was first added and does not appear
>> to be intentional.
>> 
>> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  hw/virtio-blk.c        |  9 ++-------
>>  hw/virtio-serial-bus.c | 10 ++--------
>>  hw/virtio.c            | 13 +++++++++++++
>>  hw/virtio.h            |  4 ++++
>>  4 files changed, 21 insertions(+), 15 deletions(-)
>
> virtio-scsi is missing; see virtio_scsi_load_request and
> virtio_scsi_save_request.

Indeed, thanks.

Regards,

Anthony Liguori

>
> Paolo
>
>> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
>> index e25cc96..7ab174f 100644
>> --- a/hw/virtio-blk.c
>> +++ b/hw/virtio-blk.c
>> @@ -555,7 +555,7 @@ static void virtio_blk_save(QEMUFile *f, void *opaque)
>>      
>>      while (req) {
>>          qemu_put_sbyte(f, 1);
>> -        qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
>> +        virtio_put_virt_queue_element(f, &req->elem);
>>          req = req->next;
>>      }
>>      qemu_put_sbyte(f, 0);
>> @@ -576,14 +576,9 @@ static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
>>  
>>      while (qemu_get_sbyte(f)) {
>>          VirtIOBlockReq *req = virtio_blk_alloc_request(s);
>> -        qemu_get_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
>> +        virtio_get_virt_queue_element(f, &req->elem);
>>          req->next = s->rq;
>>          s->rq = req;
>> -
>> -        virtqueue_map_sg(req->elem.in_sg, req->elem.in_addr,
>> -            req->elem.in_num, 1);
>> -        virtqueue_map_sg(req->elem.out_sg, req->elem.out_addr,
>> -            req->elem.out_num, 0);
>>      }
>>  
>>      return 0;
>> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
>> index 155da58..aa1ded0 100644
>> --- a/hw/virtio-serial-bus.c
>> +++ b/hw/virtio-serial-bus.c
>> @@ -629,8 +629,7 @@ static void virtio_serial_save(QEMUFile *f, void *opaque)
>>              qemu_put_be32s(f, &port->iov_idx);
>>              qemu_put_be64s(f, &port->iov_offset);
>>  
>> -            qemu_put_buffer(f, (unsigned char *)&port->elem,
>> -                            sizeof(port->elem));
>> +            virtio_put_virt_queue_element(f, &port->elem);
>>          }
>>      }
>>  }
>> @@ -731,12 +730,7 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
>>                  qemu_get_be32s(f, &port->iov_idx);
>>                  qemu_get_be64s(f, &port->iov_offset);
>>  
>> -                qemu_get_buffer(f, (unsigned char *)&port->elem,
>> -                                sizeof(port->elem));
>> -                virtqueue_map_sg(port->elem.in_sg, port->elem.in_addr,
>> -                                 port->elem.in_num, 1);
>> -                virtqueue_map_sg(port->elem.out_sg, port->elem.out_addr,
>> -                                 port->elem.out_num, 1);
>> +                virtio_get_virt_queue_element(f, &port->elem);
>>  
>>                  /*
>>                   *  Port was throttled on source machine.  Let's
>> diff --git a/hw/virtio.c b/hw/virtio.c
>> index f40a8c5..8eb8f69 100644
>> --- a/hw/virtio.c
>> +++ b/hw/virtio.c
>> @@ -875,6 +875,19 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f)
>>      return 0;
>>  }
>>  
>> +void virtio_put_virt_queue_element(QEMUFile *f, const VirtQueueElement *elem)
>> +{
>> +    qemu_put_buffer(f, (unsigned char*)elem, sizeof(*elem));
>> +}
>> +
>> +void virtio_get_virt_queue_element(QEMUFile *f, VirtQueueElement *elem)
>> +{
>> +    qemu_get_buffer(f, (unsigned char *)elem, sizeof(*elem));
>> +
>> +    virtqueue_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1);
>> +    virtqueue_map_sg(elem->out_sg, elem->out_addr, elem->out_num, 0);
>> +}
>> +
>>  void virtio_cleanup(VirtIODevice *vdev)
>>  {
>>      qemu_del_vm_change_state_handler(vdev->vmstate);
>> diff --git a/hw/virtio.h b/hw/virtio.h
>> index 7c17f7b..4af8239 100644
>> --- a/hw/virtio.h
>> +++ b/hw/virtio.h
>> @@ -159,6 +159,10 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f);
>>  
>>  int virtio_load(VirtIODevice *vdev, QEMUFile *f);
>>  
>> +void virtio_put_virt_queue_element(QEMUFile *f, const VirtQueueElement *elem);
>> +
>> +void virtio_get_virt_queue_element(QEMUFile *f, VirtQueueElement *elem);
>> +
>>  void virtio_cleanup(VirtIODevice *vdev);
>>  
>>  void virtio_notify_config(VirtIODevice *vdev);
>>
diff mbox

Patch

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index e25cc96..7ab174f 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -555,7 +555,7 @@  static void virtio_blk_save(QEMUFile *f, void *opaque)
     
     while (req) {
         qemu_put_sbyte(f, 1);
-        qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
+        virtio_put_virt_queue_element(f, &req->elem);
         req = req->next;
     }
     qemu_put_sbyte(f, 0);
@@ -576,14 +576,9 @@  static int virtio_blk_load(QEMUFile *f, void *opaque, int version_id)
 
     while (qemu_get_sbyte(f)) {
         VirtIOBlockReq *req = virtio_blk_alloc_request(s);
-        qemu_get_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
+        virtio_get_virt_queue_element(f, &req->elem);
         req->next = s->rq;
         s->rq = req;
-
-        virtqueue_map_sg(req->elem.in_sg, req->elem.in_addr,
-            req->elem.in_num, 1);
-        virtqueue_map_sg(req->elem.out_sg, req->elem.out_addr,
-            req->elem.out_num, 0);
     }
 
     return 0;
diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 155da58..aa1ded0 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -629,8 +629,7 @@  static void virtio_serial_save(QEMUFile *f, void *opaque)
             qemu_put_be32s(f, &port->iov_idx);
             qemu_put_be64s(f, &port->iov_offset);
 
-            qemu_put_buffer(f, (unsigned char *)&port->elem,
-                            sizeof(port->elem));
+            virtio_put_virt_queue_element(f, &port->elem);
         }
     }
 }
@@ -731,12 +730,7 @@  static int virtio_serial_load(QEMUFile *f, void *opaque, int version_id)
                 qemu_get_be32s(f, &port->iov_idx);
                 qemu_get_be64s(f, &port->iov_offset);
 
-                qemu_get_buffer(f, (unsigned char *)&port->elem,
-                                sizeof(port->elem));
-                virtqueue_map_sg(port->elem.in_sg, port->elem.in_addr,
-                                 port->elem.in_num, 1);
-                virtqueue_map_sg(port->elem.out_sg, port->elem.out_addr,
-                                 port->elem.out_num, 1);
+                virtio_get_virt_queue_element(f, &port->elem);
 
                 /*
                  *  Port was throttled on source machine.  Let's
diff --git a/hw/virtio.c b/hw/virtio.c
index f40a8c5..8eb8f69 100644
--- a/hw/virtio.c
+++ b/hw/virtio.c
@@ -875,6 +875,19 @@  int virtio_load(VirtIODevice *vdev, QEMUFile *f)
     return 0;
 }
 
+void virtio_put_virt_queue_element(QEMUFile *f, const VirtQueueElement *elem)
+{
+    qemu_put_buffer(f, (unsigned char*)elem, sizeof(*elem));
+}
+
+void virtio_get_virt_queue_element(QEMUFile *f, VirtQueueElement *elem)
+{
+    qemu_get_buffer(f, (unsigned char *)elem, sizeof(*elem));
+
+    virtqueue_map_sg(elem->in_sg, elem->in_addr, elem->in_num, 1);
+    virtqueue_map_sg(elem->out_sg, elem->out_addr, elem->out_num, 0);
+}
+
 void virtio_cleanup(VirtIODevice *vdev)
 {
     qemu_del_vm_change_state_handler(vdev->vmstate);
diff --git a/hw/virtio.h b/hw/virtio.h
index 7c17f7b..4af8239 100644
--- a/hw/virtio.h
+++ b/hw/virtio.h
@@ -159,6 +159,10 @@  void virtio_save(VirtIODevice *vdev, QEMUFile *f);
 
 int virtio_load(VirtIODevice *vdev, QEMUFile *f);
 
+void virtio_put_virt_queue_element(QEMUFile *f, const VirtQueueElement *elem);
+
+void virtio_get_virt_queue_element(QEMUFile *f, VirtQueueElement *elem);
+
 void virtio_cleanup(VirtIODevice *vdev);
 
 void virtio_notify_config(VirtIODevice *vdev);