qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v3 09/12] iov: add iov_get_ptr() to reference ve


From: Stefan Hajnoczi
Subject: Re: [Qemu-devel] [PATCH v3 09/12] iov: add iov_get_ptr() to reference vector data
Date: Thu, 22 Nov 2012 12:58:23 +0100
User-agent: Mutt/1.5.21 (2010-09-15)

On Thu, Nov 22, 2012 at 10:34:13AM +0100, Paolo Bonzini wrote:
> Il 21/11/2012 19:32, Stefan Hajnoczi ha scritto:
> > The iov_get_ptr() data returns a pointer to contiguous data within a
> > vector.  This allows the caller to manipulate data inside the vector
> > without copying in/out using iov_from_buf()/iov_to_buf() when we know
> > that data is contiguous within an iovec element.
> 
> This works for you because you have a single byte to write.  It would
> not work for the SG_IO inhdr, which would need iov_to_buf().

Guilty as charged, your honor. :)

Let me give a few more details about the motivation for this function:

In virtio-blk-data-plane we have an iovec[] array.  In the read/write
code path we discard the inhdr/outhdr so just the data buffers are left
in the iovec[] array.  Then we can pass the iovec[] array straight to
the Linux AIO functions.

Because we're using the iovec[] array for data buffers and we're not
allowed to make assumptions about iovec layout, we cannot use
iov_to_buf()/iov_from_buf() at the end to fill in the status field - the
inhdr has already been discarded from the iovec[] array.

Since I knew the inhdr is only 1 byte I decided against doing something
like dynamically allocating/freeing a QEMUIOVector which could handle
spanning iovecs.

That said, I think this function is okay as-is because it works fine for
non-virtio cases where the caller *knows* the iovec[] layout.  As a
utility function it stands on its own.

> What about the following alternative API:
> 
> void *iov_get_ptr(struct iovec *iov, unsigned int iov_cnt,
>                   ssize_t offset, size_t *bytes);
> 
> which would place the number of valid bytes (i.e. the length of the
> remainder of the iovec entry) in *bytes?
> 
> Also, I think that offset == iov_size(iov, iov_cnt) should be
> acceptable, and it would be the only case in which *bytes == 0.

Hmm...this may be more useful than the version I proposed since the
caller can also use it to find out how many bytes are contiguous.

Michael: Any concerns if I update the code to reflect Paolo's
suggestion?

Stefan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]