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 <quint...@redhat.com>
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