On Tue, 04/22 18:00, Jason Wang wrote:
> On 04/22/2014 04:55 PM, Fam Zheng wrote:
> > Set vdev's broken flag, instead of exit, if the VQ has invalid data.
> >
> > Check VirtIODevice.broken in VQ output handler, and don't pop any more
> > request if set, until the device is reset.
> >
> > Signed-off-by: Fam Zheng <f...@redhat.com>
> > ---
> > hw/block/virtio-blk.c | 20 +++++++++++++++-----
> > 1 file changed, 15 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
> > index 8a568e5..f69aca6 100644
> > --- a/hw/block/virtio-blk.c
> > +++ b/hw/block/virtio-blk.c
> > @@ -339,20 +339,23 @@ static void virtio_blk_handle_read(VirtIOBlockReq
> > *req)
> > virtio_blk_rw_complete, req);
> > }
> >
> > -static void virtio_blk_handle_request(VirtIOBlockReq *req,
> > - MultiReqBuffer *mrb)
> > +static int virtio_blk_handle_request(VirtIOBlockReq *req,
> > + MultiReqBuffer *mrb)
> > {
> > uint32_t type;
> > + VirtIODevice *vdev = VIRTIO_DEVICE(req->dev);
> >
> > if (req->elem.out_num < 1 || req->elem.in_num < 1) {
> > error_report("virtio-blk missing headers");
> > - exit(1);
> > + virtio_set_broken(vdev);
> > + return -EINVAL;
> > }
> >
> > if (req->elem.out_sg[0].iov_len < sizeof(*req->out) ||
> > req->elem.in_sg[req->elem.in_num - 1].iov_len < sizeof(*req->in)) {
> > error_report("virtio-blk header not in correct element");
> > - exit(1);
> > + virtio_set_broken(vdev);
> > + return -EINVAL;
> > }
> >
> > req->out = (void *)req->elem.out_sg[0].iov_base;
> > @@ -389,6 +392,7 @@ static void virtio_blk_handle_request(VirtIOBlockReq
> > *req,
> > virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP);
> > g_free(req);
> > }
> > + return 0;
> > }
> >
> > static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
> > @@ -399,6 +403,10 @@ static void virtio_blk_handle_output(VirtIODevice
> > *vdev, VirtQueue *vq)
> > .num_writes = 0,
> > };
> >
> > + if (virtio_broken(vdev)) {
> > + return;
> > + }
> > +
> > #ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
> > /* Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
> > * dataplane here instead of waiting for .set_status().
> > @@ -410,7 +418,9 @@ static void virtio_blk_handle_output(VirtIODevice
> > *vdev, VirtQueue *vq)
> > #endif
> >
> > while ((req = virtio_blk_get_request(s))) {
> > - virtio_blk_handle_request(req, &mrb);
> > + if (virtio_blk_handle_request(req, &mrb) < 0) {
> > + return;
> > + }
> > }
> >
> > virtio_submit_multiwrite(s->bs, &mrb);
>
> For virtio-blk and virtio-scsi, can we just check this in
> virtio_queue_notify_vq()?
That way we are more explict and also stricter with the convention of setting
"broken": no queue will be passed to the device code, for any virtio-*.
But yes that should work and may be even easier, because in virtio.c there are
error paths in VQ itself, in which case guest/host communicating with VQ should
be avoid completely.
So it sounds like a good idea.
>
> For virtio-net, just ignore the request form guest may be not enough. We
> may also stop processing incoming packets.
Yes, and similar to virtio-serial.
Thanks,
Fam
