From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:50276) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7Vuc-0007Hh-UX for qemu-devel@nongnu.org; Tue, 13 Mar 2012 13:53:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S7VuH-0004T9-LG for qemu-devel@nongnu.org; Tue, 13 Mar 2012 13:53:30 -0400 Received: from plane.gmane.org ([80.91.229.3]:55828) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S7VuH-0004Sx-9g for qemu-devel@nongnu.org; Tue, 13 Mar 2012 13:53:09 -0400 Received: from list by plane.gmane.org with local (Exim 4.69) (envelope-from ) id 1S7VuC-0005AA-IC for qemu-devel@nongnu.org; Tue, 13 Mar 2012 18:53:04 +0100 Received: from 93-34-182-16.ip50.fastwebnet.it ([93.34.182.16]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 13 Mar 2012 18:53:04 +0100 Received: from pbonzini by 93-34-182-16.ip50.fastwebnet.it with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Tue, 13 Mar 2012 18:53:04 +0100 From: Paolo Bonzini Date: Tue, 13 Mar 2012 18:52:50 +0100 Message-ID: References: <1331579663-29950-1-git-send-email-mjt@msgid.tls.msk.ru> <1331579663-29950-9-git-send-email-mjt@msgid.tls.msk.ru> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit In-Reply-To: <1331579663-29950-9-git-send-email-mjt@msgid.tls.msk.ru> Subject: Re: [Qemu-devel] [PATCHv3 8/9] cleanup qemu_co_sendv(), qemu_co_recvv() and friends List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org 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 > --- > 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