From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37712) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsOhJ-0008Q9-2t for qemu-devel@nongnu.org; Wed, 04 Jun 2014 23:50:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WsOhC-0004er-OY for qemu-devel@nongnu.org; Wed, 04 Jun 2014 23:50:36 -0400 Received: from mx1.redhat.com ([209.132.183.28]:8916) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WsOhC-0004ea-9O for qemu-devel@nongnu.org; Wed, 04 Jun 2014 23:50:30 -0400 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s553oSKO020329 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 4 Jun 2014 23:50:29 -0400 Date: Thu, 5 Jun 2014 11:50:42 +0800 From: Fam Zheng Message-ID: <20140605035042.GC10963@T430.nay.redhat.com> References: <1401933526-22436-1-git-send-email-famz@redhat.com> <1401933526-22436-9-git-send-email-famz@redhat.com> <538FDC26.4030402@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <538FDC26.4030402@redhat.com> Subject: Re: [Qemu-devel] [PATCH v2 8/8] virtio-blk: Fill in VirtIOBlockReq.out in dataplane code List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: Kevin Wolf , qemu-devel@nongnu.org, Stefan Hajnoczi On Thu, 06/05 04:55, Paolo Bonzini wrote: > Il 05/06/2014 03:58, Fam Zheng ha scritto: > >Signed-off-by: Fam Zheng > >--- > > hw/block/dataplane/virtio-blk.c | 14 +++++++++----- > > 1 file changed, 9 insertions(+), 5 deletions(-) > > > >diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c > >index 96068cd..323793a 100644 > >--- a/hw/block/dataplane/virtio-blk.c > >+++ b/hw/block/dataplane/virtio-blk.c > >@@ -113,7 +113,8 @@ static void do_get_id_cmd(VirtIOBlockDataPlane *s, > > static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, > > struct iovec *iov, unsigned iov_cnt, > > int64_t sector_num, VirtQueueElement *elem, > >- struct virtio_blk_inhdr *inhdr) > >+ struct virtio_blk_inhdr *inhdr, > >+ struct virtio_blk_outhdr *outhdr) > > { > > VirtIOBlock *dev = VIRTIO_BLK(s->vdev); > > VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); > >@@ -124,6 +125,7 @@ static void do_rdwr_cmd(VirtIOBlockDataPlane *s, bool read, > > req->elem = elem; > > req->dev = dev; > > req->in = inhdr; > >+ req->out = *outhdr; > > qemu_iovec_init_external(&req->qiov, iov, iov_cnt); > > > > qiov = &req->qiov; > >@@ -155,13 +157,15 @@ static void complete_flush(void *opaque, int ret) > > } > > > > static void do_flush_cmd(VirtIOBlockDataPlane *s, VirtQueueElement *elem, > >- struct virtio_blk_inhdr *inhdr) > >+ struct virtio_blk_inhdr *inhdr, > >+ struct virtio_blk_outhdr *outhdr) > > { > > VirtIOBlock *dev = VIRTIO_BLK(s->vdev); > > VirtIOBlockReq *req = g_slice_new0(VirtIOBlockReq); > > req->dev = dev; > > req->elem = elem; > > req->in = inhdr; > >+ req->out = *outhdr; > > > > bdrv_aio_flush(s->blk->conf.bs, complete_flush, req); > > } > >@@ -202,13 +206,13 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) > > case VIRTIO_BLK_T_IN: > > do_rdwr_cmd(s, true, in_iov, in_num, > > outhdr.sector * 512 / BDRV_SECTOR_SIZE, > >- elem, inhdr); > >+ elem, inhdr, &outhdr); > > return 0; > > > > case VIRTIO_BLK_T_OUT: > > do_rdwr_cmd(s, false, iov, out_num, > > outhdr.sector * 512 / BDRV_SECTOR_SIZE, > >- elem, inhdr); > >+ elem, inhdr, &outhdr); > > return 0; > > > > case VIRTIO_BLK_T_SCSI_CMD: > >@@ -216,7 +220,7 @@ static int process_request(VirtIOBlockDataPlane *s, VirtQueueElement *elem) > > return 0; > > > > case VIRTIO_BLK_T_FLUSH: > >- do_flush_cmd(s, elem, inhdr); > >+ do_flush_cmd(s, elem, inhdr, &outhdr); > > return 0; > > > > case VIRTIO_BLK_T_GET_ID: > > > > Can you try moving the req allocation and assignments inside process_request > instead? Then you can fill in req->out directly without the struct > assignment. > The owners of req are do_rdwr_cmd and do_flush_cmd, but do_scsi_cmd and do_get_id_cmd don't need to allocate. Moving the allocation means transfering the ownership from process_request to respective cmd functions, and it's not used in two out of four. Not sure much better than this way, but that's doable. Fam