From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([208.118.235.92]:46016)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rusty@ozlabs.org>) id 1Tgl39-0003JU-1V
	for qemu-devel@nongnu.org; Thu, 06 Dec 2012 18:40:16 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <rusty@ozlabs.org>) id 1Tgl37-0000z4-W3
	for qemu-devel@nongnu.org; Thu, 06 Dec 2012 18:40:14 -0500
Received: from ozlabs.org ([203.10.76.45]:53583)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <rusty@ozlabs.org>) id 1Tgl37-0000ym-L9
	for qemu-devel@nongnu.org; Thu, 06 Dec 2012 18:40:13 -0500
From: Rusty Russell <rusty@rustcorp.com.au>
In-Reply-To: <20121206080221.GC10837@redhat.com>
References: <87624iikcw.fsf@rustcorp.com.au> <87zk1uca8z.fsf@elfo.mitica>
	<87r4n5h0fx.fsf@rustcorp.com.au>
	<20121205110807.GA10045@redhat.com>
	<87obi7g1k5.fsf@rustcorp.com.au>
	<20121206080221.GC10837@redhat.com>
Date: Fri, 07 Dec 2012 10:09:42 +1030
Message-ID: <87hanyg37l.fsf@rustcorp.com.au>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Subject: Re: [Qemu-devel] [PATCH] virtio: verify that all outstanding
	buffers are flushed (was Re: vmstate conversion for virtio?)
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: QEMU-devel <qemu-devel@nongnu.org>, quintela@redhat.com

"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.