|
From: | Lei Li |
Subject: | Re: [Qemu-devel] [PATCH 2/4] QAPI: Introduce memchar-write QMP command |
Date: | Tue, 30 Oct 2012 18:22:28 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0 |
On 10/29/2012 09:23 PM, Luiz Capitulino wrote:
On Mon, 29 Oct 2012 12:10:24 +0800 Lei Li <address@hidden> wrote:On 10/27/2012 01:17 AM, Luiz Capitulino wrote:On Fri, 26 Oct 2012 19:21:50 +0800 Lei Li <address@hidden> wrote:Signed-off-by: Lei Li <address@hidden> --- hmp-commands.hx | 17 +++++++++++++++++ hmp.c | 15 +++++++++++++++ hmp.h | 1 + qapi-schema.json | 47 +++++++++++++++++++++++++++++++++++++++++++++++ qemu-char.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 34 ++++++++++++++++++++++++++++++++++ 6 files changed, 158 insertions(+), 0 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index e0b537d..a37b8e9 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -825,6 +825,23 @@ Inject an NMI on the given CPU (x86 only). ETEXI{+ .name = "memchar_write", + .args_type = "chardev:s,data:s", + .params = "chardev data", + .mhandler.cmd = hmp_memchar_write, + }, + +STEXI address@hidden memchar_write @var{chardev} @var{data} address@hidden memchar_write +Provide writing interface for CirMemCharDriver. Write @var{data} +to cirmemchr char device. Note that we will add 'control' options +for read and write command that specifies behavior when the queue +is full/empty, for now just assume a drop behaver in these two commands.You can drop everything after "Note".ok+ +ETEXI + + { .name = "migrate", .args_type = "detach:-d,blk:-b,inc:-i,uri:s", .params = "[-d] [-b] [-i] uri", diff --git a/hmp.c b/hmp.c index 2b97982..082985b 100644 --- a/hmp.c +++ b/hmp.c @@ -683,6 +683,21 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict) hmp_handle_error(mon, &errp); }+void hmp_memchar_write(Monitor *mon, const QDict *qdict)+{ + uint32_t size; + const char *chardev = qdict_get_str(qdict, "chardev"); + const char *data = qdict_get_str(qdict, "data"); + enum DataFormat format;Why do you need this variable?Sure, I will drop this.+ Error *errp = NULL; + + size = strlen(data); + format = DATA_FORMAT_UTF8; + qmp_memchar_write(chardev, size, data, true, format, &errp); + + hmp_handle_error(mon, &errp); +} + static void hmp_cont_cb(void *opaque, int err) { if (!err) { diff --git a/hmp.h b/hmp.h index 71ea384..406ebb1 100644 --- a/hmp.h +++ b/hmp.h @@ -43,6 +43,7 @@ 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_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 c615ee2..43ef6bc 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -325,6 +325,53 @@ { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }##+# @DataFormat: +# +# An enumeration of data format. The default value would +# be utf8.Please, remove the "default value" part. This is decided by the command using this type.Now the option format is optional, if it's not set then default by 'utf8'. I think it's a reasonable behaver. :)What I meant is that the right place to say what the value is is in the command documentation, not in @DataFormat doc. Here, only having "An enumeration of data encodings" is fine.
OK, done.
+# +# @utf8: The data format is 'utf8'. +# +# @base64: The data format is 'base64'. +# +# Note: The data format start with 'utf8' and 'base64', +# will support other data format as well.Please, drop this note. It's not needed.ok+# +# Since: 1.3 +## +{ 'enum': 'DataFormat' + 'data': [ 'utf8', 'base64' ] } + +## +# @memchar-write: +# +# Provide writing interface for memchardev. Write data to memchar +# char device. +# +# @chardev: the name of the memchar char device. +# +# @size: the size to write in bytes. Should be power of 2. +# +# @data: the source data write to memchar. +# +# @format: #optional the format of the data write to memchardev, by +# default is 'utf8'. +# +# Returns: Nothing on success +# If @chardev is not a valid memchr device, DeviceNotFound +# +# Notes: The option 'block' is not supported now due to the miss +# feature in qmp. Will add it later when we gain the necessary +# infrastructure enhancement. For now just assume 'drop' behaver +# for this command.Please, replace this note with an explanation of the current behavior. No need to talk about the future.Sure.+# +# Since: 1.3 +## +{ 'command': 'memchar-write', + 'data': {'chardev': 'str', 'size': 'int', 'data': 'str', + '*format': 'DataFormat'} } + +## # @CommandInfo: # # Information about a QMP command diff --git a/qemu-char.c b/qemu-char.c index c3ec43d..6114e29 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2717,6 +2717,50 @@ fail: return NULL; }+void qmp_memchar_write(const char *chardev, int64_t size,+ const char *data, bool has_format, + enum DataFormat format, + Error **errp) +{ + CharDriverState *chr; + guchar *write_data; + int ret; + gsize write_count; + + chr = qemu_chr_find(chardev); + if (!chr) { + error_set(errp, QERR_DEVICE_NOT_FOUND, chardev); + return; + } + + if (strcmp(chr->filename, "memchr") != 0) { + error_setg(errp,"%s is not memory char device\n", chardev); + return; + }This is wrong, as it's unreliable. A hackish (but hopefully correct) way of doing this would be to check chr->init against the circmem init function. But please, put this behind a function because it's ugly.Hmm, I guess it's because the chr->filename could be NULL. but we can still get it since CharDriverState keeps the inited type of backend. I am not sure that check chr->init against the cirmem init funciton could work, I'll have a try.+ + /* XXX: For the sync command as 'block', waiting for the qmp + * to support necessary feature. Now just act as 'drop' */Please, replace this note with an explanation of the current behavior.ok.+ if (cirmem_chr_is_full(chr)) { + error_setg(errp, "Failed to write to memchr %s", chardev); + return; + } + + write_count = (gsize)size; + + if (has_format && (format == DATA_FORMAT_BASE64)) { + write_data = g_base64_decode(data, &write_count); + } else { + write_data = (uint8_t *)data; + } + + ret = cirmem_chr_write(chr, write_data, write_count);IIRC circmem_chr_write() doesn't check if write_count if a power of two, what happens if it isn't?I have considered your suggestion on this in v4. The reason why I did not add the assert is that I think we have already check with this when cirmem init, so there is no need to check it again. Correct me if I am wrong. :)I was actually asking an honest question: does cirmem_chr_write() assume that len is a power of two? If yes, what if the user doesn't pass a power of two value to qmp_memchar_write()?
Ah, there must be some misunderstanding here... :) We let the size of ring buffer inited as power of 2, not the write_count. And it has already been checked when we init the CirMemCharDriver. That's why I did not add the assert in the function cirmem_chr_write.
+ + if (ret < 0) { + error_setg(errp, "Failed to write to memchr %s", chardev); + return; + } +} + QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename) { char host[65], port[33], width[8], height[8]; diff --git a/qmp-commands.hx b/qmp-commands.hx index 5ba8c48..7548b9b 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -466,6 +466,40 @@ Note: inject-nmi fails when the guest doesn't support injecting. EQMP{+ .name = "memchar-write", + .args_type = "chardev:s,size:i,data:s,format:s?,control:s?",You've dropped 'control', haven't you?Yes, sorry I forgot to drop here.. :(+ .help = "write size of data to memchar chardev", + .mhandler.cmd_new = qmp_marshal_input_memchar_write, + }, + +SQMP +memchar-write +------------- + +Provide writing interface for memchardev. Write data to memchar +char device. + +Arguments: + +- "chardev": the name of the char device, must be unique (json-string) +- "size": the memory size, in bytes, should be power of 2 (json-int) +- "data": the source data writed to memchar (json-string) +- "format": the data format write to memchardev, default is + utf8. (json-string, optional) + - Possible values: "utf8", "base64" + +Example: + +-> { "execute": "memchar-write", + "arguments": { "chardev": foo, + "size": 8, + "data": "abcdefgh", + "format": "utf8" } } +<- { "return": {} } + +EQMP + + { .name = "xen-save-devices-state", .args_type = "filename:F", .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,
-- Lei
[Prev in Thread] | Current Thread | [Next in Thread] |