From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=53008 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PrUzg-0001bo-U9
	for qemu-devel@nongnu.org; Mon, 21 Feb 2011 07:36:01 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1PrUzf-0004Dq-Me
	for qemu-devel@nongnu.org; Mon, 21 Feb 2011 07:36:00 -0500
Received: from mx1.redhat.com ([209.132.183.28]:14114)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kwolf@redhat.com>) id 1PrUzf-0004Dj-Ep
	for qemu-devel@nongnu.org; Mon, 21 Feb 2011 07:35:59 -0500
Message-ID: <4D625C94.404@redhat.com>
Date: Mon, 21 Feb 2011 13:37:40 +0100
From: Kevin Wolf <kwolf@redhat.com>
MIME-Version: 1.0
References: <1298033729-17945-1-git-send-email-nick@bytemark.co.uk>
	<1298033729-17945-2-git-send-email-nick@bytemark.co.uk>
In-Reply-To: <1298033729-17945-2-git-send-email-nick@bytemark.co.uk>
Content-Type: text/plain; charset=ISO-8859-15
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH 2/3] NBD library: add aio-compatible
	read/write function
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/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: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Nick Thomas <nick@bytemark.co.uk>
Cc: stefanha@gmail.com, qemu-devel@nongnu.org

Am 18.02.2011 13:55, schrieb Nick Thomas:
> Signed-off-by: Nicholas Thomas <nick@bytemark.co.uk>
> ---
>  nbd.c |   51 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  nbd.h |    2 ++
>  2 files changed, 53 insertions(+), 0 deletions(-)
> 
> diff --git a/nbd.c b/nbd.c
> index abe0ecb..83d3342 100644
> --- a/nbd.c
> +++ b/nbd.c
> @@ -107,6 +107,57 @@ size_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read)
>      return offset;
>  }
>  
> +int nbd_wr_aio(int sockfd, struct iovec *iov, size_t len,  off_t offset,
> +               bool do_read)
> +{
> +    struct msghdr msg;
> +    int ret, diff;
> +
> +    memset(&msg, 0, sizeof(msg));
> +    msg.msg_iov = iov;
> +    msg.msg_iovlen = 1;
> +
> +    len += offset;
> +
> +    while (iov->iov_len < len) {
> +        len -= iov->iov_len;
> +
> +        iov++;
> +        msg.msg_iovlen++;
> +    }

Is there a reason why you don't use a QEMUIOVector here? It already has
alls the information like the number of entries. Throwing this
information away only to recalculate it here seems a bit pointless.

> +
> +    diff = iov->iov_len - len;
> +    iov->iov_len -= diff;
> +
> +    while (msg.msg_iov->iov_len <= offset) {
> +        offset -= msg.msg_iov->iov_len;
> +
> +        msg.msg_iov++;
> +        msg.msg_iovlen--;
> +    }
> +
> +    msg.msg_iov->iov_base = (char *) msg.msg_iov->iov_base + offset;
> +    msg.msg_iov->iov_len -= offset;

You could use qemu_iovec_copy to create a qiov that covers exactly the
part that you want to use.

> +
> +retry:
> +    if (do_read) {
> +        ret = recvmsg(sockfd, &msg, 0);
> +    } else {
> +        ret = sendmsg(sockfd, &msg, 0);
> +    }
> +
> +    /* recoverable error */
> +    if (ret == -1 && (errno == EAGAIN || errno == EINTR)) {
> +        goto retry;
> +    }

Make this a do...while loop, no reason for goto here.

> +
> +    msg.msg_iov->iov_base = (char *) msg.msg_iov->iov_base - offset;
> +    msg.msg_iov->iov_len += offset;
> +
> +    iov->iov_len += diff;
> +    return ret;

This wouldn't be necessary with a copied qiov either (but you'd need to
destroy the copy)

Kevin