From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50735) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjVjP-0004if-Nq for qemu-devel@nongnu.org; Fri, 14 Dec 2012 08:55:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TjVjN-0002Wy-SN for qemu-devel@nongnu.org; Fri, 14 Dec 2012 08:55:15 -0500 Received: from e38.co.us.ibm.com ([32.97.110.159]:39397) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TjVjN-0002WT-K3 for qemu-devel@nongnu.org; Fri, 14 Dec 2012 08:55:13 -0500 Received: from /spool/local by e38.co.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 14 Dec 2012 06:55:11 -0700 Received: from d03relay02.boulder.ibm.com (d03relay02.boulder.ibm.com [9.17.195.227]) by d03dlp02.boulder.ibm.com (Postfix) with ESMTP id 19C683E4003E for ; Fri, 14 Dec 2012 06:55:05 -0700 (MST) Received: from d03av03.boulder.ibm.com (d03av03.boulder.ibm.com [9.17.195.169]) by d03relay02.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id qBEDsxQ5215246 for ; Fri, 14 Dec 2012 06:55:01 -0700 Received: from d03av03.boulder.ibm.com (loopback [127.0.0.1]) by d03av03.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id qBEDsuA4009585 for ; Fri, 14 Dec 2012 06:54:56 -0700 From: Anthony Liguori In-Reply-To: <50CB11AC.1070500@redhat.com> References: <1355149790-8125-1-git-send-email-aliguori@us.ibm.com> <1355149790-8125-3-git-send-email-aliguori@us.ibm.com> <50CB11AC.1070500@redhat.com> Date: Fri, 14 Dec 2012 07:54:45 -0600 Message-ID: <87y5h0pwm2.fsf@codemonkey.ws> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Subject: Re: [Qemu-devel] [PATCH 2/4] virtio: add wrapper for saving/restoring virtqueue elements List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Rusty Russell , qemu-devel@nongnu.org, David Gibson , "Michael S. Tsirkin" Paolo Bonzini 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 >> --- >> 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); >>