From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1O9eSy-0007nK-Lb for qemu-devel@nongnu.org; Wed, 05 May 2010 09:16:44 -0400 Received: from [140.186.70.92] (port=39672 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1O9eSw-0007kO-U3 for qemu-devel@nongnu.org; Wed, 05 May 2010 09:16:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69) (envelope-from ) id 1O9eSu-0001Gg-Js for qemu-devel@nongnu.org; Wed, 05 May 2010 09:16:42 -0400 Received: from mail-iw0-f204.google.com ([209.85.223.204]:43989) by eggs.gnu.org with esmtp (Exim 4.69) (envelope-from ) id 1O9eSu-0001GX-Ct for qemu-devel@nongnu.org; Wed, 05 May 2010 09:16:40 -0400 Received: by iwn42 with SMTP id 42so6224438iwn.14 for ; Wed, 05 May 2010 06:16:39 -0700 (PDT) Message-ID: <4BE16FB5.6040708@codemonkey.ws> Date: Wed, 05 May 2010 08:16:37 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1273009170-17530-1-git-send-email-amit.shah@redhat.com> <1273009170-17530-2-git-send-email-amit.shah@redhat.com> <1273009170-17530-3-git-send-email-amit.shah@redhat.com> <1273009170-17530-4-git-send-email-amit.shah@redhat.com> <1273009170-17530-5-git-send-email-amit.shah@redhat.com> In-Reply-To: <1273009170-17530-5-git-send-email-amit.shah@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [PATCH v7 4/6] char: Add qemu_chr_write_nb() for nonblocking writes List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: Paul Brook , Juan Quintela , qemu list , Gerd Hoffmann On 05/04/2010 04:39 PM, Amit Shah wrote: > For char devices whose backing files are open in non-blocking mode, > non-blocking writes can now be made using qemu_chr_write_nb(). > > For non-blocking chardevs, we can return -EAGAIN to callers of > qemu_chr_write_nb(). When the backend is ready to accept more data, > we can let the caller know via a callback. > > -EAGAIN is returned only when the backend's file is non-blocking > and a callback is registered by the caller when invoking > qemu_chr_add_handlers(). > > In case a backend doesn't support non-blocking writes, > qemu_chr_write_nb() invokes qemu_chr_write(). > > Individual callers can update their code to add a callback handler, > call qemu_chr_write_nb() instead of qemu_chr_write() if they wish to > receive -EAGAIN notifications. > > No backend currently supports non-blocking writes. > > Signed-off-by: Amit Shah > --- > net/socket.c | 4 ++-- > qemu-char.c | 31 ++++++++++++++++++++++++------- > qemu-char.h | 8 ++++++++ > qemu_socket.h | 3 ++- > 4 files changed, 36 insertions(+), 10 deletions(-) > > diff --git a/net/socket.c b/net/socket.c > index 1c4e153..8a401e6 100644 > --- a/net/socket.c > +++ b/net/socket.c > @@ -56,8 +56,8 @@ static ssize_t net_socket_receive(VLANClientState *nc, const uint8_t *buf, size_ > uint32_t len; > len = htonl(size); > > - send_all(s->fd, (const uint8_t *)&len, sizeof(len)); > - return send_all(s->fd, buf, size); > + send_all(s->fd, (const uint8_t *)&len, sizeof(len), false); > + return send_all(s->fd, buf, size, false); > } > > static ssize_t net_socket_receive_dgram(VLANClientState *nc, const uint8_t *buf, size_t size) > diff --git a/qemu-char.c b/qemu-char.c > index decf687..5e4dec3 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -145,6 +145,16 @@ int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len) > return s->chr_write(s, buf, len); > } > > +ssize_t qemu_chr_write_nb(CharDriverState *s, const uint8_t *buf, size_t len) > +{ > + if (!s->nonblock) { > + /* Fallback to blocking write if no callback registered */ > + return qemu_chr_write(s, buf, len); > + } > + > + return s->chr_write_nb(s, buf, len); > +} > I really dislike the idea of adding another function for this. Can you explain why you need this functionality for virtio-console and why this functionality isn't needed for everything else? Regards, Anthony Liguori > int qemu_chr_ioctl(CharDriverState *s, int cmd, void *arg) > { > if (!s->chr_ioctl) > @@ -203,11 +213,15 @@ void qemu_chr_add_handlers(CharDriverState *s, > } > s->chr_can_read = handlers->fd_can_read; > s->chr_read = handlers->fd_read; > + s->chr_write_unblocked = handlers->fd_write_unblocked; > s->chr_event = handlers->fd_event; > s->handler_opaque = opaque; > if (s->chr_update_read_handler) > s->chr_update_read_handler(s); > > + /* We'll set this at connect-time */ > + s->nonblock = false; > + > /* We're connecting to an already opened device, so let's make sure we > also get the open event */ > if (s->opened) { > @@ -499,7 +513,7 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState *drv) > > > #ifdef _WIN32 > -int send_all(int fd, const void *buf, int len1) > +int send_all(int fd, const void *buf, int len1, bool nonblock) > { > int ret, len; > > @@ -526,7 +540,7 @@ int send_all(int fd, const void *buf, int len1) > > #else > > -static int unix_write(int fd, const uint8_t *buf, int len1) > +static int unix_write(int fd, const uint8_t *buf, int len1, bool nonblock) > { > int ret, len; > > @@ -540,6 +554,9 @@ static int unix_write(int fd, const uint8_t *buf, int len1) > if (len1 - len) { > return len1 - len; > } > + if (errno == EAGAIN&& nonblock) { > + return -EAGAIN; > + } > if (errno != EAGAIN) { > return -1; > } > @@ -553,9 +570,9 @@ static int unix_write(int fd, const uint8_t *buf, int len1) > return len1 - len; > } > > -int send_all(int fd, const void *buf, int len1) > +int send_all(int fd, const void *buf, int len1, bool nonblock) > { > - return unix_write(fd, buf, len1); > + return unix_write(fd, buf, len1, nonblock); > } > #endif /* !_WIN32 */ > > @@ -572,7 +589,7 @@ static int stdio_nb_clients = 0; > static int fd_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > { > FDCharDriver *s = chr->opaque; > - return send_all(s->fd_out, buf, len); > + return send_all(s->fd_out, buf, len, false); > } > > static int fd_chr_read_poll(void *opaque) > @@ -884,7 +901,7 @@ static int pty_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > pty_chr_update_read_handler(chr); > return 0; > } > - return send_all(s->fd, buf, len); > + return send_all(s->fd, buf, len, false); > } > > static int pty_chr_read_poll(void *opaque) > @@ -1949,7 +1966,7 @@ static int tcp_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > { > TCPCharDriver *s = chr->opaque; > if (s->connected) { > - return send_all(s->fd, buf, len); > + return send_all(s->fd, buf, len, false); > } else { > /* XXX: indicate an error ? */ > return len; > diff --git a/qemu-char.h b/qemu-char.h > index eacb3b9..52f9ef1 100644 > --- a/qemu-char.h > +++ b/qemu-char.h > @@ -1,6 +1,8 @@ > #ifndef QEMU_CHAR_H > #define QEMU_CHAR_H > > +#include > + > #include "qemu-common.h" > #include "qemu-queue.h" > #include "qemu-option.h" > @@ -53,12 +55,15 @@ typedef void IOEventHandler(void *opaque, int event); > struct CharDriverState { > void (*init)(struct CharDriverState *s); > int (*chr_write)(struct CharDriverState *s, const uint8_t *buf, int len); > + ssize_t (*chr_write_nb)(struct CharDriverState *s, const uint8_t *buf, > + size_t len); > void (*chr_update_read_handler)(struct CharDriverState *s); > int (*chr_ioctl)(struct CharDriverState *s, int cmd, void *arg); > int (*get_msgfd)(struct CharDriverState *s); > IOEventHandler *chr_event; > IOCanReadHandler *chr_can_read; > IOReadHandler *chr_read; > + IOHandler *chr_write_unblocked; > void *handler_opaque; > void (*chr_send_event)(struct CharDriverState *chr, int event); > void (*chr_close)(struct CharDriverState *chr); > @@ -68,6 +73,8 @@ struct CharDriverState { > char *label; > char *filename; > int opened; > + bool nonblock; > + bool write_blocked; > QTAILQ_ENTRY(CharDriverState) next; > }; > > @@ -85,6 +92,7 @@ CharDriverState *qemu_chr_open(const char *label, const char *filename, void (*i > void qemu_chr_close(CharDriverState *chr); > void qemu_chr_printf(CharDriverState *s, const char *fmt, ...); > int qemu_chr_write(CharDriverState *s, const uint8_t *buf, int len); > +ssize_t qemu_chr_write_nb(CharDriverState *s, const uint8_t *buf, size_t len); > void qemu_chr_send_event(CharDriverState *s, int event); > void qemu_chr_add_handlers(CharDriverState *s, const QemuChrHandlers *handlers, > void *opaque); > diff --git a/qemu_socket.h b/qemu_socket.h > index 164ae3e..bdf878b 100644 > --- a/qemu_socket.h > +++ b/qemu_socket.h > @@ -23,6 +23,7 @@ int inet_aton(const char *cp, struct in_addr *ia); > #include > #include > #include > +#include > > #define socket_error() errno > #define closesocket(s) close(s) > @@ -35,7 +36,7 @@ int inet_aton(const char *cp, struct in_addr *ia); > int qemu_socket(int domain, int type, int protocol); > int qemu_accept(int s, struct sockaddr *addr, socklen_t *addrlen); > void socket_set_nonblock(int fd); > -int send_all(int fd, const void *buf, int len1); > +int send_all(int fd, const void *buf, int len1, bool nonblock); > > /* New, ipv6-ready socket helper functions, see qemu-sockets.c */ > int inet_listen_opts(QemuOpts *opts, int port_offset); >