At Fri, 16 Sep 2011 16:25:40 +0200, Paolo Bonzini wrote: > > Outside coroutines, avoid busy waiting on EAGAIN by temporarily > making the socket blocking. > > The API of qemu_recvv/qemu_sendv is slightly different from > do_readv/do_writev because they do not handle coroutines. It > returns the number of bytes written before encountering an > EAGAIN. The specificity of yielding on EAGAIN is entirely in > qemu-coroutine.c. > > Reviewed-by: MORITA Kazutaka <morita.kazut...@lab.ntt.co.jp> > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > block/sheepdog.c | 225 > ++++++------------------------------------------------ > cutils.c | 177 ++++++++++++++++++++++++++++++++++++++++++ > qemu-common.h | 30 +++++++ > 3 files changed, 230 insertions(+), 202 deletions(-)
It seems this patch causes a compile error of qemu-ga. Other things I noticed: > static int send_req(int sockfd, SheepdogReq *hdr, void *data, > unsigned int *wlen) > { > @@ -691,10 +509,9 @@ static int send_req(int sockfd, SheepdogReq *hdr, void > *data, > iov[1].iov_len = *wlen; > } > > - ret = do_writev(sockfd, iov, sizeof(*hdr) + *wlen, 0); > - if (ret) { > + ret = qemu_sendv(sockfd, iov, sizeof(*hdr) + *wlen, 0); This is wrong because qemu_sendv() may return a smaller value than (sizeof(*hdr) + *wlen). We need to do things like qemu_write_full() here. > + if (ret < 0) { > error_report("failed to send a req, %s", strerror(errno)); > - ret = -1; > } > > return ret; > @@ -704,17 +521,19 @@ static int do_req(int sockfd, SheepdogReq *hdr, void > *data, > unsigned int *wlen, unsigned int *rlen) > { > int ret; > + struct iovec iov; > > + socket_set_block(sockfd); > ret = send_req(sockfd, hdr, data, wlen); > - if (ret) { > - ret = -1; > + if (ret < 0) { > goto out; > } > > - ret = do_read(sockfd, hdr, sizeof(*hdr)); > - if (ret) { > + iov.iov_base = hdr; > + iov.iov_len = sizeof(*hdr); > + ret = qemu_recvv(sockfd, &iov, sizeof(*hdr), 0); qemu_recvv() may also return a smaller value than sizeof(*hdr) here. > + if (ret < 0) { > error_report("failed to get a rsp, %s", strerror(errno)); > - ret = -1; > goto out; > } > > @@ -723,15 +542,17 @@ static int do_req(int sockfd, SheepdogReq *hdr, void > *data, > } > > if (*rlen) { > - ret = do_read(sockfd, data, *rlen); > - if (ret) { > + iov.iov_base = data; > + iov.iov_len = *rlen; > + ret = qemu_recvv(sockfd, &iov, *rlen, 0); Same here. > + if (ret < 0) { > error_report("failed to get the data, %s", strerror(errno)); > - ret = -1; > goto out; > } > } > ret = 0; > out: > + socket_set_nonblock(sockfd); > return ret; > } > [snip] > + > +/* > + * Send/recv data with iovec buffers > + * > + * This function send/recv data from/to the iovec buffer directly. > + * The first `offset' bytes in the iovec buffer are skipped and next > + * `len' bytes are used. > + * > + * For example, > + * > + * do_sendv_recvv(sockfd, iov, len, offset, 1); > + * > + * is equal to > + * > + * char *buf = malloc(size); > + * iov_to_buf(iov, iovcnt, buf, offset, size); > + * send(sockfd, buf, size, 0); > + * free(buf); > + */ > +static int do_sendv_recvv(int sockfd, struct iovec *iov, int len, int offset, > + int do_sendv) > +{ > + int ret, diff, iovlen; > + struct iovec *last_iov; > + > + /* last_iov is inclusive, so count from one. */ > + iovlen = 1; > + last_iov = iov; > + len += offset; > + > + while (last_iov->iov_len < len) { > + len -= last_iov->iov_len; > + > + last_iov++; > + iovlen++; > + } > + > + diff = last_iov->iov_len - len; > + last_iov->iov_len -= diff; > + > + while (iov->iov_len <= offset) { > + offset -= iov->iov_len; > + > + iov++; > + iovlen--; > + } > + > + iov->iov_base = (char *) iov->iov_base + offset; > + iov->iov_len -= offset; > + > + { > +#ifdef CONFIG_IOVEC > + struct msghdr msg; > + memset(&msg, 0, sizeof(msg)); > + msg.msg_iov = iov; > + msg.msg_iovlen = iovlen; > + > + do { > + if (do_sendv) { > + ret = sendmsg(sockfd, &msg, 0); > + } else { > + ret = recvmsg(sockfd, &msg, 0); > + } > + } while (ret == -1 && errno == EINTR); > +#else > + struct iovec *p = iov; > + ret = 0; > + while (iovlen > 0) { > + int rc; > + if (do_sendv) { > + rc = send(sockfd, p->iov_base, p->iov_len, 0); > + } else { > + rc = qemu_recv(sockfd, p->iov_base, p->iov_len, 0); > + } > + if (rc == -1) { > + if (errno == EINTR) { > + continue; > + } > + if (ret == 0) { > + ret = -1; > + } > + break; > + } > + iovlen--, p++; > + ret += rc; > + } This code can be called inside coroutines with a non-blocking fd, so should we avoid busy waiting? Thanks, Kazutaka