Re: [Qemu-devel] [PATCH] virtio: verify that all outstanding buffers are flushed (was Re: vmstate conversion for virtio?)

[Rusty Russell <rusty@rustcorp.com.au>]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Thu, Dec 06, 2012 at 04:33:06PM +1030, Rusty Russell wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> > Add sanity check to address the following concern:
>> >
>> > On Wed, Dec 05, 2012 at 09:47:22AM +1030, Rusty Russell wrote:
>> >> All we need is the index of the request; the rest can be re-read from
>> >> the ring.
>> 
>> The terminology I used here was loose, indeed.
>> 
>> We need the head of the chained descriptor, which we already read from
>> the ring when we gathered the request.
>
> So ack that patch?

No, because I don't understand it.  Is it true for the case of
virtio_blk, which has outstanding requests?

>> Currently we dump a massive structure; it's inelegant at the very least.
>> 
>> Cheers,
>> Rusty.
>
> Hmm not sure what you refer to. I see this per ring:
>
>         qemu_put_be32(f, vdev->vq[i].vring.num);
>         qemu_put_be64(f, vdev->vq[i].pa);
>         qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
>
> Looks like there's no way around savng these fields.

Not what I'm referring to.  See here:

virtio.h defines a 48k structure:

#define VIRTQUEUE_MAX_SIZE 1024

typedef struct VirtQueueElement
{
    unsigned int index;
    unsigned int out_num;
    unsigned int in_num;
    hwaddr in_addr[VIRTQUEUE_MAX_SIZE];
    hwaddr out_addr[VIRTQUEUE_MAX_SIZE];
    struct iovec in_sg[VIRTQUEUE_MAX_SIZE];
    struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
} VirtQueueElement;

virtio-blk.c uses it in its request struct:

typedef struct VirtIOBlockReq
{
    VirtIOBlock *dev;
    VirtQueueElement elem;
    struct virtio_blk_inhdr *in;
    struct virtio_blk_outhdr *out;
    struct virtio_scsi_inhdr *scsi;
    QEMUIOVector qiov;
    struct VirtIOBlockReq *next;
    BlockAcctCookie acct;
} VirtIOBlockReq;

... and saves it in virtio_blk_save:

static void virtio_blk_save(QEMUFile *f, void *opaque)
{
    VirtIOBlock *s = opaque;
    VirtIOBlockReq *req = s->rq;

    virtio_save(&s->vdev, f);
    
    while (req) {
        qemu_put_sbyte(f, 1);
        qemu_put_buffer(f, (unsigned char*)&req->elem, sizeof(req->elem));
        req = req->next;
    }
    qemu_put_sbyte(f, 0);
}

Cheers,
Rusty.