From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42083) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S8Zvh-0005Yl-GT for qemu-devel@nongnu.org; Fri, 16 Mar 2012 12:23:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S8Zvb-0006zl-Rm for qemu-devel@nongnu.org; Fri, 16 Mar 2012 12:23:01 -0400 Received: from mail-yx0-f173.google.com ([209.85.213.173]:42813) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S8Zvb-0006ze-MQ for qemu-devel@nongnu.org; Fri, 16 Mar 2012 12:22:55 -0400 Received: by yenr5 with SMTP id r5so5020043yen.4 for ; Fri, 16 Mar 2012 09:22:54 -0700 (PDT) Message-ID: <4F6368D9.3070507@codemonkey.ws> Date: Fri, 16 Mar 2012 11:22:49 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1331845217-21705-1-git-send-email-mjt@msgid.tls.msk.ru> <1331845217-21705-11-git-send-email-mjt@msgid.tls.msk.ru> In-Reply-To: <1331845217-21705-11-git-send-email-mjt@msgid.tls.msk.ru> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv4 10/11] cleanup qemu_co_sendv(), qemu_co_recvv() and friends List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev Cc: Paolo Bonzini , qemu-devel@nongnu.org, MORITA Kazutaka On 03/15/2012 04:00 PM, Michael Tokarev wrote: > 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 iov_send_recv() 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 and extra > `iov_cnt' argument. > > Cc: MORITA Kazutaka > Cc: Paolo Bonzini > Signed-off-by: Michael Tokarev Same comments here re: macros. Regards, Anthony Liguori > --- > block/nbd.c | 16 +++++----- > block/sheepdog.c | 6 ++-- > qemu-common.h | 37 ++++++++++------------ > qemu-coroutine-io.c | 82 +++++++++++++++----------------------------------- > 4 files changed, 53 insertions(+), 88 deletions(-) > > diff --git a/block/nbd.c b/block/nbd.c > index 161b299..c451a25 100644 > --- a/block/nbd.c > +++ b/block/nbd.c > @@ -184,7 +184,7 @@ static void nbd_restart_write(void *opaque) > } > > static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request, > - struct iovec *iov, int offset) > + QEMUIOVector *qiov, int offset) > { > int rc, ret; > > @@ -193,8 +193,8 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request, > qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write, > 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); > + if (rc != -1&& qiov) { > + ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov, offset, request->len); > if (ret != request->len) { > errno = -EIO; > rc = -1; > @@ -209,7 +209,7 @@ static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request, > > static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request, > struct nbd_reply *reply, > - struct iovec *iov, int offset) > + QEMUIOVector *qiov, int offset) > { > int ret; > > @@ -220,8 +220,8 @@ static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request, > if (reply->handle != request->handle) { > reply->error = EIO; > } else { > - if (iov&& reply->error == 0) { > - ret = qemu_co_recvv(s->sock, iov, request->len, offset); > + if (qiov&& reply->error == 0) { > + ret = qemu_co_recvv(s->sock, qiov->iov, qiov->niov, offset, request->len); > if (ret != request->len) { > reply->error = EIO; > } > @@ -336,7 +336,7 @@ static int nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num, > if (nbd_co_send_request(s,&request, NULL, 0) == -1) { > reply.error = errno; > } else { > - nbd_co_receive_reply(s,&request,&reply, qiov->iov, offset); > + nbd_co_receive_reply(s,&request,&reply, qiov, offset); > } > nbd_coroutine_end(s,&request); > return -reply.error; > @@ -360,7 +360,7 @@ static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num, > request.len = nb_sectors * 512; > > nbd_coroutine_start(s,&request); > - if (nbd_co_send_request(s,&request, qiov->iov, offset) == -1) { > + if (nbd_co_send_request(s,&request, qiov, offset) == -1) { > reply.error = errno; > } else { > nbd_co_receive_reply(s,&request,&reply, NULL, 0); > diff --git a/block/sheepdog.c b/block/sheepdog.c > index 00276f6f..e688e49 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, acb->qiov->niov, > + 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, niov, 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 d4a0243..d9858d1 100644 > --- a/qemu-common.h > +++ b/qemu-common.h > @@ -297,32 +297,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, unsigned iov_cnt, > + size_t offset, size_t bytes, bool do_send); > +#define qemu_co_recvv(sockfd, iov, iov_cnt, offset, bytes) \ > + qemu_co_sendv_recvv(sockfd, iov, iov_cnt, offset, bytes, false) > +#define qemu_co_sendv(sockfd, iov, iov_cnt, offset, bytes) \ > + qemu_co_sendv_recvv(sockfd, iov, iov_cnt, 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_send); > +#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 0461a9a..6693c78 100644 > --- a/qemu-coroutine-io.c > +++ b/qemu-coroutine-io.c > @@ -27,71 +27,39 @@ > #include "qemu-coroutine.h" > #include "iov.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, unsigned iov_cnt, > + size_t offset, size_t bytes, bool do_send) > { > - int total = 0; > - int ret; > - while (len) { > - ret = iov_recv(sockfd, iov, iov_offset + total, len); > - if (ret< 0) { > + size_t done = 0; > + ssize_t ret; > + while (done< bytes) { > + ret = iov_send_recv(sockfd, iov, > + offset + done, bytes - done, do_send); > + 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 = iov_send(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_send) { > + /* 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_send) > { > - 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, 1, 0, bytes, do_send); > }