From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NFtCQ-000678-Nz for qemu-devel@nongnu.org; Wed, 02 Dec 2009 12:41:10 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NFtCM-00063O-Ng for qemu-devel@nongnu.org; Wed, 02 Dec 2009 12:41:10 -0500 Received: from [199.232.76.173] (port=36230 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NFtCM-000632-DF for qemu-devel@nongnu.org; Wed, 02 Dec 2009 12:41:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:24057) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NFtCL-0000Gn-U9 for qemu-devel@nongnu.org; Wed, 02 Dec 2009 12:41:06 -0500 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nB2Hf48j027722 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 2 Dec 2009 12:41:04 -0500 Date: Wed, 2 Dec 2009 19:38:23 +0200 From: "Michael S. Tsirkin" Message-ID: <20091202173823.GA3638@redhat.com> References: <1971390dc6d0ae9014e796f8ee444dee4a90815a.1259754427.git.quintela@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1971390dc6d0ae9014e796f8ee444dee4a90815a.1259754427.git.quintela@redhat.com> Subject: [Qemu-devel] Re: [PATCH 38/41] virtio-blk: use QLIST for the list of requests List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Juan Quintela Cc: qemu-devel@nongnu.org On Wed, Dec 02, 2009 at 01:04:36PM +0100, Juan Quintela wrote: > viltio_blak_dma_restart_bh() was unsafe, it used req->next after having > (possible) put req in another list > > Signed-off-by: Juan Quintela Sounds good, but why is this part of vmstate patchset? > --- > hw/virtio-blk.c | 29 ++++++++++++++--------------- > 1 files changed, 14 insertions(+), 15 deletions(-) > > diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c > index b716a36..838ec32 100644 > --- a/hw/virtio-blk.c > +++ b/hw/virtio-blk.c > @@ -26,7 +26,7 @@ typedef struct VirtIOBlock > VirtIODevice vdev; > BlockDriverState *bs; > VirtQueue *vq; > - VirtIOBlockReq *rq; > + QLIST_HEAD (,VirtIOBlockReq) rq; > char serial_str[BLOCK_SERIAL_STRLEN + 1]; > QEMUBH *bh; > size_t config_size; > @@ -81,7 +81,7 @@ struct VirtIOBlockReq > struct virtio_blk_outhdr *out; > struct virtio_scsi_inhdr *scsi; > QEMUIOVector qiov; > - struct VirtIOBlockReq *next; > + QLIST_ENTRY(VirtIOBlockReq) next; > }; > > static void virtio_blk_req_complete(VirtIOBlockReq *req, int status) > @@ -105,8 +105,7 @@ static int virtio_blk_handle_write_error(VirtIOBlockReq *req, int error) > > if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC) > || action == BLOCK_ERR_STOP_ANY) { > - req->next = s->rq; > - s->rq = req; > + QLIST_INSERT_HEAD(&s->rq, req, next); > vm_stop(0); > } else { > virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); > @@ -366,17 +365,19 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq) > static void virtio_blk_dma_restart_bh(void *opaque) > { > VirtIOBlock *s = opaque; > - VirtIOBlockReq *req = s->rq; > + QLIST_HEAD (, VirtIOBlockReq) rq_copy; > + VirtIOBlockReq *req, *next_req; > > qemu_bh_delete(s->bh); > s->bh = NULL; > > - s->rq = NULL; > + QLIST_COPY_HEAD(&rq_copy, &s->rq); > + QLIST_INIT(&s->rq); > > - while (req) { > + QLIST_FOREACH_SAFE(req, &rq_copy, next, next_req) { > + QLIST_REMOVE(req, next); > bdrv_aio_writev(req->dev->bs, req->out->sector, &req->qiov, > req->qiov.size / 512, virtio_blk_rw_complete, req); > - req = req->next; > } > } > > @@ -451,14 +452,13 @@ static uint32_t virtio_blk_get_features(VirtIODevice *vdev) > static void virtio_blk_save(QEMUFile *f, void *opaque) > { > VirtIOBlock *s = opaque; > - VirtIOBlockReq *req = s->rq; > + VirtIOBlockReq *req;; > > virtio_save(&s->vdev, f); > - > - while (req) { > + > + QLIST_FOREACH(req, &s->rq, next) { > qemu_put_sbyte(f, 1); > qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem)); > - req = req->next; > } > qemu_put_sbyte(f, 0); > } > @@ -474,8 +474,7 @@ 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)); > - req->next = s->rq; > - s->rq = req->next; > + QLIST_INSERT_HEAD(&s->rq, req, next); > } > > return 0; > @@ -499,7 +498,7 @@ VirtIODevice *virtio_blk_init(DeviceState *dev, DriveInfo *dinfo) > s->vdev.get_features = virtio_blk_get_features; > s->vdev.reset = virtio_blk_reset; > s->bs = dinfo->bdrv; > - s->rq = NULL; > + QLIST_INIT(&s->rq); > if (strlen(ps)) > strncpy(s->serial_str, ps, sizeof(s->serial_str)); > else > -- > 1.6.5.2