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 <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
diff --git a/hmp-commands.hx b/hmp-commands.hx
index bdd48f3..66ec716 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -841,40 +841,37 @@
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 f6bb767..cbd5727 100644
--- a/hmp.c
+++ b/hmp.c
@@ -662,25 +662,25 @@
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 076d8cf..30b3c20 100644
--- a/hmp.h
+++ b/hmp.h
@@ -43,8 +43,8 @@
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 6f63791..736f881 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 831d564..8a35403 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2645,25 +2645,25 @@
}
/*********************************************************/
-/* 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
}
}
-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 @@
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 @@
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 @@
{ .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 @@
.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 2d44137..046bdc0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -1736,7 +1736,7 @@
"-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 @@
@option{udp},
@option{msmouse},
@option{vc},
-@option{memory},
+@option{ringbuf},
@option{file},
@option{pipe},
@option{console},
@@ -1887,13 +1887,10 @@
@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 51ce2e6..799adea 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,30 +466,30 @@
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 @@
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"}
