qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/5] QAPI: Introduce memchar-write QMP command


From: Lei Li
Subject: Re: [Qemu-devel] [PATCH 3/5] QAPI: Introduce memchar-write QMP command
Date: Thu, 20 Sep 2012 15:42:30 +0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120827 Thunderbird/15.0

On 09/20/2012 02:05 AM, Luiz Capitulino wrote:
On Wed, 12 Sep 2012 19:57:24 +0800
Lei Li <address@hidden> wrote:

Signed-off-by: Lei Li <address@hidden>
---
  hmp-commands.hx  |   23 ++++++++++++++++++
  hmp.c            |   19 +++++++++++++++
  hmp.h            |    1 +
  qapi-schema.json |   69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
  qemu-char.c      |   48 +++++++++++++++++++++++++++++++++++++
  qmp-commands.hx  |   39 ++++++++++++++++++++++++++++++
  6 files changed, 199 insertions(+), 0 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ed67e99..fe11926 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -796,6 +796,29 @@ Inject an NMI on the given CPU (x86 only).
  ETEXI
{
+        .name       = "memchar_write",
+        .args_type  = "chardev:s,size:i,data:s,format:s?,control:s?",
+        .params     = "chardev size data [format] [control]",
+        .help       = "Provide writing interface for CirMemCharDriver. Write"
+                      "'data' to it with size 'size'",
+        .mhandler.cmd = hmp_memchar_write,
+    },
That's a too low level command for hmp. I'd actually consider dropping
it in favor of the console command, but if you really want to have this
then I suggest making the following changes:

  o drop 'size', as this can be calculated from the user input string
  o drop 'format', as base64 doesn't make much sense for hmp
  o turn 'control' into a flag, like -b for block and -d for drop

Also applies for memchar_read.

+
+STEXI
address@hidden memchar_write @var{chardev} @var{size} @var{data} address@hidden 
address@hidden
address@hidden memchar_write
+Provide writing interface for CirMemCharDriver. Write @var{data}
+to cirmemchr char device with size @var{size}.
+
address@hidden is the format of the data write to CirMemCharDriver.
+Support 'utf8' and 'base64', by default is 'utf8'.
+
address@hidden is option('block', 'drop') for read and write command
+that specifies behavior when the queue is full/empty. By default is
+'drop'. Note that the 'block' option is not supported now.
+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 ba6fbd3..97f5058 100644
--- a/hmp.c
+++ b/hmp.c
@@ -671,6 +671,25 @@ 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 = qdict_get_int(qdict, "size");
+    const char *chardev = qdict_get_str(qdict, "chardev");
+    const char *data = qdict_get_str(qdict, "data");
+    int val = qdict_get_try_bool(qdict, "base64", 0);
+    enum DataFormat format;
+    int con = qdict_get_try_bool(qdict, "block", 0);
+    enum CongestionControl control;
+    Error *errp = NULL;
+
+    format = val ? DATA_FORMAT_BASE64 : DATA_FORMAT_UTF8;
+    control = con ? CONGESTION_CONTROL_BLOCK : CONGESTION_CONTROL_DROP;
+    qmp_memchar_write(chardev, size, data, true, format,
+                      true, control, &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 48b9c59..44b6463 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 a9f465a..371239a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -236,6 +236,75 @@
  { 'command': 'query-chardev', 'returns': ['ChardevInfo'] }
##
+# @DataFormat:
+#
+# An enumeration of data format write to or read from
+# memchardev. The default value would be utf8.
+#
+# @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.
+#
+# Since: 1.3
+##
+{ 'enum': 'DataFormat'
+  'data': [ 'utf8', 'base64' ] }
+
+##
+# @CongestionControl
+#
+# An enumeration of options for the read and write command that
+# specifies behavior when the queue is full/empty. The default
+# option would be dropped.
+#
+# @drop: Would result in reads returning empty strings and writes
+#        dropping queued data.
+#
+# @block: Would make the session block until data was available
+#         or the queue had space available.
+#
+# Note: 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.
That's not acceptable. We either, hold the inclusion of this series
until this is ready or we hardcode drop for now and add new commands
supporting @CongestionControl later.

+#
+# Since: 1.3
+##
+{'enum': 'CongestionControl'
+ 'data': [ 'drop', 'block' ] }
+
+##
+# @memchar-write:
+#
+# Provide writing interface for CirMemCharDriver. Write data to cirmemchar
+# char device.
+#
+# @chardev: the name of the cirmemchar char device.
+#
+# @size: the size to write in bytes.
+#
+# @data: the source data write to cirmemchar.
+#
+# @format: #optional the format of the data write to cirmemchar, by
+#          default is 'utf8'.
+#
+# @control: #optional options for read and write command that specifies
+#           behavior when the queue is full/empty.
+#
+# Returns: Nothing on success
+#          If @chardev is not a valid cirmemchr device, DeviceNotFound
+#          If an I/O error occurs while writing, IOError
This last line can be dropped.

+#
+# Since: 1.3
+##
+{ 'command': 'memchar-write',
+  'data': {'chardev': 'str', 'size': 'int', 'data': 'str',
+           '*format': 'DataFormat',
I don't think format should be optional.

+           '*control': 'CongestionControl'} }
+
+##
  # @CommandInfo:
  #
  # Information about a QMP command
diff --git a/qemu-char.c b/qemu-char.c
index 6e84acc..be1d79a 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2700,6 +2700,54 @@ static CharDriverState *qemu_chr_open_cirmemchr(QemuOpts 
*opts)
      return chr;
  }
+void qmp_memchar_write(const char *chardev, int64_t size,
+                       const char *data, bool has_format,
+                       enum DataFormat format, bool has_control,
+                       enum CongestionControl control,
+                       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;
+    }
+
+    /* XXX: For the sync command as 'block', waiting for the qmp
+ *      * to support necessary feature. Now just act as 'drop'. */
+    if (cirmem_chr_is_full(chr)) {
+        if (has_control && (control == CONGESTION_CONTROL_BLOCK)) {
+            g_free((char *)data);
+            return;
+        } else {
+            g_free((char *)data);
+            error_setg(errp, "Failed to write to memchr %s", chardev);
+            return;
+        }
+    }
Why do you free data above?

Like the document motioned, we have two options { 'drop', 'block' } as 
congestion
mechanism suggested by Anthony, when set 'drop' would result in reads returning
empty strings and writes dropping queued data. Set 'block' then we could make 
the
session block until data was available or the queue had space available.
Now free the data for option 'block' too since we lack such infrastructure now,
will act as 'block' later when we gain the necessary infrastructure enhancement.

+
+    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);
+    /* */
+    if (ret <= 0) {
+        error_setg(errp, "Failed to write to memchr %s", chardev);
+    }
+
+    g_free(write_data);
+}
+
  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 6e21ddb..5a1b86b 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -466,6 +466,45 @@ 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?",
+        .mhandler.cmd_new = qmp_marshal_input_memchar_write,
+    },
+
+SQMP
+memchar-write
+-------------
+
+Provide writing interface for CirMemCharDriver. Write data to cirmemchar
+char device.
+
+Arguments:
+
+- "chardev": the name of the char device, must be unique (json-string)
+- "size": the memory size, in bytes (json-int)
+- "data": the source data writed to cirmemchar (json-string)
+- "format": the data format write to CirMemCharDriver, default is
+            utf8. (json-string, optional)
+          - Possible values: "utf8", "base64"
+
+- "control": options for read and write command that specifies
+             behavior when the queue is full/empty, default is
+             drop. (json-string, optional)
+          - Possible values: "drop", "block"
+
+Example:
+
+-> { "execute": "memchar-write",
+                "arguments": { "chardev": foo,
+                               "size": 7,
+                               "data": "abcdefg",
+                               "format": "utf8",
+                               "control": "drop" } }
+<- { "return": {} }
+
+EQMP
+
+    {
          .name       = "xen-save-devices-state",
          .args_type  = "filename:F",
      .mhandler.cmd_new = qmp_marshal_input_xen_save_devices_state,



--
Lei




reply via email to

[Prev in Thread] Current Thread [Next in Thread]