From 3949e59414fccefadc50ae65650d676cc734048c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 6 Feb 2013 21:27:24 +0100 Subject: qemu-char: Saner naming of memchar stuff & doc fixes New device, has never been released, so we can still improve things without worrying about compatibility. Naming is a mess. The code calls the device driver CirMemCharDriver, the public API calls it "memory", "memchardev", or "memchar", and the special commands are named like "memchar-FOO". "memory" is a particularly unfortunate choice, because there's another character device driver called MemoryDriver. Moreover, the device's distinctive property is that it's a ring buffer, not that's in memory. Therefore: * Rename CirMemCharDriver to RingBufCharDriver, and call the thing a "ringbuf" in the API. * Rename QMP and HMP commands from memchar-FOO to ringbuf-FOO. * Rename device parameter from maxcapacity to size (simple words are good for you). * Clearly mark the parameter as optional in documentation. * Fix error reporting so that chardev-add reports to current monitor, not stderr. * Replace cirmem in C identifiers by ringbuf. * Rework documentation. Document the impact of our crappy UTF-8 handling on reading. * QMP examples that even work. I could split this up into multiple commits, but they'd change the same documentation lines multiple times. Not worth it. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Signed-off-by: Anthony Liguori --- hmp-commands.hx | 37 +++++++++++++++----------------- hmp.c | 8 +++---- hmp.h | 4 ++-- qapi-schema.json | 47 ++++++++++++++++++++++++---------------- qemu-char.c | 65 +++++++++++++++++++++++++++++++------------------------- qemu-options.hx | 13 +++++------- qmp-commands.hx | 55 ++++++++++++++++++++++++----------------------- 7 files changed, 122 insertions(+), 107 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index bdd48f3469..66ec716e03 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -841,40 +841,37 @@ Inject an NMI on the given CPU (x86 only). ETEXI { - .name = "memchar_write", + .name = "ringbuf_write", .args_type = "device:s,data:s", .params = "device data", - .help = "Provide writing interface for CirMemCharDriver. Write" - "'data' to it.", - .mhandler.cmd = hmp_memchar_write, + .help = "Write to a ring buffer character device", + .mhandler.cmd = hmp_ringbuf_write, }, STEXI -@item memchar_write @var{device} @var{data} -@findex memchar_write -Provide writing interface for CirMemCharDriver. Write @var{data} -to char device 'memory'. +@item ringbuf_write @var{device} @var{data} +@findex ringbuf_write +Write @var{data} to ring buffer character device @var{device}. +@var{data} must be a UTF-8 string. ETEXI { - .name = "memchar_read", + .name = "ringbuf_read", .args_type = "device:s,size:i", .params = "device size", - .help = "Provide read interface for CirMemCharDriver. Read from" - "it and return the data with size.", - .mhandler.cmd = hmp_memchar_read, + .help = "Read from a ring buffer character device", + .mhandler.cmd = hmp_ringbuf_read, }, STEXI -@item memchar_read @var{device} -@findex memchar_read -Provide read interface for CirMemCharDriver. Read from char device -'memory' and return the data. - -@var{size} is the size of data want to read from. Refer to unencoded -size of the raw data, would adjust to the init size of the memchar -if the requested size is larger than it. +@item ringbuf_read @var{device} +@findex ringbuf_read +Read and print up to @var{size} bytes from ring buffer character +device @var{device}. +Bug: can screw up when the buffer contains invalid UTF-8 sequences, +NUL characters, after the ring buffer lost data, and when reading +stops because the size limit is reached. ETEXI diff --git a/hmp.c b/hmp.c index f6bb7675ba..cbd572753b 100644 --- a/hmp.c +++ b/hmp.c @@ -662,25 +662,25 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &errp); } -void hmp_memchar_write(Monitor *mon, const QDict *qdict) +void hmp_ringbuf_write(Monitor *mon, const QDict *qdict) { const char *chardev = qdict_get_str(qdict, "device"); const char *data = qdict_get_str(qdict, "data"); Error *errp = NULL; - qmp_memchar_write(chardev, data, false, 0, &errp); + qmp_ringbuf_write(chardev, data, false, 0, &errp); hmp_handle_error(mon, &errp); } -void hmp_memchar_read(Monitor *mon, const QDict *qdict) +void hmp_ringbuf_read(Monitor *mon, const QDict *qdict) { uint32_t size = qdict_get_int(qdict, "size"); const char *chardev = qdict_get_str(qdict, "device"); char *data; Error *errp = NULL; - data = qmp_memchar_read(chardev, size, false, 0, &errp); + data = qmp_ringbuf_read(chardev, size, false, 0, &errp); if (errp) { monitor_printf(mon, "%s\n", error_get_pretty(errp)); error_free(errp); diff --git a/hmp.h b/hmp.h index 076d8cf378..30b3c20ca4 100644 --- a/hmp.h +++ b/hmp.h @@ -43,8 +43,8 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict); void hmp_cpu(Monitor *mon, const QDict *qdict); void hmp_memsave(Monitor *mon, const QDict *qdict); void hmp_pmemsave(Monitor *mon, const QDict *qdict); -void hmp_memchar_write(Monitor *mon, const QDict *qdict); -void hmp_memchar_read(Monitor *mon, const QDict *qdict); +void hmp_ringbuf_write(Monitor *mon, const QDict *qdict); +void hmp_ringbuf_read(Monitor *mon, const QDict *qdict); void hmp_cont(Monitor *mon, const QDict *qdict); void hmp_system_wakeup(Monitor *mon, const QDict *qdict); void hmp_inject_nmi(Monitor *mon, const QDict *qdict); diff --git a/qapi-schema.json b/qapi-schema.json index 6f6379151f..736f881b75 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -329,9 +329,9 @@ # # An enumeration of data format. # -# @utf8: The data format is 'utf8'. +# @utf8: Data is a UTF-8 string (RFC 3629) # -# @base64: The data format is 'base64'. +# @base64: Data is Base64 encoded binary (RFC 3548) # # Since: 1.4 ## @@ -339,44 +339,55 @@ 'data': [ 'utf8', 'base64' ] } ## -# @memchar-write: +# @ringbuf-write: # -# Provide writing interface for memchardev. Write data to char -# device 'memory'. +# Write to a ring buffer character device. # -# @device: the name of the memory char device. +# @device: the ring buffer character device name # -# @data: the source data write to memchar. +# @data: data to write # -# @format: #optional the format of the data write to chardev 'memory', -# by default is 'utf8'. +# @format: #optional data encoding (default 'utf8'). +# - base64: data must be base64 encoded text. Its binary +# decoding gets written. +# Bug: invalid base64 is currently not rejected. +# Whitespace *is* invalid. +# - utf8: data's UTF-8 encoding is written +# - data itself is always Unicode regardless of format, like +# any other string. # # Returns: Nothing on success # # Since: 1.4 ## -{ 'command': 'memchar-write', +{ 'command': 'ringbuf-write', 'data': {'device': 'str', 'data': 'str', '*format': 'DataFormat'} } ## -# @memchar-read: +# @ringbuf-read: # -# Provide read interface for memchardev. Read from the char -# device 'memory' and return the data. +# Read from a ring buffer character device. # -# @device: the name of the memory char device. +# @device: the ring buffer character device name # -# @size: the size to read in bytes. +# @size: how many bytes to read at most # -# @format: #optional the format of the data want to read from -# memchardev, by default is 'utf8'. +# @format: #optional data encoding (default 'utf8'). +# - base64: the data read is returned in base64 encoding. +# - utf8: the data read is interpreted as UTF-8. +# Bug: can screw up when the buffer contains invalid UTF-8 +# sequences, NUL characters, after the ring buffer lost +# data, and when reading stops because the size limit is +# reached. +# - The return value is always Unicode regardless of format, +# like any other string. # # Returns: data read from the device # # Since: 1.4 ## -{ 'command': 'memchar-read', +{ 'command': 'ringbuf-read', 'data': {'device': 'str', 'size': 'int', '*format': 'DataFormat'}, 'returns': 'str' } diff --git a/qemu-char.c b/qemu-char.c index 831d564a8b..8a3540389a 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2645,25 +2645,25 @@ size_t qemu_chr_mem_osize(const CharDriverState *chr) } /*********************************************************/ -/* CircularMemory chardev */ +/* Ring buffer chardev */ typedef struct { size_t size; size_t prod; size_t cons; uint8_t *cbuf; -} CirMemCharDriver; +} RingBufCharDriver; -static size_t cirmem_count(const CharDriverState *chr) +static size_t ringbuf_count(const CharDriverState *chr) { - const CirMemCharDriver *d = chr->opaque; + const RingBufCharDriver *d = chr->opaque; return d->prod - d->cons; } -static int cirmem_chr_write(CharDriverState *chr, const uint8_t *buf, int len) +static int ringbuf_chr_write(CharDriverState *chr, const uint8_t *buf, int len) { - CirMemCharDriver *d = chr->opaque; + RingBufCharDriver *d = chr->opaque; int i; if (!buf || (len < 0)) { @@ -2680,9 +2680,9 @@ static int cirmem_chr_write(CharDriverState *chr, const uint8_t *buf, int len) return 0; } -static int cirmem_chr_read(CharDriverState *chr, uint8_t *buf, int len) +static int ringbuf_chr_read(CharDriverState *chr, uint8_t *buf, int len) { - CirMemCharDriver *d = chr->opaque; + RingBufCharDriver *d = chr->opaque; int i; for (i = 0; i < len && d->cons != d->prod; i++) { @@ -2692,31 +2692,31 @@ static int cirmem_chr_read(CharDriverState *chr, uint8_t *buf, int len) return i; } -static void cirmem_chr_close(struct CharDriverState *chr) +static void ringbuf_chr_close(struct CharDriverState *chr) { - CirMemCharDriver *d = chr->opaque; + RingBufCharDriver *d = chr->opaque; g_free(d->cbuf); g_free(d); chr->opaque = NULL; } -static CharDriverState *qemu_chr_open_cirmemchr(QemuOpts *opts) +static CharDriverState *qemu_chr_open_ringbuf(QemuOpts *opts) { CharDriverState *chr; - CirMemCharDriver *d; + RingBufCharDriver *d; chr = g_malloc0(sizeof(CharDriverState)); d = g_malloc(sizeof(*d)); - d->size = qemu_opt_get_number(opts, "maxcapacity", 0); + d->size = qemu_opt_get_number(opts, "size", 0); if (d->size == 0) { d->size = CBUFF_SIZE; } /* The size must be power of 2 */ if (d->size & (d->size - 1)) { - fprintf(stderr, "chardev: size of memory device must be power of 2\n"); + error_report("size of ringbuf device must be power of two"); goto fail; } @@ -2725,8 +2725,8 @@ static CharDriverState *qemu_chr_open_cirmemchr(QemuOpts *opts) d->cbuf = g_malloc0(d->size); chr->opaque = d; - chr->chr_write = cirmem_chr_write; - chr->chr_close = cirmem_chr_close; + chr->chr_write = ringbuf_chr_write; + chr->chr_close = ringbuf_chr_close; return chr; @@ -2736,12 +2736,12 @@ fail: return NULL; } -static bool chr_is_cirmem(const CharDriverState *chr) +static bool chr_is_ringbuf(const CharDriverState *chr) { - return chr->chr_write == cirmem_chr_write; + return chr->chr_write == ringbuf_chr_write; } -void qmp_memchar_write(const char *device, const char *data, +void qmp_ringbuf_write(const char *device, const char *data, bool has_format, enum DataFormat format, Error **errp) { @@ -2756,8 +2756,8 @@ void qmp_memchar_write(const char *device, const char *data, return; } - if (!chr_is_cirmem(chr)) { - error_setg(errp,"%s is not memory char device", device); + if (!chr_is_ringbuf(chr)) { + error_setg(errp,"%s is not a ringbuf device", device); return; } @@ -2768,7 +2768,7 @@ void qmp_memchar_write(const char *device, const char *data, write_count = strlen(data); } - ret = cirmem_chr_write(chr, write_data, write_count); + ret = ringbuf_chr_write(chr, write_data, write_count); if (write_data != (uint8_t *)data) { g_free((void *)write_data); @@ -2780,7 +2780,7 @@ void qmp_memchar_write(const char *device, const char *data, } } -char *qmp_memchar_read(const char *device, int64_t size, +char *qmp_ringbuf_read(const char *device, int64_t size, bool has_format, enum DataFormat format, Error **errp) { @@ -2795,8 +2795,8 @@ char *qmp_memchar_read(const char *device, int64_t size, return NULL; } - if (!chr_is_cirmem(chr)) { - error_setg(errp,"%s is not memory char device", device); + if (!chr_is_ringbuf(chr)) { + error_setg(errp,"%s is not a ringbuf device", device); return NULL; } @@ -2805,16 +2805,23 @@ char *qmp_memchar_read(const char *device, int64_t size, return NULL; } - count = cirmem_count(chr); + count = ringbuf_count(chr); size = size > count ? count : size; read_data = g_malloc(size + 1); - cirmem_chr_read(chr, read_data, size); + ringbuf_chr_read(chr, read_data, size); if (has_format && (format == DATA_FORMAT_BASE64)) { data = g_base64_encode(read_data, size); g_free(read_data); } else { + /* + * FIXME should read only complete, valid UTF-8 characters up + * to @size bytes. Invalid sequences should be replaced by a + * suitable replacement character. Except when (and only + * when) ring buffer lost characters since last read, initial + * continuation characters should be dropped. + */ read_data[size] = 0; data = (char *)read_data; } @@ -2975,7 +2982,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 = "memory", .open = qemu_chr_open_cirmemchr }, + { .name = "memory", .open = qemu_chr_open_ringbuf }, #ifdef _WIN32 { .name = "file", .open = qemu_chr_open_win_file_out }, { .name = "pipe", .open = qemu_chr_open_win_pipe }, @@ -3236,7 +3243,7 @@ QemuOptsList qemu_chardev_opts = { .name = "debug", .type = QEMU_OPT_NUMBER, },{ - .name = "maxcapacity", + .name = "size", .type = QEMU_OPT_NUMBER, }, { /* end of list */ } diff --git a/qemu-options.hx b/qemu-options.hx index 2d44137bf9..046bdc0f63 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -1736,7 +1736,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 memory,id=id,maxcapacity=maxcapacity\n" + "-chardev ringbuf,id=id[,size=size]\n" "-chardev file,id=id,path=path[,mux=on|off]\n" "-chardev pipe,id=id,path=path[,mux=on|off]\n" #ifdef _WIN32 @@ -1778,7 +1778,7 @@ Backend is one of: @option{udp}, @option{msmouse}, @option{vc}, -@option{memory}, +@option{ringbuf}, @option{file}, @option{pipe}, @option{console}, @@ -1887,13 +1887,10 @@ 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 memory ,id=@var{id} ,maxcapacity=@var{maxcapacity} +@item -chardev ringbuf ,id=@var{id} [,size=@var{size}] -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. +Create a ring buffer with fixed size @option{size}. +@var{size} must be a power of two, and defaults to @code{64K}). @item -chardev file ,id=@var{id} ,path=@var{path} diff --git a/qmp-commands.hx b/qmp-commands.hx index 51ce2e6b41..799adea1b7 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -466,30 +466,30 @@ Note: inject-nmi fails when the guest doesn't support injecting. EQMP { - .name = "memchar-write", + .name = "ringbuf-write", .args_type = "device:s,data:s,format:s?", - .mhandler.cmd_new = qmp_marshal_input_memchar_write, + .mhandler.cmd_new = qmp_marshal_input_ringbuf_write, }, SQMP -memchar-write +ringbuf-write ------------- -Provide writing interface for CirMemCharDriver. Write data to memory -char device. +Write to a ring buffer character device. Arguments: -- "device": the name of the char device, must be unique (json-string) -- "data": the source data write to memory (json-string) -- "format": the data format write to memory, default is - utf8. (json-string, optional) - - Possible values: "utf8", "base64" +- "device": ring buffer character device name (json-string) +- "data": data to write (json-string) +- "format": data format (json-string, optional) + - Possible values: "utf8" (default), "base64" + Bug: invalid base64 is currently not rejected. + Whitespace *is* invalid. Example: --> { "execute": "memchar-write", - "arguments": { "device": foo, +-> { "execute": "ringbuf-write", + "arguments": { "device": "foo", "data": "abcdefgh", "format": "utf8" } } <- { "return": {} } @@ -497,32 +497,35 @@ Example: EQMP { - .name = "memchar-read", + .name = "ringbuf-read", .args_type = "device:s,size:i,format:s?", - .mhandler.cmd_new = qmp_marshal_input_memchar_read, + .mhandler.cmd_new = qmp_marshal_input_ringbuf_read, }, SQMP -memchar-read +ringbuf-read ------------- -Provide read interface for CirMemCharDriver. Read from the char -device memory and return the data with size. +Read from a ring buffer character device. Arguments: -- "device": the name of the char device, must be unique (json-string) -- "size": the memory size wanted to read in bytes (refer to unencoded - size of the raw data), would adjust to the init size of the - memchar if the requested size is larger than it. (json-int) -- "format": the data format write to memchardev, default is - utf8. (json-string, optional) - - Possible values: "utf8", "base64" +- "device": ring buffer character device name (json-string) +- "size": how many bytes to read at most (json-int) + - Number of data bytes, not number of characters in encoded data +- "format": data format (json-string, optional) + - Possible values: "utf8" (default), "base64" + - Naturally, format "utf8" works only when the ring buffer + contains valid UTF-8 text. Invalid UTF-8 sequences get + replaced. Bug: replacement doesn't work. Bug: can screw + up on encountering NUL characters, after the ring buffer + lost data, and when reading stops because the size limit + is reached. Example: --> { "execute": "memchar-read", - "arguments": { "device": foo, +-> { "execute": "ringbuf-read", + "arguments": { "device": "foo", "size": 1000, "format": "utf8" } } <- {"return": "abcdefgh"} -- cgit v1.2.1