[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCHv3 8/9] cleanup qemu_co_sendv(), qemu_co_recvv()
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCHv3 8/9] cleanup qemu_co_sendv(), qemu_co_recvv() and friends |
Date: |
Tue, 13 Mar 2012 18:52:50 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:10.0.1) Gecko/20120216 Thunderbird/10.0.1 |
Il 12/03/2012 20:14, Michael Tokarev ha scritto:
> The same as for non-coroutine versions in previous
> patches: rename arguments to be more obvious, change
> type of arguments from int to size_t where appropriate,
> and use common code for send and receive paths (with
> one extra argument) since these are exactly the same.
> Use common qemu_sendv_recvv() directly.
>
> qemu_co_sendv(), qemu_co_recvv(), and qemu_co_recv()
> are now trivial #define's merely adding one extra arg.
>
> qemu_co_sendv() and qemu_co_recvv() callers are
> converted to different argument order.
>
> Also add comment near qemu_sendv() and qemu_recvv()
> stating that these are now unused.
>
> Signed-off-by: Michael Tokarev <address@hidden>
> ---
> block/nbd.c | 4 +-
> block/sheepdog.c | 6 ++--
> qemu-common.h | 39 ++++++++++++------------
> qemu-coroutine-io.c | 82 +++++++++++++++-----------------------------------
> 4 files changed, 49 insertions(+), 82 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index 161b299..9919acc 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -194,7 +194,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct
> nbd_request *request,
> nbd_have_request, NULL, s);
> rc = nbd_send_request(s->sock, request);
> if (rc != -1 && iov) {
> - ret = qemu_co_sendv(s->sock, iov, request->len, offset);
> + ret = qemu_co_sendv(s->sock, iov, offset, request->len);
> if (ret != request->len) {
> errno = -EIO;
> rc = -1;
> @@ -221,7 +221,7 @@ static void nbd_co_receive_reply(BDRVNBDState *s, struct
> nbd_request *request,
> reply->error = EIO;
> } else {
> if (iov && reply->error == 0) {
> - ret = qemu_co_recvv(s->sock, iov, request->len, offset);
> + ret = qemu_co_recvv(s->sock, iov, offset, request->len);
> if (ret != request->len) {
> reply->error = EIO;
> }
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 00276f6f..1083f27 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -657,8 +657,8 @@ static void coroutine_fn aio_read_response(void *opaque)
> }
> break;
> case AIOCB_READ_UDATA:
> - ret = qemu_co_recvv(fd, acb->qiov->iov, rsp.data_length,
> - aio_req->iov_offset);
> + ret = qemu_co_recvv(fd, acb->qiov->iov,
> + aio_req->iov_offset, rsp.data_length);
> if (ret < 0) {
> error_report("failed to get the data, %s", strerror(errno));
> goto out;
> @@ -924,7 +924,7 @@ static int coroutine_fn add_aio_request(BDRVSheepdogState
> *s, AIOReq *aio_req,
> }
>
> if (wlen) {
> - ret = qemu_co_sendv(s->fd, iov, wlen, aio_req->iov_offset);
> + ret = qemu_co_sendv(s->fd, iov, aio_req->iov_offset, wlen);
> if (ret < 0) {
> qemu_co_mutex_unlock(&s->lock);
> error_report("failed to send a data, %s", strerror(errno));
> diff --git a/qemu-common.h b/qemu-common.h
> index b01d84c..eae1bde 100644
> --- a/qemu-common.h
> +++ b/qemu-common.h
> @@ -207,6 +207,8 @@ int qemu_pipe(int pipefd[2]);
> */
> ssize_t qemu_sendv_recvv(int sockfd, struct iovec *iov,
> size_t offset, size_t bytes, bool do_sendv);
> +/* these are basically unused now when everything
> + * switched is using coroutine versions */
> #define qemu_recvv(sockfd, iov, offset, bytes) \
> qemu_sendv_recvv(sockfd, iov, offset, bytes, false)
> #define qemu_sendv(sockfd, iov, offset, bytes) \
> @@ -306,32 +308,29 @@ struct qemu_work_item {
> void qemu_init_vcpu(void *env);
> #endif
>
> -/**
> - * Sends an iovec (or optionally a part of it) down a socket, yielding
> - * when the socket is full.
> - */
> -int qemu_co_sendv(int sockfd, struct iovec *iov,
> - int len, int iov_offset);
>
> /**
> - * Receives data into an iovec (or optionally into a part of it) from
> - * a socket, yielding when there is no data in the socket.
> + * Sends a (part of) iovec down a socket, yielding when the socket is full,
> or
> + * Receives data into a (part of) iovec from a socket,
> + * yielding when there is no data in the socket.
> + * The same interface as qemu_sendv_recvv(), with added yielding.
> + * XXX should mark these as coroutine_fn
> */
> -int qemu_co_recvv(int sockfd, struct iovec *iov,
> - int len, int iov_offset);
> -
> +ssize_t qemu_co_sendv_recvv(int sockfd, struct iovec *iov,
> + size_t offset, size_t bytes, bool do_sendv);
> +#define qemu_co_recvv(sockfd, iov, offset, bytes) \
> + qemu_co_sendv_recvv(sockfd, iov, offset, bytes, false)
> +#define qemu_co_sendv(sockfd, iov, offset, bytes) \
> + qemu_co_sendv_recvv(sockfd, iov, offset, bytes, true)
>
> /**
> - * Sends a buffer down a socket, yielding when the socket is full.
> + * The same as above, but with just a single buffer
> */
> -int qemu_co_send(int sockfd, void *buf, int len);
> -
> -/**
> - * Receives data into a buffer from a socket, yielding when there
> - * is no data in the socket.
> - */
> -int qemu_co_recv(int sockfd, void *buf, int len);
> -
> +ssize_t qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool
> do_sendv);
> +#define qemu_co_recv(sockfd, buf, bytes) \
> + qemu_co_send_recv(sockfd, buf, bytes, false)
> +#define qemu_co_send(sockfd, buf, bytes) \
> + qemu_co_send_recv(sockfd, buf, bytes, true)
>
> typedef struct QEMUIOVector {
> struct iovec *iov;
> diff --git a/qemu-coroutine-io.c b/qemu-coroutine-io.c
> index df8ff21..7c978ea 100644
> --- a/qemu-coroutine-io.c
> +++ b/qemu-coroutine-io.c
> @@ -26,71 +26,39 @@
> #include "qemu_socket.h"
> #include "qemu-coroutine.h"
>
> -int coroutine_fn qemu_co_recvv(int sockfd, struct iovec *iov,
> - int len, int iov_offset)
> +ssize_t coroutine_fn
> +qemu_co_sendv_recvv(int sockfd, struct iovec *iov,
> + size_t offset, size_t bytes, bool do_sendv)
> {
> - int total = 0;
> - int ret;
> - while (len) {
> - ret = qemu_recvv(sockfd, iov, iov_offset + total, len);
> - if (ret < 0) {
> + size_t done = 0;
> + ssize_t ret;
> + while (done < bytes) {
> + ret = qemu_sendv_recvv(sockfd, iov,
> + offset + done, bytes - done, do_sendv);
> + if (ret > 0) {
> + done += ret;
> + } else if (ret < 0) {
> if (errno == EAGAIN) {
> qemu_coroutine_yield();
> - continue;
> - }
> - if (total == 0) {
> - total = -1;
> - }
> - break;
> - }
> - if (ret == 0) {
> - break;
> - }
> - total += ret, len -= ret;
> - }
> -
> - return total;
> -}
> -
> -int coroutine_fn qemu_co_sendv(int sockfd, struct iovec *iov,
> - int len, int iov_offset)
> -{
> - int total = 0;
> - int ret;
> - while (len) {
> - ret = qemu_sendv(sockfd, iov, iov_offset + total, len);
> - if (ret < 0) {
> - if (errno == EAGAIN) {
> - qemu_coroutine_yield();
> - continue;
> - }
> - if (total == 0) {
> - total = -1;
> + } else if (done == 0) {
> + return -1;
> + } else {
> + break;
> }
> + } else if (ret == 0 && !do_sendv) {
> + /* write (send) should never return 0.
> + * read (recv) returns 0 for end-of-file (-data).
> + * In both cases there's little point retrying,
> + * but we do for write anyway, just in case */
> break;
> }
> - total += ret, len -= ret;
> }
> -
> - return total;
> + return done;
> }
>
> -int coroutine_fn qemu_co_recv(int sockfd, void *buf, int len)
> +ssize_t coroutine_fn
> +qemu_co_send_recv(int sockfd, void *buf, size_t bytes, bool do_sendv)
> {
> - struct iovec iov;
> -
> - iov.iov_base = buf;
> - iov.iov_len = len;
> -
> - return qemu_co_recvv(sockfd, &iov, len, 0);
> -}
> -
> -int coroutine_fn qemu_co_send(int sockfd, void *buf, int len)
> -{
> - struct iovec iov;
> -
> - iov.iov_base = buf;
> - iov.iov_len = len;
> -
> - return qemu_co_sendv(sockfd, &iov, len, 0);
> + struct iovec iov = { .iov_base = buf, .iov_len = bytes };
> + return qemu_co_sendv_recvv(sockfd, &iov, 0, bytes, do_sendv);
> }
Looks good.
Paolo
- [Qemu-devel] [PATCHv3 2/9] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), (continued)
- [Qemu-devel] [PATCHv3 2/9] consolidate qemu_iovec_memset{, _skip}() into single function and use existing iov_memset(), Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 1/9] refresh iov_* functions, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 4/9] consolidate qemu_iovec_copy() and qemu_iovec_concat() and make them consistent, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 5/9] change qemu_iovec_to_buf() to match other to, from_buf functions, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 8/9] cleanup qemu_co_sendv(), qemu_co_recvv() and friends, Michael Tokarev, 2012/03/12
- Re: [Qemu-devel] [PATCHv3 8/9] cleanup qemu_co_sendv(), qemu_co_recvv() and friends,
Paolo Bonzini <=
- [Qemu-devel] [PATCHv3 7/9] export qemu_sendv_recvv() and use it in qemu_sendv() and qemu_recvv(), Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 3/9] allow qemu_iovec_from_buffer() to specify offset from which to start copying, Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 6/9] change prototypes of qemu_sendv() and qemu_recvv(), Michael Tokarev, 2012/03/12
- [Qemu-devel] [PATCHv3 9/9] rewrite and comment qemu_sendv_recvv(), Michael Tokarev, 2012/03/12
- Re: [Qemu-devel] [PATCHv3 0/9] cleanup/consolidate some iovec functions, Michael Tokarev, 2012/03/13