Re: [Qemu-devel] [PATCH v3 1/2] virtio: check VirtQueue Vring object is set

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1257 bytes --]

On Tue, Nov 28, 2017 at 10:11:54AM +0100, Cornelia Huck wrote:
> On Mon, 27 Nov 2017 23:25:28 +0530 (IST)
> P J P <ppandit@redhat.com> wrote:
> > +-- On Mon, 27 Nov 2017, Stefan Hajnoczi wrote --+
> > | > +    if (!vdev->vq[n].vring.num || !desc || !vdev->vq[n].vring.align) {
> > | ...
> > |   vdev->vq[n].vring.desc = desc;
> > |
> > | Why !desc?
> > 
> > virtio_queue_set_rings
> >  virtio_init_region_cache
> >    VirtQueue *vq = &vdev->vq[n];
> >    ...
> >    addr = vq->vring.desc;
> >    if (!addr) {
> >        return;
> >    }
> > 
> > These checks seem to be repeating all over. As mentioned earlier, could these 
> > be collated in one place, maybe virtio_queue_get_num()?
> > 
> >   int virtio_queue_get_num(VirtIODevice *vdev, int n)
> >   {
> >       VirtQueue *vq = &vdev->vq[n];
> > 
> >       if (!vq->.vring.num
> >            || !vq->vring.desc
> >            || !vq->vring.align) {
> >           return 0;  /* vq not set */
> >       }
> > 
> >       return vdev->vq[n].vring.num;
> >   }
> 
> This is conflating different things:
> - vq does not exist (num == 0)
> - vq is not setup by the guest (desc == 0)
> - vq has no valid alignment (which is only relevant for legacy)

I agree.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]