On 10/27/2012 12:47 AM, Luiz Capitulino wrote:
On Fri, 26 Oct 2012 19:21:49 +0800 Lei Li <li...@linux.vnet.ibm.com> wrote:Signed-off-by: Lei Li <li...@linux.vnet.ibm.com> --- qemu-char.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ qemu-config.c | 3 + qemu-options.hx | 10 ++++ 3 files changed, 153 insertions(+), 0 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index b082bae..c3ec43d 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -99,6 +99,7 @@ #include "ui/qemu-spice.h"#define READ_BUF_LEN 4096+#define CBUFF_SIZE 65536/***********************************************************//* character device */ @@ -2588,6 +2589,134 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr) return d->outbuf_size; }+/*********************************************************/+/*CircularMemory chardev*/ + +typedef struct { + size_t size; + size_t producer; + size_t consumer; + uint8_t *cbuf; +} CirMemCharDriver; + +static bool cirmem_chr_is_empty(const CharDriverState *chr) +{ + const CirMemCharDriver *d = chr->opaque; + + return d->producer == d->consumer; +} + +static bool cirmem_chr_is_full(const CharDriverState *chr) +{ + const CirMemCharDriver *d = chr->opaque; + + return (d->producer - d->consumer) >= d->size; +} + +static bool cirmem_chr_unlimit(const CharDriverState *chr) +{ + const CirMemCharDriver *d = chr->opaque; + + return d->producer >= d->consumer; +}I'm not sure I get why this is needed. Are you trying to fix the problem I mentioned in my last review that producer and consumer might overflow for long running VMs? If yes, then the problem still exists. The only fix I can think of is to re-initialize them. Of course that you could do it in a way that they keep pointing to the right position in cbuf[].+ +static int cirmem_chr_write(CharDriverState *chr, const uint8_t *buf, int len) +{ + CirMemCharDriver *d = chr->opaque; + int i; + + if (!buf || (len < 0)) { + return -1; + } + + for (i = 0; i < len; i++ ) { + if (cirmem_chr_unlimit(chr)) { + /* Avoid writing the IAC information to the queue. */ + if ((unsigned char)buf[i] == IAC) { + continue; + } + d->cbuf[d->producer % d->size] = buf[i]; + d->producer++; + } else { + return -1; + } + } + + return 0; +} + +static int cirmem_chr_read(CharDriverState *chr, uint8_t *buf, int len) +{ + CirMemCharDriver *d = chr->opaque; + int i; + + if (cirmem_chr_is_empty(chr) || len < 0) { + return -1; + } + + if (len > d->size) { + len = d->size; + } + + for (i = 0; i < len; i++) { + if (cirmem_chr_unlimit(chr)) { + buf[i] = d->cbuf[d->consumer % d->size]; + d->consumer++; + + if (cirmem_chr_is_empty(chr)) { + break; + } + } else { + return -1; + } + } + + return 0; +} + +static void cirmem_chr_close(struct CharDriverState *chr) +{ + CirMemCharDriver *d = chr->opaque; + + g_free(d->cbuf); + g_free(d); + chr->opaque = NULL; +} + +static CharDriverState *qemu_chr_open_cirmemchr(QemuOpts *opts) +{ + CharDriverState *chr; + CirMemCharDriver *d; + + chr = g_malloc0(sizeof(CharDriverState)); + d = g_malloc(sizeof(*d)); + + d->size = qemu_opt_get_number(opts, "maxcapacity", 0); + if (d->size == 0) { + d->size = CBUFF_SIZE; + } + + /* The size must be power of 2 */If this is enforced in vl.c (which I can't remember right now), then you should turn this into an assert().
It is just enforced in here, not in vl.c.
+ if (d->size & (d->size - 1)) { + goto fail; + } + + d->producer = 0; + d->consumer = 0; + d->cbuf = g_malloc0(d->size); + + chr->opaque = d; + chr->chr_write = cirmem_chr_write; + chr->chr_close = cirmem_chr_close; + + return chr; + +fail: + g_free(d); + g_free(chr); + return NULL; +} + QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) { char host[65], port[33], width[8], height[8]; @@ -2652,6 +2781,16 @@ QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) qemu_opt_set(opts, "path", p); return opts; } + if (strstart(filename, "memchr", &p)) { + qemu_opt_set(opts, "backend", "memchr"); + qemu_opt_set(opts, "maxcapacity", p); + return opts; + } + if (strstart(filename, "memchr", &p)) { + qemu_opt_set(opts, "backend", "memchr"); + qemu_opt_set(opts, "maxcapacity", p); + return opts;Duplication?
Yeah...
+ } if (strstart(filename, "tcp:", &p) || strstart(filename, "telnet:", &p)) { if (sscanf(p, "%64[^:]:%32[^,]%n", host, port, &pos) < 2) { @@ -2725,6 +2864,7 @@ static const struct { { .name = "udp", .open = qemu_chr_open_udp }, { .name = "msmouse", .open = qemu_chr_open_msmouse }, { .name = "vc", .open = text_console_init }, + { .name = "memchr", .open = qemu_chr_open_cirmemchr },What about my suggestion of calling this "memory"? All backends here are chardevs, but we don't have vcchr, msmousechr, stdiochr etc.
Yes, it make sense.
#ifdef _WIN32 { .name = "file", .open = qemu_chr_open_win_file_out }, { .name = "pipe", .open = qemu_chr_open_win_pipe }, diff --git a/qemu-config.c b/qemu-config.c index cd1ec21..5553bb6 100644 --- a/qemu-config.c +++ b/qemu-config.c @@ -213,6 +213,9 @@ static QemuOptsList qemu_chardev_opts = { },{ .name = "debug", .type = QEMU_OPT_NUMBER, + },{ + .name = "maxcapacity", + .type = QEMU_OPT_NUMBER, }, { /* end of list */ } }, diff --git a/qemu-options.hx b/qemu-options.hx index 46f0539..9b2d792 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1680,6 +1680,7 @@ DEF("chardev", HAS_ARG, QEMU_OPTION_chardev, "-chardev msmouse,id=id[,mux=on|off]\n" "-chardev vc,id=id[[,width=width][,height=height]][[,cols=cols][,rows=rows]]\n" " [,mux=on|off]\n" + "-chardev memchr,id=id,maxcapacity=maxcapacity\n" "-chardev file,id=id,path=path[,mux=on|off]\n" "-chardev pipe,id=id,path=path[,mux=on|off]\n" #ifdef _WIN32 @@ -1718,6 +1719,7 @@ Backend is one of: @option{udp}, @option{msmouse}, @option{vc}, +@option{memchr}, @option{file}, @option{pipe}, @option{console}, @@ -1824,6 +1826,14 @@ the console, in pixels. @option{cols} and @option{rows} specify that the console be sized to fit a text console with the given dimensions.+@item -chardev memchr ,id=@var{id} ,maxcapacity=@var{maxcapacity}+ +Create a circular buffer with fixed size indicated by optionally @option{maxcapacity} +which will be default 64K if it is not given. + +@option{maxcapacity} specifies the max capacity of the size of circular buffer +to create. Should be power of 2. + @item -chardev file ,id=@var{id} ,path=@var{path}Log all traffic received from the guest to a file.
-- Lei
