Patchwork [05/11] chardev: add hmp hotplug commands

login
register
mail settings
Submitter Gerd Hoffmann
Date Jan. 7, 2013, 1:55 p.m.
Message ID <1357566928-25361-6-git-send-email-kraxel@redhat.com>
Download mbox | patch
Permalink /patch/209938/
State New
Headers show

Comments

Gerd Hoffmann - Jan. 7, 2013, 1:55 p.m.
Add chardev-add and chardev-remove commands to the human monitor.
chardev-add accepts the same syntax as -chardev, chardev-remove
expects a chardev id.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hmp-commands.hx |   32 ++++++++++++++++++++++++++++++++
 hmp.c           |   23 +++++++++++++++++++++++
 hmp.h           |    2 ++
 3 files changed, 57 insertions(+), 0 deletions(-)
Paolo Bonzini - Jan. 10, 2013, 10:33 a.m.
Il 07/01/2013 14:55, Gerd Hoffmann ha scritto:
> +void hmp_chardev_add(Monitor *mon, const QDict *qdict)
> +{
> +    const char *args = qdict_get_str(qdict, "args");
> +    Error *err = NULL;
> +    QemuOpts *opts;
> +
> +    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
> +    if (opts == NULL) {
> +        error_setg(&err, "Parsing chardev args failed\n");
> +    } else {
> +        qemu_chr_new_from_opts(opts, NULL, &err);

This ought to use qmp_chardev_add and a generic opts->ChardevBackend
conversion.

But IMHO, this kind of intermediate conversion is okay, with the
"correct" thing deferred; being able to play with hotplug from HMP is
worth the small wart.  It's really Luiz's decision, so I'm not giving
the reviewed-by (yet).

Paolo

> +    }
> +    hmp_handle_error(mon, &err);
> +}
> +
Gerd Hoffmann - Jan. 10, 2013, 10:53 a.m.
On 01/10/13 11:33, Paolo Bonzini wrote:
> Il 07/01/2013 14:55, Gerd Hoffmann ha scritto:
>> +void hmp_chardev_add(Monitor *mon, const QDict *qdict)
>> +{
>> +    const char *args = qdict_get_str(qdict, "args");
>> +    Error *err = NULL;
>> +    QemuOpts *opts;
>> +
>> +    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
>> +    if (opts == NULL) {
>> +        error_setg(&err, "Parsing chardev args failed\n");
>> +    } else {
>> +        qemu_chr_new_from_opts(opts, NULL, &err);
> 
> This ought to use qmp_chardev_add and a generic opts->ChardevBackend
> conversion.
> 
> But IMHO, this kind of intermediate conversion is okay, with the
> "correct" thing deferred; being able to play with hotplug from HMP is
> worth the small wart.  It's really Luiz's decision, so I'm not giving
> the reviewed-by (yet).

Once qmp_chardev_add() can handle everything supported by
qemu_chr_new_from_opts we can flip over, make qmp_chardev_add the
primary interface and qemu_chr_new_from_opts legacy (which then does the
opts->ChardevBackend conversion and calls qmp_chardev_add).

We are not there yet, even with the full series applied.
And even when we arrive there some day we don't have to touch
hmp_chardev_add when making the switch ;)

cheers,
  Gerd
Paolo Bonzini - Jan. 10, 2013, 10:57 a.m.
Il 10/01/2013 11:53, Gerd Hoffmann ha scritto:
>> > 
>> > This ought to use qmp_chardev_add and a generic opts->ChardevBackend
>> > conversion.
>> > 
>> > But IMHO, this kind of intermediate conversion is okay, with the
>> > "correct" thing deferred; being able to play with hotplug from HMP is
>> > worth the small wart.  It's really Luiz's decision, so I'm not giving
>> > the reviewed-by (yet).
> Once qmp_chardev_add() can handle everything supported by
> qemu_chr_new_from_opts we can flip over, make qmp_chardev_add the
> primary interface and qemu_chr_new_from_opts legacy (which then does the
> opts->ChardevBackend conversion and calls qmp_chardev_add).
> 
> We are not there yet, even with the full series applied.

Yup.  Strange ones like msmouse, braille, etc. are missing.

> And even when we arrive there some day we don't have to touch
> hmp_chardev_add when making the switch ;)

Indeed.

Paolo
Luiz Capitulino - Jan. 10, 2013, 12:35 p.m.
On Thu, 10 Jan 2013 11:33:35 +0100
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Il 07/01/2013 14:55, Gerd Hoffmann ha scritto:
> > +void hmp_chardev_add(Monitor *mon, const QDict *qdict)
> > +{
> > +    const char *args = qdict_get_str(qdict, "args");
> > +    Error *err = NULL;
> > +    QemuOpts *opts;
> > +
> > +    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
> > +    if (opts == NULL) {
> > +        error_setg(&err, "Parsing chardev args failed\n");
> > +    } else {
> > +        qemu_chr_new_from_opts(opts, NULL, &err);
> 
> This ought to use qmp_chardev_add and a generic opts->ChardevBackend
> conversion.
> 
> But IMHO, this kind of intermediate conversion is okay, with the
> "correct" thing deferred; being able to play with hotplug from HMP is
> worth the small wart.  It's really Luiz's decision, so I'm not giving
> the reviewed-by (yet).

Fine with me. But please, add a comment if you respin.

Patch

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 010b8c9..67569ef 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1485,6 +1485,38 @@  passed since 1970, i.e. unix epoch.
 ETEXI
 
     {
+        .name       = "chardev-add",
+        .args_type  = "args:s",
+        .params     = "args",
+        .help       = "add chardev",
+        .mhandler.cmd = hmp_chardev_add,
+    },
+
+STEXI
+@item chardev_add args
+@findex chardev_add
+
+chardev_add accepts the same parameters as the -chardev command line switch.
+
+ETEXI
+
+    {
+        .name       = "chardev-remove",
+        .args_type  = "id:s",
+        .params     = "id",
+        .help       = "remove chardev",
+        .mhandler.cmd = hmp_chardev_remove,
+    },
+
+STEXI
+@item chardev_remove id
+@findex chardev_remove
+
+Removes the chardev @var{id}.
+
+ETEXI
+
+    {
         .name       = "info",
         .args_type  = "item:s?",
         .params     = "[subcommand]",
diff --git a/hmp.c b/hmp.c
index 9e9e624..68929b4 100644
--- a/hmp.c
+++ b/hmp.c
@@ -1336,3 +1336,26 @@  void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
     qmp_nbd_server_stop(&errp);
     hmp_handle_error(mon, &errp);
 }
+
+void hmp_chardev_add(Monitor *mon, const QDict *qdict)
+{
+    const char *args = qdict_get_str(qdict, "args");
+    Error *err = NULL;
+    QemuOpts *opts;
+
+    opts = qemu_opts_parse(qemu_find_opts("chardev"), args, 1);
+    if (opts == NULL) {
+        error_setg(&err, "Parsing chardev args failed\n");
+    } else {
+        qemu_chr_new_from_opts(opts, NULL, &err);
+    }
+    hmp_handle_error(mon, &err);
+}
+
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
+{
+    Error *local_err = NULL;
+
+    qmp_chardev_remove(qdict_get_str(qdict, "id"), &local_err);
+    hmp_handle_error(mon, &local_err);
+}
diff --git a/hmp.h b/hmp.h
index 21f3e05..700fbdc 100644
--- a/hmp.h
+++ b/hmp.h
@@ -80,5 +80,7 @@  void hmp_screen_dump(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
+void hmp_chardev_add(Monitor *mon, const QDict *qdict);
+void hmp_chardev_remove(Monitor *mon, const QDict *qdict);
 
 #endif