qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: kwolf@redhat.com, marcandre.lureau@redhat.com,
	qemu-block@nongnu.org, pbonzini@redhat.com
Subject: [PATCH 4/4] qemu-storage-daemon: QAPIfy --chardev
Date: Mon, 26 Oct 2020 11:10:05 +0100	[thread overview]
Message-ID: <20201026101005.2940615-5-armbru@redhat.com> (raw)
In-Reply-To: <20201026101005.2940615-1-armbru@redhat.com>

From: Kevin Wolf <kwolf@redhat.com>

This removes the dependency on QemuOpts from the --chardev option of
the storage daemon.

Help on option parameters is still wrong.  Marked FIXME.

There are quite a few differences between qemu-system-FOO -chardev,
QMP chardev-add, and qemu-storage-daemon --chardev:

* QMP chardev-add wraps arguments other than "id" in a "backend"
  object.  Parameters other than "type" are further wrapped in a
  "data" object.  Example:

        {"execute": "chardev-add",
         "arguments": {
             "id":"sock0",
             "backend": {
                 "type": "socket",
                 "data": {
                     "addr": {
                         "type": "inet",
			 ...
        }}}}}

  qemu-system-FOO -chardev does not wrap.  Neither does
  qemu-storage-daemon --chardev.

* qemu-system-FOO -chardev parameter "backend" corresponds to QMP
  chardev-add "backend" member "type".  qemu-storage-daemon names it
  "backend".

* qemu-system-FOO -chardev parameter "backend" recognizes a few
  additional aliases for compatibility.  QMP chardev-add does not.
  Neither does qemu-storage-daemon --chardev.

* qemu-system-FOO -chardev' with types "serial", "parallel" and "pipe"
  parameter "path" corresponds to QMP chardev-add member "device".
  qemu-storage-daemon --chardev follows QMP.

* Backend type "socket":

  - Intentionally different defaults (documented as such):
    qemu-system-FOO -chardev defaults to server=false and
    wait=true (if server=true), but QMP chardev-add defaults to
    server=true and wait=false.  qemu-storage-daemon --chardev follows
    QMP.

  - Accidentally different defaults: qemu-system-FOO -chardev defaults
    to tight=true, QMP chardev-add defaults to tight=false in
    QMP (this is a bug in commit 776b97d3).  qemu-storage-daemon
    follows QMP.

  - QMP chardev-add wraps socket address arguments "path", "host",
    "port", etc in a "data" object.  qemu-system-FOO -chardev does not
    wrap.  Neither does qemu-storage-daemon --chardev.

  - qemu-system-FOO -chardev parameter "delay" corresponds to QMP
    chardev-add member "nodelay" with the sense reversed.
    qemu-storage-daemon --chardev follows QMP.

* Backend type "udp":

  - QMP chardev-add wraps remote and local address arguments in a
    "remote" and a "local" object, respectively.  qemu-system-FOO
    -chardev does not wrap, but prefixes the local address parameter
    names with "local" instead.

  - QMP chardev-add wraps socket address arguments in a "data" object.
    qemu-system-FOO -chardev does not wrap.  Neither does
    qemu-storage-daemon --chardev.  Same as for type "socket".

* I'm not sure qemu-system-FOO -chardev supports everything QMP
  chardev-add does.  I am sure qemu-storage-daemon --chardev does.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 storage-daemon/qemu-storage-daemon.c | 37 +++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/storage-daemon/qemu-storage-daemon.c b/storage-daemon/qemu-storage-daemon.c
index e419ba9f19..f1f3bdc320 100644
--- a/storage-daemon/qemu-storage-daemon.c
+++ b/storage-daemon/qemu-storage-daemon.c
@@ -37,10 +37,13 @@
 #include "qapi/error.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "qapi/qapi-visit-block-export.h"
+#include "qapi/qapi-visit-char.h"
+#include "qapi/qapi-visit-char.h"
 #include "qapi/qapi-visit-control.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qstring.h"
 #include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
 
 #include "qemu-common.h"
 #include "qemu-version.h"
@@ -207,18 +210,34 @@ static void process_options(int argc, char *argv[])
             }
         case OPTION_CHARDEV:
             {
-                /* TODO This interface is not stable until we QAPIfy it */
-                QemuOpts *opts = qemu_opts_parse_noisily(&qemu_chardev_opts,
-                                                         optarg, true);
-                if (opts == NULL) {
-                    exit(EXIT_FAILURE);
-                }
+                QDict *args;
+                Visitor *v;
+                ChardevOptions *chr;
+                q_obj_chardev_add_arg *arg;
+                bool help;
 
-                if (!qemu_chr_new_from_opts(opts, NULL, &error_fatal)) {
-                    /* No error, but NULL returned means help was printed */
+                args = keyval_parse(optarg, "backend", &help, &error_fatal);
+                if (help) {
+                    if (qdict_haskey(args, "backend")) {
+                        /* FIXME wrong where QAPI differs from QemuOpts */
+                        qemu_opts_print_help(&qemu_chardev_opts, true);
+                    } else {
+                        qemu_chr_print_types();
+                    }
                     exit(EXIT_SUCCESS);
                 }
-                qemu_opts_del(opts);
+
+                v = qobject_input_visitor_new_keyval(QOBJECT(args));
+                visit_type_ChardevOptions(v, NULL, &chr, &error_fatal);
+                visit_free(v);
+
+                arg = chardev_options_crumple(chr);
+
+                qmp_chardev_add(arg->id, arg->backend, &error_fatal);
+                g_free(arg->id);
+                qapi_free_ChardevBackend(arg->backend);
+                qapi_free_ChardevOptions(chr);
+                qobject_unref(args);
                 break;
             }
         case OPTION_EXPORT:
-- 
2.26.2



  parent reply	other threads:[~2020-10-26 10:41 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-26 10:10 [PATCH 0/4] qemu-storage-daemon: QAPIfy --chardev the stupid way Markus Armbruster
2020-10-26 10:10 ` [PATCH 1/4] char/stdio: Fix QMP default for 'signal' Markus Armbruster
2020-10-26 10:10 ` [PATCH 2/4] char: Factor out qemu_chr_print_types() Markus Armbruster
2020-10-26 10:10 ` [PATCH 3/4] char: Flat alternative to overly nested chardev-add arguments Markus Armbruster
2020-10-27 18:23   ` Eric Blake
2020-10-28  7:33     ` Markus Armbruster
2020-10-26 10:10 ` Markus Armbruster [this message]
2020-10-27 18:59   ` [PATCH 4/4] qemu-storage-daemon: QAPIfy --chardev Eric Blake
2020-10-28  7:42     ` Markus Armbruster
2020-10-28  9:18   ` Markus Armbruster
2020-10-27 18:36 ` [PATCH 0/4] qemu-storage-daemon: QAPIfy --chardev the stupid way Paolo Bonzini
2020-10-28  7:01   ` Markus Armbruster
2020-10-28 11:46     ` Kevin Wolf
2020-10-28 14:39       ` Paolo Bonzini
2020-10-28 14:59         ` Kevin Wolf
2020-10-28 15:09           ` Paolo Bonzini
2020-10-28 15:39             ` Kevin Wolf
2020-10-28 16:01               ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201026101005.2940615-5-armbru@redhat.com \
    --to=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=marcandre.lureau@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).