From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=58757 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PGu8Q-0001mG-U7 for qemu-devel@nongnu.org; Fri, 12 Nov 2010 08:57:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PGu8P-0003NT-1o for qemu-devel@nongnu.org; Fri, 12 Nov 2010 08:57:46 -0500 Received: from mx1.redhat.com ([209.132.183.28]:2842) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PGu8O-0003My-QX for qemu-devel@nongnu.org; Fri, 12 Nov 2010 08:57:45 -0500 Date: Fri, 12 Nov 2010 11:57:40 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 1/3] qemu-char: Introduce Memory driver Message-ID: <20101112115740.3f5d33c7@doriath> In-Reply-To: References: <1289503913-27413-1-git-send-email-lcapitulino@redhat.com> <1289503913-27413-2-git-send-email-lcapitulino@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@redhat.com On Fri, 12 Nov 2010 11:21:57 +0100 Markus Armbruster wrote: > Luiz Capitulino writes: > > > This driver handles in-memory chardev operations. That's, all writes > > to this driver are stored in an internal buffer and it doesn't talk > > to the external world in any way. > > > > Right now it's very simple: it supports only writes. But it can be > > easily extended to support more operations. > > > > This is going to be used by the monitor's "HMP passthrough via QMP" > > feature, which needs to run monitor handlers without a backing > > device. > > > > Signed-off-by: Luiz Capitulino > > --- > > qemu-char.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > qemu-char.h | 7 ++++++ > > 2 files changed, 77 insertions(+), 0 deletions(-) > > > > diff --git a/qemu-char.c b/qemu-char.c > > index 88997f9..36d23c6 100644 > > --- a/qemu-char.c > > +++ b/qemu-char.c > > @@ -2275,6 +2275,76 @@ static CharDriverState *qemu_chr_open_socket(QemuOpts *opts) > > return NULL; > > } > > > > +/***********************************************************/ > > +/* Memory chardev */ > > +typedef struct { > > + size_t outbuf_size; > > + size_t outbuf_capacity; > > + uint8_t *outbuf; > > +} MemoryDriver; > > + > > +static int mem_chr_write(CharDriverState *chr, const uint8_t *buf, int len) > > +{ > > + MemoryDriver *d = chr->opaque; > > + > > + /* TODO: the QString implementation has the same code, we should > > + * introduce a generic way to do this in cutils.c */ > > + if (d->outbuf_capacity < d->outbuf_size + len) { > > + /* grow outbuf */ > > + d->outbuf_capacity += len; > > + d->outbuf_capacity *= 2; > > + d->outbuf = qemu_realloc(d->outbuf, d->outbuf_capacity); > > + } > > + > > + memcpy(d->outbuf + d->outbuf_size, buf, len); > > + d->outbuf_size += len; > > + > > + return len; > > +} > > + > > +void qemu_chr_init_mem(CharDriverState *chr) > > +{ > > + MemoryDriver *d; > > + > > + d = qemu_malloc(sizeof(*d)); > > + d->outbuf_size = 0; > > + d->outbuf_capacity = 4096; > > + d->outbuf = qemu_mallocz(d->outbuf_capacity); > > + > > + memset(chr, 0, sizeof(*chr)); > > + chr->opaque = d; > > + chr->chr_write = mem_chr_write; > > +} > > + > > +/* assumes the stored data is a string */ > > This could indicate a problem. See my reply in the thread for v2. Replied, but I can't see the problem. > > +QString *qemu_chr_mem_to_qs(CharDriverState *chr) > > +{ > > + MemoryDriver *d = chr->opaque; > > + > > + if (d->outbuf_size == 0) { > > + return qstring_new(); > > + } > > Why is this necessary? Is qstring_from_substr() broken for empty > substrings? If it is, it ought to be fixed! qstring_from_substr() takes a character range; outbuf_size stores a size, not a string length. So we do: > > + return qstring_from_substr((char *) d->outbuf, 0, d->outbuf_size - 1); If outbuf_size is 0, we'll be passing a negative value down. > > +} > > + > > +/* NOTE: this driver can not be closed with qemu_chr_close()! */ > > +void qemu_chr_close_mem(CharDriverState *chr) > > +{ > > + MemoryDriver *d = chr->opaque; > > + > > + qemu_free(d->outbuf); > > + qemu_free(chr->opaque); > > + chr->opaque = NULL; > > + chr->chr_write = NULL; > > +} > > + > > +size_t qemu_chr_mem_osize(const CharDriverState *chr) > > +{ > > + const MemoryDriver *d = chr->opaque; > > + return d->outbuf_size; > > +} > > + > > QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) > > { > > char host[65], port[33], width[8], height[8]; > > diff --git a/qemu-char.h b/qemu-char.h > > index 18ad12b..e6ee6c4 100644 > > --- a/qemu-char.h > > +++ b/qemu-char.h > > @@ -6,6 +6,7 @@ > > #include "qemu-option.h" > > #include "qemu-config.h" > > #include "qobject.h" > > +#include "qstring.h" > > > > /* character device */ > > > > @@ -100,6 +101,12 @@ CharDriverState *qemu_chr_open_eventfd(int eventfd); > > > > extern int term_escape_char; > > > > +/* memory chardev */ > > +void qemu_chr_init_mem(CharDriverState *chr); > > +void qemu_chr_close_mem(CharDriverState *chr); > > +QString *qemu_chr_mem_to_qs(CharDriverState *chr); > > +size_t qemu_chr_mem_osize(const CharDriverState *chr); > > + > > /* async I/O support */ > > > > int qemu_set_fd_handler2(int fd, >