All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>,
	Anthony Liguori <aliguori@us.ibm.com>,
	dkoch@cloudswitch.com, qemu-devel@nongnu.org,
	Michael Roth <mdroth@linux.vnet.ibm.com>,
	Blue Swirl <blauwirbel@gmail.com>,
	khoa@us.ibm.com, Stefan Hajnoczi <stefanha@redhat.com>,
	Asias He <asias@redhat.com>
Subject: Re: [Qemu-devel] [PATCH v3 09/12] iov: add iov_get_ptr() to reference vector data
Date: Thu, 22 Nov 2012 13:14:12 +0200	[thread overview]
Message-ID: <20121122111412.GA25594@redhat.com> (raw)
In-Reply-To: <50AE0479.50404@redhat.com>

On Thu, Nov 22, 2012 at 11:54:49AM +0100, Paolo Bonzini wrote:
> Il 22/11/2012 11:38, Michael S. Tsirkin ha scritto:
> >> > The code is a little simpler, because we know the footer is 1 byte only.
> >
> > Yes but the APIs don't make sense in the generic case
> > of >1 byte: users will have to code up two paths for when
> > the buffer they want to access gets scattered across.
> 
> That would be premature optimization; with >1 byte you just use
> iov_from/to_buf.

If the API makes it very clear that it only works for 1 byte
I would have no objection.

> BTW, something like this function is also useful for the broken SCSI
> outhdr ("the CDB starts after the common outhdr and is in a single
> iovec").  But the API must be changed slightly, as in my answer to Stefan.

The scsi command is special in that the length is
iov_length. It's all legacy anyway so I think we can
just assume it's the second s/g entry:
iov[1].iov_length.
We should probably have a wrapper that gets it
after checking iov_cnt > 1.

	size_t iov_get_seg_length(...)
	{
		assert(iov_cnt <= idx);
		return iov[idx].iov_length;
	}

Rusty says he'll put the length of the command
in the buffer in the future, so we will have
if (new_cmd_feature)
		iov_to_buf(.... &len)
else
		len = iov_get_seg_length



> > If the point is to avoid scanning iov vector when data is towards the
> > end of the iov, then this does sound reasonable.  In that case IMHO we
> > should just have accessors that work back from end of the iov. E.g.
> > 
> > size_t iov_from_buf_end(const struct iovec *iov, unsigned int iov_cnt,
> > 			size_t offset, const void *buf, size_t bytes)
> 
> That's also a possibility.
> 
> Paolo

  reply	other threads:[~2012-11-22 11:11 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-21 18:32 [Qemu-devel] [PATCH v3 00/12] virtio: virtio-blk data plane Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 01/12] raw-posix: add raw_get_aio_fd() for virtio-blk-data-plane Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 02/12] configure: add CONFIG_VIRTIO_BLK_DATA_PLANE Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 03/12] dataplane: add host memory mapping code Stefan Hajnoczi
2012-11-26 14:46   ` Don Koch
2012-11-26 15:36     ` Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 04/12] dataplane: add virtqueue vring code Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 05/12] dataplane: add event loop Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 06/12] dataplane: add Linux AIO request queue Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 07/12] iov: add iov_discard() to remove data Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 08/12] test-iov: add iov_discard() testcase Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 09/12] iov: add iov_get_ptr() to reference vector data Stefan Hajnoczi
2012-11-22  9:34   ` Paolo Bonzini
2012-11-22  9:45     ` Michael S. Tsirkin
2012-11-22  9:52       ` Paolo Bonzini
2012-11-22 10:38         ` Michael S. Tsirkin
2012-11-22 10:54           ` Paolo Bonzini
2012-11-22 11:14             ` Michael S. Tsirkin [this message]
2012-11-22 11:58     ` Stefan Hajnoczi
2012-11-22 12:15       ` Paolo Bonzini
2012-11-22 12:35       ` Michael S. Tsirkin
2012-11-22 15:18         ` Stefan Hajnoczi
2012-11-21 18:32 ` [Qemu-devel] [PATCH v3 10/12] test-iov: add iov_get_ptr() test case Stefan Hajnoczi
2012-11-21 18:33 ` [Qemu-devel] [PATCH v3 11/12] dataplane: add virtio-blk data plane code Stefan Hajnoczi
2012-11-21 18:33 ` [Qemu-devel] [PATCH v3 12/12] virtio-blk: add x-data-plane=on|off performance feature Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20121122111412.GA25594@redhat.com \
    --to=mst@redhat.com \
    --cc=aliguori@us.ibm.com \
    --cc=asias@redhat.com \
    --cc=blauwirbel@gmail.com \
    --cc=dkoch@cloudswitch.com \
    --cc=khoa@us.ibm.com \
    --cc=kwolf@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.