On Wed, 5 Feb 2020 14:49:46 +0000 Stefan Hajnoczi <stefa...@gmail.com> wrote:
> On Tue, Feb 04, 2020 at 05:02:39PM +0100, Cornelia Huck wrote:
> > On Tue, 4 Feb 2020 15:16:18 +0000
> > Stefan Hajnoczi <stefa...@redhat.com> wrote:
> > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > index 2c5410e981..5d7f619a1e 100644
> > > --- a/hw/virtio/virtio.c
> > > +++ b/hw/virtio/virtio.c
> > > @@ -2163,6 +2163,11 @@ void virtio_queue_set_rings(VirtIODevice *vdev,
> > > int n, hwaddr desc,
> > > vdev->vq[n].vring.avail = avail;
> > > vdev->vq[n].vring.used = used;
> > > virtio_init_region_cache(vdev, n);
> > > + if (vdev->broken) {
> > > + vdev->vq[n].vring.desc = 0;
> > > + vdev->vq[n].vring.avail = 0;
> > > + vdev->vq[n].vring.used = 0;
> > > + }
> > > }
> > >
> > > void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
> >
> > This looks correct; but shouldn't virtio_queue_set_addr() also set
> > .desc to 0 on failure?
>
> Now that you mention it, there are a number of other
> virtio_init_region_cache() callers that could be affected.
>
> I added the error handling code to virtio_queue_set_rings() because
> that's symmetric - this function sets .desc and so it should be the one
> to clear it on error. But now I think virtio_init_region_cache() should
> take on that responsibility so callers don't need to duplicate this
> error handling code.
Is it clear in every case what the correct error handling procedure
would be? It would feel a bit surprising if the addresses were cleared
for callers that don't directly change them.
pgptcl9brVNDm.pgp
Description: OpenPGP digital signature
