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().
> + 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?
> + }
> 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.
> #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.
