* [Qemu-devel] [PATCH RESENT 0/3] chardev hotplug patch series
@ 2012-12-14 9:38 Gerd Hoffmann
2012-12-14 9:38 ` [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Gerd Hoffmann @ 2012-12-14 9:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Gerd Hoffmann
Hi,
Got stuck in discussions & 1.3 freeze. Resending series to resume merge
effort, almost unmodified, only rebased & trivial conflicts resolved.
please review & comment,
Gerd
Gerd Hoffmann (3):
chardev: add error reporting for qemu_chr_new_from_opts
chardev: fix QemuOpts lifecycle
chardev: add hotplug support.
hmp-commands.hx | 44 +++++++++++++++++++++++++++++
hmp.c | 29 +++++++++++++++++++
hmp.h | 3 ++
qapi-schema.json | 43 +++++++++++++++++++++++++++++
qemu-char.c | 80 ++++++++++++++++++++++++++++++++++++++++++++---------
qemu-char.h | 6 +++-
qmp-commands.hx | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++
vl.c | 9 ++++--
8 files changed, 272 insertions(+), 18 deletions(-)
^ permalink raw reply [flat|nested] 13+ messages in thread* [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts 2012-12-14 9:38 [Qemu-devel] [PATCH RESENT 0/3] chardev hotplug patch series Gerd Hoffmann @ 2012-12-14 9:38 ` Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 2/3] chardev: fix QemuOpts lifecycle Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 3/3] chardev: add hotplug support Gerd Hoffmann 2 siblings, 0 replies; 13+ messages in thread From: Gerd Hoffmann @ 2012-12-14 9:38 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- qemu-char.c | 24 +++++++++++++++--------- qemu-char.h | 3 ++- vl.c | 9 ++++++--- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 242b799..5b91228 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2766,19 +2766,20 @@ static const struct { }; CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, - void (*init)(struct CharDriverState *s)) + void (*init)(struct CharDriverState *s), + Error **errp) { CharDriverState *chr; int i; if (qemu_opts_id(opts) == NULL) { - fprintf(stderr, "chardev: no id specified\n"); + error_setg(errp, "chardev: no id specified\n"); return NULL; } if (qemu_opt_get(opts, "backend") == NULL) { - fprintf(stderr, "chardev: \"%s\" missing backend\n", - qemu_opts_id(opts)); + error_setg(errp, "chardev: \"%s\" missing backend\n", + qemu_opts_id(opts)); return NULL; } for (i = 0; i < ARRAY_SIZE(backend_table); i++) { @@ -2786,15 +2787,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, break; } if (i == ARRAY_SIZE(backend_table)) { - fprintf(stderr, "chardev: backend \"%s\" not found\n", - qemu_opt_get(opts, "backend")); + error_setg(errp, "chardev: backend \"%s\" not found\n", + qemu_opt_get(opts, "backend")); return NULL; } chr = backend_table[i].open(opts); if (!chr) { - fprintf(stderr, "chardev: opening backend \"%s\" failed\n", - qemu_opt_get(opts, "backend")); + error_setg(errp, "chardev: opening backend \"%s\" failed\n", + qemu_opt_get(opts, "backend")); return NULL; } @@ -2824,6 +2825,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in const char *p; CharDriverState *chr; QemuOpts *opts; + Error *err = NULL; if (strstart(filename, "chardev:", &p)) { return qemu_chr_find(p); @@ -2833,7 +2835,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in if (!opts) return NULL; - chr = qemu_chr_new_from_opts(opts, init); + chr = qemu_chr_new_from_opts(opts, init, &err); + if (error_is_set(&err)) { + fprintf(stderr, "%s\n", error_get_pretty(err)); + error_free(err); + } if (chr && qemu_opt_get_bool(opts, "mux", 0)) { monitor_init(chr, MONITOR_USE_READLINE); } diff --git a/qemu-char.h b/qemu-char.h index a121e04..d7eed34 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -89,7 +89,8 @@ struct CharDriverState { * Returns: a new character backend */ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, - void (*init)(struct CharDriverState *s)); + void (*init)(struct CharDriverState *s), + Error **errp); /** * @qemu_chr_new: diff --git a/vl.c b/vl.c index a3ab384..353817a 100644 --- a/vl.c +++ b/vl.c @@ -2057,11 +2057,14 @@ static int device_init_func(QemuOpts *opts, void *opaque) static int chardev_init_func(QemuOpts *opts, void *opaque) { - CharDriverState *chr; + Error *local_err = NULL; - chr = qemu_chr_new_from_opts(opts, NULL); - if (!chr) + qemu_chr_new_from_opts(opts, NULL, &local_err); + if (error_is_set(&local_err)) { + fprintf(stderr, "%s\n", error_get_pretty(local_err)); + error_free(local_err); return -1; + } return 0; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/3] chardev: fix QemuOpts lifecycle 2012-12-14 9:38 [Qemu-devel] [PATCH RESENT 0/3] chardev hotplug patch series Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann @ 2012-12-14 9:38 ` Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 3/3] chardev: add hotplug support Gerd Hoffmann 2 siblings, 0 replies; 13+ messages in thread From: Gerd Hoffmann @ 2012-12-14 9:38 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann qemu_chr_new_from_opts handles QemuOpts release now, so callers don't have to worry. It will either be saved in CharDriverState, then released in qemu_chr_delete, or in the error case released instantly. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- qemu-char.c | 15 ++++++++++----- qemu-char.h | 1 + 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index 5b91228..876714f 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2774,13 +2774,13 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, if (qemu_opts_id(opts) == NULL) { error_setg(errp, "chardev: no id specified\n"); - return NULL; + goto err; } if (qemu_opt_get(opts, "backend") == NULL) { error_setg(errp, "chardev: \"%s\" missing backend\n", qemu_opts_id(opts)); - return NULL; + goto err; } for (i = 0; i < ARRAY_SIZE(backend_table); i++) { if (strcmp(backend_table[i].name, qemu_opt_get(opts, "backend")) == 0) @@ -2789,14 +2789,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, if (i == ARRAY_SIZE(backend_table)) { error_setg(errp, "chardev: backend \"%s\" not found\n", qemu_opt_get(opts, "backend")); - return NULL; + goto err; } chr = backend_table[i].open(opts); if (!chr) { error_setg(errp, "chardev: opening backend \"%s\" failed\n", qemu_opt_get(opts, "backend")); - return NULL; + goto err; } if (!chr->filename) @@ -2817,7 +2817,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, chr->avail_connections = 1; } chr->label = g_strdup(qemu_opts_id(opts)); + chr->opts = opts; return chr; + +err: + qemu_opts_del(opts); + return NULL; } CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s)) @@ -2843,7 +2848,6 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in if (chr && qemu_opt_get_bool(opts, "mux", 0)) { monitor_init(chr, MONITOR_USE_READLINE); } - qemu_opts_del(opts); return chr; } @@ -2875,6 +2879,7 @@ void qemu_chr_delete(CharDriverState *chr) chr->chr_close(chr); g_free(chr->filename); g_free(chr->label); + qemu_opts_del(chr->opts); g_free(chr); } diff --git a/qemu-char.h b/qemu-char.h index d7eed34..f984071 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -75,6 +75,7 @@ struct CharDriverState { char *filename; int opened; int avail_connections; + QemuOpts *opts; QTAILQ_ENTRY(CharDriverState) next; }; -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 9:38 [Qemu-devel] [PATCH RESENT 0/3] chardev hotplug patch series Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 2/3] chardev: fix QemuOpts lifecycle Gerd Hoffmann @ 2012-12-14 9:38 ` Gerd Hoffmann 2012-12-14 12:17 ` Paolo Bonzini 2 siblings, 1 reply; 13+ messages in thread From: Gerd Hoffmann @ 2012-12-14 9:38 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann This patch adds chardev_add_file, chardev_add_tty and chardev_remove monitor commands. chardev_add_file and chardev_add_tty expect an id and a path, they create a file/tty chardev. chardev_del just takes an id argument and zaps the chardev specified. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hmp-commands.hx | 44 +++++++++++++++++++++++++++++++ hmp.c | 29 ++++++++++++++++++++ hmp.h | 3 ++ qapi-schema.json | 43 ++++++++++++++++++++++++++++++ qemu-char.c | 41 +++++++++++++++++++++++++++++ qemu-char.h | 2 + qmp-commands.hx | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 238 insertions(+), 0 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 010b8c9..82a855a 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1485,6 +1485,50 @@ passed since 1970, i.e. unix epoch. ETEXI { + .name = "chardev_add_file", + .args_type = "id:s,path:s", + .params = "id path ", + .help = "add file chardev", + .mhandler.cmd = hmp_chardev_add_file, + }, + +STEXI +@item chardev_add_file id path +@findex chardev_add_file + +ETEXI + + { + .name = "chardev_add_tty", + .args_type = "id:s,path:s", + .params = "id path ", + .help = "add tty chardev", + .mhandler.cmd = hmp_chardev_add_tty, + }, + +STEXI +@item chardev_add_tty id path +@findex chardev_add_tty + +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 180ba2b..8780e7c 100644 --- a/hmp.c +++ b/hmp.c @@ -1335,3 +1335,32 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict) qmp_nbd_server_stop(&errp); hmp_handle_error(mon, &errp); } + +static void hmp_chardev_add_path(Monitor *mon, const QDict *qdict, + const char *backend) +{ + const char *id = qdict_get_str(qdict, "args"); + const char *path = qdict_get_str(qdict, "path"); + Error *local_err = NULL; + + qmp_chardev_add_path(id, path, backend, &local_err); + hmp_handle_error(mon, &local_err); +} + +void hmp_chardev_add_file(Monitor *mon, const QDict *qdict) +{ + hmp_chardev_add_path(mon, qdict, "file"); +} + +void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict) +{ + hmp_chardev_add_path(mon, qdict, "tty"); +} + +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 0ab03be..8cd50d1 100644 --- a/hmp.h +++ b/hmp.h @@ -80,5 +80,8 @@ 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_file(Monitor *mon, const QDict *qdict); +void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict); +void hmp_chardev_remove(Monitor *mon, const QDict *qdict); #endif diff --git a/qapi-schema.json b/qapi-schema.json index 5dfa052..34c0e58 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3017,3 +3017,46 @@ # Since: 1.3.0 ## { 'command': 'nbd-server-stop' } + +## +# @chardev-add-file: +# +# Add a file chardev +# +# @id: the chardev's ID, must be unique +# @path: file path +# +# Returns: Nothing on success +# +# Since: 1.3.0 +## +{ 'command': 'chardev-add-file', 'data': {'id' : 'str', + 'path' : 'str' } } + +## +# @chardev-add-tty: +# +# Add a terminal chardev +# +# @id: the chardev's ID, must be unique +# @path: device path +# +# Returns: Nothing on success +# +# Since: 1.3.0 +## +{ 'command': 'chardev-add-tty', 'data': {'id' : 'str', + 'path' : 'str' } } + +## +# @chardev-remove: +# +# Remove a chardev +# +# @id: the chardev's ID, must exist and not be in use +# +# Returns: Nothing on success +# +# Since: 1.3.0 +## +{ 'command': 'chardev-remove', 'data': {'id': 'str'} } diff --git a/qemu-char.c b/qemu-char.c index 876714f..169743b 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2922,3 +2922,44 @@ CharDriverState *qemu_char_get_next_serial(void) return serial_hds[next_serial++]; } +void qmp_chardev_add_path(const char *id, const char *path, + const char *backend, Error **errp) +{ + QemuOpts *opts; + + opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp); + if (error_is_set(errp)) { + return; + } + + qemu_opt_set(opts, "path", path); + qemu_opt_set(opts, "backend", backend); + qemu_chr_new_from_opts(opts, NULL, errp); +} + +void qmp_chardev_add_file(const char *id, const char *path, Error **errp) +{ + qmp_chardev_add_path(id, path, "file", errp); +} + +void qmp_chardev_add_tty(const char *id, const char *path, Error **errp) +{ + qmp_chardev_add_path(id, path, "tty", errp); +} + +void qmp_chardev_remove(const char *id, Error **errp) +{ + CharDriverState *chr; + + chr = qemu_chr_find(id); + if (NULL == chr) { + error_setg(errp, "Chardev '%s' not found\n", id); + return; + } + if (chr->chr_can_read || chr->chr_read || + chr->chr_event || chr->handler_opaque) { + error_setg(errp, "Chardev '%s' is busy\n", id); + return; + } + qemu_chr_delete(chr); +} diff --git a/qemu-char.h b/qemu-char.h index f984071..18b9e99 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -239,6 +239,8 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data); CharDriverState *qemu_chr_find(const char *name); QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename); +void qmp_chardev_add_path(const char *id, const char *path, + const char *backend, Error **errp); /* add an eventfd to the qemu devices that are polled */ CharDriverState *qemu_chr_open_eventfd(int eventfd); diff --git a/qmp-commands.hx b/qmp-commands.hx index 5c692d0..5ff8cd1 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2654,3 +2654,79 @@ EQMP .args_type = "", .mhandler.cmd_new = qmp_marshal_input_query_target, }, + + { + .name = "chardev-add-file", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_chardev_add_file, + }, + +SQMP +chardev-add-file +---------------- + +Add a file chardev. + +Arguments: + +- "id": the chardev's ID, must be unique (json-string) +- "path": file path (json-string) + +Example: + +-> { "execute" : "chardev-add-file", + "arguments" : { "id" : "foo", + "path" : "/tmp/foo" } } +<- { "return": {} } + +EQMP + + { + .name = "chardev-add-tty", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_chardev_add_tty, + }, + +SQMP +chardev-add-tty +--------------- + +Add a terminal chardev. + +Arguments: + +- "id": the chardev's ID, must be unique (json-string) +- "path": device path (json-string) + +Example: + +-> { "execute" : "chardev-add-tty", + "arguments" : { "id" : "serial", + "path" : "/dev/ttyS0" } } +<- { "return": {} } + +EQMP + + { + .name = "chardev-remove", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_chardev_remove, + }, + + +SQMP +chardev-remove +-------------- + +Remove a chardev. + +Arguments: + +- "id": the chardev's ID, must exist and not be in use (json-string) + +Example: + +-> { "execute": "chardev-remove", "arguments": { "id" : "foo" } } +<- { "return": {} } + +EQMP -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 9:38 ` [Qemu-devel] [PATCH 3/3] chardev: add hotplug support Gerd Hoffmann @ 2012-12-14 12:17 ` Paolo Bonzini 2012-12-14 13:05 ` Eric Blake 2012-12-14 13:18 ` Gerd Hoffmann 0 siblings, 2 replies; 13+ messages in thread From: Paolo Bonzini @ 2012-12-14 12:17 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel Il 14/12/2012 10:38, Gerd Hoffmann ha scritto: > This patch adds chardev_add_file, chardev_add_tty and chardev_remove > monitor commands. > > chardev_add_file and chardev_add_tty expect an id and a path, they > create a file/tty chardev. I'd rather avoid introducing this interface. Using multiple commands is different from all previous examples, both HMP and QMP (including recent ones such as the NBD server). It is also hard to extend, for example file descriptor passing is hard to retrofit. Perhaps you can define a QAPI union and slowly build it up? Something that ultimately can become this: { 'enum': 'ChardevFileMode', 'data': # pty = console under Windows # serial = tty under POSIX [ 'file', 'pipe', 'parport', 'pty', 'serial' ] } { 'enum: 'ChardevFileSource', 'data': [ 'path', 'fd' ] } { 'type': 'ChardevFile', 'data': {'source': 'string', 'source-type': 'ChardevFileSource', 'mode': 'ChardevFileMode'}} { 'type': 'ChardevVC', 'data': {'width': 'int', 'height': 'int', '*characters': 'bool'}} { 'type': 'ChardevSocket', 'data': {'addr': 'SocketAddress', '*server': 'bool', '*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} } # For future extensibility... { 'ChardevDummy', 'data': {} } { 'union': 'ChardevBackend', 'data': { 'socket': 'ChardevSocket', 'udp': 'UDPSocketAddress', 'file': 'ChardevFile', 'null': 'ChardevDummy', 'msmouse': 'ChardevDummy', 'braille': 'ChardevDummy', 'stdio': 'ChardevDummy', 'vc': 'ChardevVC', # Solely for HMP usage. 'legacy': 'str' } { 'command': 'chardev-add', 'data': { 'backend': 'ChardevBackend', 'id': 'str', '*mux': 'bool' } } A simple conversion from this to QemuOpts should be easy, later the backends can move to taking a ChardevBackend *. Paolo > chardev_del just takes an id argument and zaps the chardev specified. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hmp-commands.hx | 44 +++++++++++++++++++++++++++++++ > hmp.c | 29 ++++++++++++++++++++ > hmp.h | 3 ++ > qapi-schema.json | 43 ++++++++++++++++++++++++++++++ > qemu-char.c | 41 +++++++++++++++++++++++++++++ > qemu-char.h | 2 + > qmp-commands.hx | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 7 files changed, 238 insertions(+), 0 deletions(-) > > diff --git a/hmp-commands.hx b/hmp-commands.hx > index 010b8c9..82a855a 100644 > --- a/hmp-commands.hx > +++ b/hmp-commands.hx > @@ -1485,6 +1485,50 @@ passed since 1970, i.e. unix epoch. > ETEXI > > { > + .name = "chardev_add_file", > + .args_type = "id:s,path:s", > + .params = "id path ", > + .help = "add file chardev", > + .mhandler.cmd = hmp_chardev_add_file, > + }, > + > +STEXI > +@item chardev_add_file id path > +@findex chardev_add_file > + > +ETEXI > + > + { > + .name = "chardev_add_tty", > + .args_type = "id:s,path:s", > + .params = "id path ", > + .help = "add tty chardev", > + .mhandler.cmd = hmp_chardev_add_tty, > + }, > + > +STEXI > +@item chardev_add_tty id path > +@findex chardev_add_tty > + > +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 180ba2b..8780e7c 100644 > --- a/hmp.c > +++ b/hmp.c > @@ -1335,3 +1335,32 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict) > qmp_nbd_server_stop(&errp); > hmp_handle_error(mon, &errp); > } > + > +static void hmp_chardev_add_path(Monitor *mon, const QDict *qdict, > + const char *backend) > +{ > + const char *id = qdict_get_str(qdict, "args"); > + const char *path = qdict_get_str(qdict, "path"); > + Error *local_err = NULL; > + > + qmp_chardev_add_path(id, path, backend, &local_err); > + hmp_handle_error(mon, &local_err); > +} > + > +void hmp_chardev_add_file(Monitor *mon, const QDict *qdict) > +{ > + hmp_chardev_add_path(mon, qdict, "file"); > +} > + > +void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict) > +{ > + hmp_chardev_add_path(mon, qdict, "tty"); > +} > + > +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 0ab03be..8cd50d1 100644 > --- a/hmp.h > +++ b/hmp.h > @@ -80,5 +80,8 @@ 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_file(Monitor *mon, const QDict *qdict); > +void hmp_chardev_add_tty(Monitor *mon, const QDict *qdict); > +void hmp_chardev_remove(Monitor *mon, const QDict *qdict); > > #endif > diff --git a/qapi-schema.json b/qapi-schema.json > index 5dfa052..34c0e58 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3017,3 +3017,46 @@ > # Since: 1.3.0 > ## > { 'command': 'nbd-server-stop' } > + > +## > +# @chardev-add-file: > +# > +# Add a file chardev > +# > +# @id: the chardev's ID, must be unique > +# @path: file path > +# > +# Returns: Nothing on success > +# > +# Since: 1.3.0 > +## > +{ 'command': 'chardev-add-file', 'data': {'id' : 'str', > + 'path' : 'str' } } > + > +## > +# @chardev-add-tty: > +# > +# Add a terminal chardev > +# > +# @id: the chardev's ID, must be unique > +# @path: device path > +# > +# Returns: Nothing on success > +# > +# Since: 1.3.0 > +## > +{ 'command': 'chardev-add-tty', 'data': {'id' : 'str', > + 'path' : 'str' } } > + > +## > +# @chardev-remove: > +# > +# Remove a chardev > +# > +# @id: the chardev's ID, must exist and not be in use > +# > +# Returns: Nothing on success > +# > +# Since: 1.3.0 > +## > +{ 'command': 'chardev-remove', 'data': {'id': 'str'} } > diff --git a/qemu-char.c b/qemu-char.c > index 876714f..169743b 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2922,3 +2922,44 @@ CharDriverState *qemu_char_get_next_serial(void) > return serial_hds[next_serial++]; > } > > +void qmp_chardev_add_path(const char *id, const char *path, > + const char *backend, Error **errp) > +{ > + QemuOpts *opts; > + > + opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp); > + if (error_is_set(errp)) { > + return; > + } > + > + qemu_opt_set(opts, "path", path); > + qemu_opt_set(opts, "backend", backend); > + qemu_chr_new_from_opts(opts, NULL, errp); > +} > + > +void qmp_chardev_add_file(const char *id, const char *path, Error **errp) > +{ > + qmp_chardev_add_path(id, path, "file", errp); > +} > + > +void qmp_chardev_add_tty(const char *id, const char *path, Error **errp) > +{ > + qmp_chardev_add_path(id, path, "tty", errp); > +} > + > +void qmp_chardev_remove(const char *id, Error **errp) > +{ > + CharDriverState *chr; > + > + chr = qemu_chr_find(id); > + if (NULL == chr) { > + error_setg(errp, "Chardev '%s' not found\n", id); > + return; > + } > + if (chr->chr_can_read || chr->chr_read || > + chr->chr_event || chr->handler_opaque) { > + error_setg(errp, "Chardev '%s' is busy\n", id); > + return; > + } > + qemu_chr_delete(chr); > +} > diff --git a/qemu-char.h b/qemu-char.h > index f984071..18b9e99 100644 > --- a/qemu-char.h > +++ b/qemu-char.h > @@ -239,6 +239,8 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data); > CharDriverState *qemu_chr_find(const char *name); > > QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename); > +void qmp_chardev_add_path(const char *id, const char *path, > + const char *backend, Error **errp); > > /* add an eventfd to the qemu devices that are polled */ > CharDriverState *qemu_chr_open_eventfd(int eventfd); > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 5c692d0..5ff8cd1 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2654,3 +2654,79 @@ EQMP > .args_type = "", > .mhandler.cmd_new = qmp_marshal_input_query_target, > }, > + > + { > + .name = "chardev-add-file", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_chardev_add_file, > + }, > + > +SQMP > +chardev-add-file > +---------------- > + > +Add a file chardev. > + > +Arguments: > + > +- "id": the chardev's ID, must be unique (json-string) > +- "path": file path (json-string) > + > +Example: > + > +-> { "execute" : "chardev-add-file", > + "arguments" : { "id" : "foo", > + "path" : "/tmp/foo" } } > +<- { "return": {} } > + > +EQMP > + > + { > + .name = "chardev-add-tty", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_chardev_add_tty, > + }, > + > +SQMP > +chardev-add-tty > +--------------- > + > +Add a terminal chardev. > + > +Arguments: > + > +- "id": the chardev's ID, must be unique (json-string) > +- "path": device path (json-string) > + > +Example: > + > +-> { "execute" : "chardev-add-tty", > + "arguments" : { "id" : "serial", > + "path" : "/dev/ttyS0" } } > +<- { "return": {} } > + > +EQMP > + > + { > + .name = "chardev-remove", > + .args_type = "", > + .mhandler.cmd_new = qmp_marshal_input_chardev_remove, > + }, > + > + > +SQMP > +chardev-remove > +-------------- > + > +Remove a chardev. > + > +Arguments: > + > +- "id": the chardev's ID, must exist and not be in use (json-string) > + > +Example: > + > +-> { "execute": "chardev-remove", "arguments": { "id" : "foo" } } > +<- { "return": {} } > + > +EQMP > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 12:17 ` Paolo Bonzini @ 2012-12-14 13:05 ` Eric Blake 2012-12-14 13:18 ` Gerd Hoffmann 1 sibling, 0 replies; 13+ messages in thread From: Eric Blake @ 2012-12-14 13:05 UTC (permalink / raw) To: Paolo Bonzini; +Cc: Gerd Hoffmann, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2281 bytes --] On 12/14/2012 05:17 AM, Paolo Bonzini wrote: > Il 14/12/2012 10:38, Gerd Hoffmann ha scritto: >> This patch adds chardev_add_file, chardev_add_tty and chardev_remove >> monitor commands. >> >> chardev_add_file and chardev_add_tty expect an id and a path, they >> create a file/tty chardev. > > I'd rather avoid introducing this interface. Using multiple commands is > different from all previous examples, both HMP and QMP (including recent > ones such as the NBD server). It is also hard to extend, for example > file descriptor passing is hard to retrofit. File descriptor passing via magic /dev/fdset/nnn should probably already work. That said, a single command that uses a QAPI union, rather than one command per source type, would be nicer from the UI perspective, and it is the QMP UI perspective that libvirt is concerned about. > > Perhaps you can define a QAPI union and slowly build it up? Something > that ultimately can become this: > > { 'enum': 'ChardevFileMode', 'data': > # pty = console under Windows > # serial = tty under POSIX > [ 'file', 'pipe', 'parport', 'pty', 'serial' ] } > > { 'enum: 'ChardevFileSource', 'data': > [ 'path', 'fd' ] } > > { 'type': 'ChardevFile', > 'data': {'source': 'string', 'source-type': 'ChardevFileSource', > 'mode': 'ChardevFileMode'}} > > { 'type': 'ChardevVC', > 'data': {'width': 'int', 'height': 'int', '*characters': 'bool'}} > > { 'type': 'ChardevSocket', > 'data': {'addr': 'SocketAddress', '*server': 'bool', > '*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} } > > # For future extensibility... > { 'ChardevDummy', 'data': {} } > > { 'union': 'ChardevBackend', 'data': { > 'socket': 'ChardevSocket', > 'udp': 'UDPSocketAddress', > 'file': 'ChardevFile', > 'null': 'ChardevDummy', > 'msmouse': 'ChardevDummy', > 'braille': 'ChardevDummy', > 'stdio': 'ChardevDummy', > 'vc': 'ChardevVC', > > # Solely for HMP usage. > 'legacy': 'str' > } > > { 'command': 'chardev-add', 'data': { > 'backend': 'ChardevBackend', 'id': 'str', '*mux': 'bool' } } Yes, this looks nicer. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 619 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 12:17 ` Paolo Bonzini 2012-12-14 13:05 ` Eric Blake @ 2012-12-14 13:18 ` Gerd Hoffmann 2012-12-14 13:45 ` Paolo Bonzini 1 sibling, 1 reply; 13+ messages in thread From: Gerd Hoffmann @ 2012-12-14 13:18 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 840 bytes --] Hi, > { 'enum': 'ChardevFileMode', 'data': > # pty = console under Windows > # serial = tty under POSIX > [ 'file', 'pipe', 'parport', 'pty', 'serial' ] } Hmm, why this enum? I'd stay close to -chardev, i.e. specify the type by backend name. > { 'enum: 'ChardevFileSource', 'data': > [ 'path', 'fd' ] } I guess I'd just create a new backend type for file descriptor passing instead of fitting that into all the existing ones. > { 'union': 'ChardevBackend', 'data': { This union thing is new, isn't it? Makes sense to use that indeed. > 'socket': 'ChardevSocket', > 'udp': 'UDPSocketAddress', > 'file': 'ChardevFile', > 'null': 'ChardevDummy', > 'msmouse': 'ChardevDummy', > 'braille': 'ChardevDummy', > 'stdio': 'ChardevDummy', > 'vc': 'ChardevVC', I doubt we need them all hotpluggable. cheers, Gerd [-- Attachment #2: 0001-chardev-add-hotplug-support.patch --] [-- Type: text/plain, Size: 7216 bytes --] >From 6ea61630245d8ff9f87ed56b825dcc5f8d1f6e6d Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann <kraxel@redhat.com> Date: Thu, 11 Oct 2012 14:53:00 +0200 Subject: [PATCH] chardev: add hotplug support. This patch adds chardev_add and chardev_remove monitor commands. chardev_add expects a backend struct filled in and creates the chardev from that. For now only file and tty backends are supported. chardev_del just takes an id argument and zaps the chardev specified. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hmp-commands.hx | 30 ++++++++++++++++++++++++++++++ hmp.c | 19 +++++++++++++++++++ hmp.h | 2 ++ qapi-schema.json | 31 +++++++++++++++++++++++++++++++ qemu-char.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ qemu-char.h | 2 ++ qmp-commands.hx | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 7 files changed, 180 insertions(+), 0 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 010b8c9..9a0b2eb 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1485,6 +1485,36 @@ passed since 1970, i.e. unix epoch. ETEXI { + .name = "chardev_add", + .args_type = "id:s,backend:s,arg1:s", + .params = "id backend arg", + .help = "add chardev", + .mhandler.cmd = hmp_chardev_add, + }, + +STEXI +@item chardev_add id backend arg +@findex chardev_add + +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 180ba2b..c65f4f7 100644 --- a/hmp.c +++ b/hmp.c @@ -1335,3 +1335,22 @@ 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 *id = qdict_get_str(qdict, "id"); + const char *backend = qdict_get_str(qdict, "backend"); + const char *path = qdict_get_str(qdict, "arg1"); + Error *local_err = NULL; + + qmp_chardev_add_path(id, path, backend, &local_err); + hmp_handle_error(mon, &local_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 0ab03be..e67e482 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 diff --git a/qapi-schema.json b/qapi-schema.json index 5dfa052..7349757 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3017,3 +3017,34 @@ # Since: 1.3.0 ## { 'command': 'nbd-server-stop' } + +## +# @chardev-add: +# +# Add a file chardev +# +# @id: the chardev's ID, must be unique +# @backend: backend type and parameters +# +# Returns: Nothing on success +# +# Since: 1.3.0 +## +{ 'type': 'ChardevFile', 'data': { 'path': 'str' } } +{ 'union': 'ChardevBackend', 'data': { 'file': 'ChardevFile', + 'tty': 'ChardevFile' } } +{ 'command': 'chardev-add', 'data': {'id' : 'str', + 'backend' : 'ChardevBackend' } } + +## +# @chardev-remove: +# +# Remove a chardev +# +# @id: the chardev's ID, must exist and not be in use +# +# Returns: Nothing on success +# +# Since: 1.3.0 +## +{ 'command': 'chardev-remove', 'data': {'id': 'str'} } diff --git a/qemu-char.c b/qemu-char.c index 876714f..bf7fdb6 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2922,3 +2922,49 @@ CharDriverState *qemu_char_get_next_serial(void) return serial_hds[next_serial++]; } +void qmp_chardev_add_path(const char *id, const char *path, + const char *backend, Error **errp) +{ + QemuOpts *opts; + + opts = qemu_opts_create(qemu_find_opts("chardev"), id, 1, errp); + if (error_is_set(errp)) { + return; + } + + qemu_opt_set(opts, "path", path); + qemu_opt_set(opts, "backend", backend); + qemu_chr_new_from_opts(opts, NULL, errp); +} + +void qmp_chardev_add(const char *id, ChardevBackend *backend, Error **errp) +{ + switch (backend->kind) { + case CHARDEV_BACKEND_KIND_FILE: + qmp_chardev_add_path(id, backend->file->path, "file", errp); + break; + case CHARDEV_BACKEND_KIND_TTY: + qmp_chardev_add_path(id, backend->tty->path, "tty", errp); + break; + case CHARDEV_BACKEND_KIND_MAX: + /* make gcc happy */ + break; + } +} + +void qmp_chardev_remove(const char *id, Error **errp) +{ + CharDriverState *chr; + + chr = qemu_chr_find(id); + if (NULL == chr) { + error_setg(errp, "Chardev '%s' not found\n", id); + return; + } + if (chr->chr_can_read || chr->chr_read || + chr->chr_event || chr->handler_opaque) { + error_setg(errp, "Chardev '%s' is busy\n", id); + return; + } + qemu_chr_delete(chr); +} diff --git a/qemu-char.h b/qemu-char.h index f984071..18b9e99 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -239,6 +239,8 @@ void qemu_chr_info(Monitor *mon, QObject **ret_data); CharDriverState *qemu_chr_find(const char *name); QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename); +void qmp_chardev_add_path(const char *id, const char *path, + const char *backend, Error **errp); /* add an eventfd to the qemu devices that are polled */ CharDriverState *qemu_chr_open_eventfd(int eventfd); diff --git a/qmp-commands.hx b/qmp-commands.hx index 5c692d0..10408e9 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2654,3 +2654,53 @@ EQMP .args_type = "", .mhandler.cmd_new = qmp_marshal_input_query_target, }, + + { + .name = "chardev-add", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_chardev_add, + }, + +SQMP +chardev-add +---------------- + +Add a chardev. + +Arguments: + +- "id": the chardev's ID, must be unique (json-string) +- "backend": chardev backend type + parameters + +Example: + +-> { "execute" : "chardev-add", + "arguments" : { "id" : "foo", + FIXME } } +<- { "return": {} } + +EQMP + + { + .name = "chardev-remove", + .args_type = "", + .mhandler.cmd_new = qmp_marshal_input_chardev_remove, + }, + + +SQMP +chardev-remove +-------------- + +Remove a chardev. + +Arguments: + +- "id": the chardev's ID, must exist and not be in use (json-string) + +Example: + +-> { "execute": "chardev-remove", "arguments": { "id" : "foo" } } +<- { "return": {} } + +EQMP -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 13:18 ` Gerd Hoffmann @ 2012-12-14 13:45 ` Paolo Bonzini 2012-12-14 14:05 ` Gerd Hoffmann 0 siblings, 1 reply; 13+ messages in thread From: Paolo Bonzini @ 2012-12-14 13:45 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel Il 14/12/2012 14:18, Gerd Hoffmann ha scritto: > Hi, > >> { 'enum': 'ChardevFileMode', 'data': >> # pty = console under Windows >> # serial = tty under POSIX >> [ 'file', 'pipe', 'parport', 'pty', 'serial' ] } > > Hmm, why this enum? I'd stay close to -chardev, i.e. specify the type > by backend name. Because... >> { 'enum: 'ChardevFileSource', 'data': >> [ 'path', 'fd' ] } > > I guess I'd just create a new backend type for file descriptor passing > instead of fitting that into all the existing ones. ... are you passing a file descriptor for a pipe, a file or a parallel/serial port? (pty and console have no arguments, I misremembered). >> { 'union': 'ChardevBackend', 'data': { > > This union thing is new, isn't it? Yeah, a few months old. > Makes sense to use that indeed. > >> 'socket': 'ChardevSocket', >> 'udp': 'UDPSocketAddress', >> 'file': 'ChardevFile', >> 'null': 'ChardevDummy', >> 'msmouse': 'ChardevDummy', >> 'braille': 'ChardevDummy', >> 'stdio': 'ChardevDummy', >> 'vc': 'ChardevVC', > > I doubt we need them all hotpluggable. True, but: 1) I believe long term it's good to move away from QemuOpts; it's good to provide a complete interface even if all we're doing for now is building QemuOpts out of the struct. 2) most of them have no options and trivial anyway; 3) the complicated ones (socket, file, perhaps udp) are also the useful ones. Paolo ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 13:45 ` Paolo Bonzini @ 2012-12-14 14:05 ` Gerd Hoffmann 2012-12-14 14:19 ` Gerd Hoffmann 0 siblings, 1 reply; 13+ messages in thread From: Gerd Hoffmann @ 2012-12-14 14:05 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel On 12/14/12 14:45, Paolo Bonzini wrote: > Il 14/12/2012 14:18, Gerd Hoffmann ha scritto: >> Hi, >> >>> { 'enum': 'ChardevFileMode', 'data': >>> # pty = console under Windows >>> # serial = tty under POSIX >>> [ 'file', 'pipe', 'parport', 'pty', 'serial' ] } >> >> Hmm, why this enum? I'd stay close to -chardev, i.e. specify the type >> by backend name. > > Because... > >>> { 'enum: 'ChardevFileSource', 'data': >>> [ 'path', 'fd' ] } >> >> I guess I'd just create a new backend type for file descriptor passing >> instead of fitting that into all the existing ones. > > ... are you passing a file descriptor for a pipe, a file or a > parallel/serial port? The open function of the file-based backends basically do (1) create file handles and (2) call qemu_chr_open_fd(). So of you already have an fd the differences are gone. Well, almost. tty has an special ioctl callback to configure line speed. cheers, Gerd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 14:05 ` Gerd Hoffmann @ 2012-12-14 14:19 ` Gerd Hoffmann 2012-12-14 15:07 ` Paolo Bonzini 0 siblings, 1 reply; 13+ messages in thread From: Gerd Hoffmann @ 2012-12-14 14:19 UTC (permalink / raw) To: Paolo Bonzini; +Cc: qemu-devel Hi, >> ... are you passing a file descriptor for a pipe, a file or a >> parallel/serial port? > > The open function of the file-based backends basically do (1) create > file handles and (2) call qemu_chr_open_fd(). So of you already have an > fd the differences are gone. Well, almost. tty has an special ioctl > callback to configure line speed. Also you might want to pass in a socket fd ... So I really think a -chardev fd,type={listening-stream-socket,connected-stream-socket,datagram-socket,tty,fd-readwrite,fd-writeonly} (+ QMP for that) will be more useful. cheers, Gerd ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 3/3] chardev: add hotplug support. 2012-12-14 14:19 ` Gerd Hoffmann @ 2012-12-14 15:07 ` Paolo Bonzini 0 siblings, 0 replies; 13+ messages in thread From: Paolo Bonzini @ 2012-12-14 15:07 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel Il 14/12/2012 15:19, Gerd Hoffmann ha scritto: > Hi, > >>> ... are you passing a file descriptor for a pipe, a file or a >>> parallel/serial port? >> >> The open function of the file-based backends basically do (1) create >> file handles and (2) call qemu_chr_open_fd(). Right, and there's the ugliness where you can only specify an output file, not an input. >> So of you already have an >> fd the differences are gone. Well, almost. tty has an special ioctl >> callback to configure line speed. What about this then: { 'union': 'ChardevFileSource', 'data': {'path': 'str', 'fd': 'str'} } { 'type': 'ChardevFile', 'data': {'*in': 'ChardevFileSource', '*out': 'ChardevFileSource'} } { 'enum': 'ChardevPortKind', 'data': ['serial', 'parallel'] } { 'type': 'ChardevPort', 'data': {'device': 'ChardevFileSource', 'type': 'ChardevPortKind'} } { 'type': 'ChardevSpice', 'data': {'name': 'str', '*debug': 'bool'}} { 'type': 'ChardevVC', 'data': {'*width': 'int', '*height': 'int', '*cols': 'int', '*rows': 'int'}} { 'type': 'ChardevSocket', 'data': {'addr': 'SocketAddress', '*server': 'bool', '*wait': 'bool', '*nodelay': 'bool', '*telnet': 'bool'} } { 'type': 'ChardevUDP', 'data': {'peer': 'SocketAddress', '*localhost': 'str', '*localport': 'str' } } # For future extensibility... { 'ChardevDummy', 'data': {} } { 'union': 'ChardevBackend', 'data': { 'socket': 'ChardevSocket', 'winpipe': 'String', # "pipe" for _WIN32 case 'udp': 'ChardevUDP', 'file': 'ChardevFile', 'port': 'ChardevPort', 'pty': 'ChardevDummy', 'null': 'ChardevDummy', 'msmouse': 'ChardevDummy', 'braille': 'ChardevDummy', 'stdio': 'ChardevDummy', 'vc': 'ChardevVC', # Solely for HMP usage. 'legacy': 'str' } > Also you might want to pass in a socket fd ... This is already supported by socket_connect/socket_listen in qemu-sockets.c (thus by the SocketAddress QAPI union). Paolo > So I really think a -chardev > fd,type={listening-stream-socket,connected-stream-socket,datagram-socket,tty,fd-readwrite,fd-writeonly} > (+ QMP for that) will be more useful. > cheers, > Gerd > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 0/3] chardev hotplug patch series @ 2012-10-17 10:09 Gerd Hoffmann 2012-10-17 10:09 ` [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann 0 siblings, 1 reply; 13+ messages in thread From: Gerd Hoffmann @ 2012-10-17 10:09 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann Hi, Here comes the updated chardev hotplug patch series. Addressed most review comments. It's chardev-add and chardev-remove now. Parameters have been added to the schema. Little nits and spell fixes here and there. Making 'backend' an enum is tricky given that the actual implementation just turns the qdict into a QemuOpts, then goes piggyback on qemu_chr_new_from_opts(). Not rebased (yet) on top of the qom queue as I expect more discussions. please review, Gerd Gerd Hoffmann (3): chardev: add error reporting for qemu_chr_new_from_opts chardev: fix QemuOpts lifecycle chardev: add hotplug support. hmp-commands.hx | 32 +++++++++++++++++++++ hmp.c | 23 +++++++++++++++ hmp.h | 2 + qapi-schema.json | 47 ++++++++++++++++++++++++++++++ qemu-char.c | 83 ++++++++++++++++++++++++++++++++++++++++++++--------- qemu-char.h | 5 ++- qmp-commands.hx | 61 +++++++++++++++++++++++++++++++++++++++ vl.c | 8 ++++- 8 files changed, 244 insertions(+), 17 deletions(-) ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts 2012-10-17 10:09 [Qemu-devel] [PATCH 0/3] chardev hotplug patch series Gerd Hoffmann @ 2012-10-17 10:09 ` Gerd Hoffmann 2012-10-17 16:22 ` Luiz Capitulino 0 siblings, 1 reply; 13+ messages in thread From: Gerd Hoffmann @ 2012-10-17 10:09 UTC (permalink / raw) To: qemu-devel; +Cc: Gerd Hoffmann Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- qemu-char.c | 24 +++++++++++++++--------- qemu-char.h | 3 ++- vl.c | 8 ++++++-- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/qemu-char.c b/qemu-char.c index b082bae..e2e1da8 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2755,19 +2755,20 @@ static const struct { }; CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, - void (*init)(struct CharDriverState *s)) + void (*init)(struct CharDriverState *s), + Error **errp) { CharDriverState *chr; int i; if (qemu_opts_id(opts) == NULL) { - fprintf(stderr, "chardev: no id specified\n"); + error_setg(errp, "chardev: no id specified\n"); return NULL; } if (qemu_opt_get(opts, "backend") == NULL) { - fprintf(stderr, "chardev: \"%s\" missing backend\n", - qemu_opts_id(opts)); + error_setg(errp, "chardev: \"%s\" missing backend\n", + qemu_opts_id(opts)); return NULL; } for (i = 0; i < ARRAY_SIZE(backend_table); i++) { @@ -2775,15 +2776,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, break; } if (i == ARRAY_SIZE(backend_table)) { - fprintf(stderr, "chardev: backend \"%s\" not found\n", - qemu_opt_get(opts, "backend")); + error_setg(errp, "chardev: backend \"%s\" not found\n", + qemu_opt_get(opts, "backend")); return NULL; } chr = backend_table[i].open(opts); if (!chr) { - fprintf(stderr, "chardev: opening backend \"%s\" failed\n", - qemu_opt_get(opts, "backend")); + error_setg(errp, "chardev: opening backend \"%s\" failed\n", + qemu_opt_get(opts, "backend")); return NULL; } @@ -2813,6 +2814,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in const char *p; CharDriverState *chr; QemuOpts *opts; + Error *err = NULL; if (strstart(filename, "chardev:", &p)) { return qemu_chr_find(p); @@ -2822,7 +2824,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in if (!opts) return NULL; - chr = qemu_chr_new_from_opts(opts, init); + chr = qemu_chr_new_from_opts(opts, init, &err); + if (error_is_set(&err)) { + fprintf(stderr, "%s\n", error_get_pretty(err)); + error_free(err); + } if (chr && qemu_opt_get_bool(opts, "mux", 0)) { monitor_init(chr, MONITOR_USE_READLINE); } diff --git a/qemu-char.h b/qemu-char.h index 486644b..adae13e 100644 --- a/qemu-char.h +++ b/qemu-char.h @@ -88,7 +88,8 @@ struct CharDriverState { * Returns: a new character backend */ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, - void (*init)(struct CharDriverState *s)); + void (*init)(struct CharDriverState *s), + Error **errp); /** * @qemu_chr_new: diff --git a/vl.c b/vl.c index 5b357a3..d2f1c10 100644 --- a/vl.c +++ b/vl.c @@ -1940,10 +1940,14 @@ static int device_init_func(QemuOpts *opts, void *opaque) static int chardev_init_func(QemuOpts *opts, void *opaque) { CharDriverState *chr; + Error *local_err = NULL; - chr = qemu_chr_new_from_opts(opts, NULL); - if (!chr) + chr = qemu_chr_new_from_opts(opts, NULL, &local_err); + if (error_is_set(&local_err)) { + fprintf(stderr, "%s\n", error_get_pretty(local_err)); + error_free(local_err); return -1; + } return 0; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts 2012-10-17 10:09 ` [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann @ 2012-10-17 16:22 ` Luiz Capitulino 0 siblings, 0 replies; 13+ messages in thread From: Luiz Capitulino @ 2012-10-17 16:22 UTC (permalink / raw) To: Gerd Hoffmann; +Cc: qemu-devel On Wed, 17 Oct 2012 12:09:53 +0200 Gerd Hoffmann <kraxel@redhat.com> wrote: > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Luiz Capitulino <lcapitulino@redhat.com> > --- > qemu-char.c | 24 +++++++++++++++--------- > qemu-char.h | 3 ++- > vl.c | 8 ++++++-- > 3 files changed, 23 insertions(+), 12 deletions(-) > > diff --git a/qemu-char.c b/qemu-char.c > index b082bae..e2e1da8 100644 > --- a/qemu-char.c > +++ b/qemu-char.c > @@ -2755,19 +2755,20 @@ static const struct { > }; > > CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, > - void (*init)(struct CharDriverState *s)) > + void (*init)(struct CharDriverState *s), > + Error **errp) > { > CharDriverState *chr; > int i; > > if (qemu_opts_id(opts) == NULL) { > - fprintf(stderr, "chardev: no id specified\n"); > + error_setg(errp, "chardev: no id specified\n"); > return NULL; > } > > if (qemu_opt_get(opts, "backend") == NULL) { > - fprintf(stderr, "chardev: \"%s\" missing backend\n", > - qemu_opts_id(opts)); > + error_setg(errp, "chardev: \"%s\" missing backend\n", > + qemu_opts_id(opts)); > return NULL; > } > for (i = 0; i < ARRAY_SIZE(backend_table); i++) { > @@ -2775,15 +2776,15 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, > break; > } > if (i == ARRAY_SIZE(backend_table)) { > - fprintf(stderr, "chardev: backend \"%s\" not found\n", > - qemu_opt_get(opts, "backend")); > + error_setg(errp, "chardev: backend \"%s\" not found\n", > + qemu_opt_get(opts, "backend")); > return NULL; > } > > chr = backend_table[i].open(opts); > if (!chr) { > - fprintf(stderr, "chardev: opening backend \"%s\" failed\n", > - qemu_opt_get(opts, "backend")); > + error_setg(errp, "chardev: opening backend \"%s\" failed\n", > + qemu_opt_get(opts, "backend")); > return NULL; > } > > @@ -2813,6 +2814,7 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in > const char *p; > CharDriverState *chr; > QemuOpts *opts; > + Error *err = NULL; > > if (strstart(filename, "chardev:", &p)) { > return qemu_chr_find(p); > @@ -2822,7 +2824,11 @@ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*in > if (!opts) > return NULL; > > - chr = qemu_chr_new_from_opts(opts, init); > + chr = qemu_chr_new_from_opts(opts, init, &err); > + if (error_is_set(&err)) { > + fprintf(stderr, "%s\n", error_get_pretty(err)); > + error_free(err); > + } > if (chr && qemu_opt_get_bool(opts, "mux", 0)) { > monitor_init(chr, MONITOR_USE_READLINE); > } > diff --git a/qemu-char.h b/qemu-char.h > index 486644b..adae13e 100644 > --- a/qemu-char.h > +++ b/qemu-char.h > @@ -88,7 +88,8 @@ struct CharDriverState { > * Returns: a new character backend > */ > CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, > - void (*init)(struct CharDriverState *s)); > + void (*init)(struct CharDriverState *s), > + Error **errp); > > /** > * @qemu_chr_new: > diff --git a/vl.c b/vl.c > index 5b357a3..d2f1c10 100644 > --- a/vl.c > +++ b/vl.c > @@ -1940,10 +1940,14 @@ static int device_init_func(QemuOpts *opts, void *opaque) > static int chardev_init_func(QemuOpts *opts, void *opaque) > { > CharDriverState *chr; > + Error *local_err = NULL; > > - chr = qemu_chr_new_from_opts(opts, NULL); > - if (!chr) > + chr = qemu_chr_new_from_opts(opts, NULL, &local_err); > + if (error_is_set(&local_err)) { > + fprintf(stderr, "%s\n", error_get_pretty(local_err)); > + error_free(local_err); > return -1; > + } > return 0; > } > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-12-14 15:07 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-12-14 9:38 [Qemu-devel] [PATCH RESENT 0/3] chardev hotplug patch series Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 2/3] chardev: fix QemuOpts lifecycle Gerd Hoffmann 2012-12-14 9:38 ` [Qemu-devel] [PATCH 3/3] chardev: add hotplug support Gerd Hoffmann 2012-12-14 12:17 ` Paolo Bonzini 2012-12-14 13:05 ` Eric Blake 2012-12-14 13:18 ` Gerd Hoffmann 2012-12-14 13:45 ` Paolo Bonzini 2012-12-14 14:05 ` Gerd Hoffmann 2012-12-14 14:19 ` Gerd Hoffmann 2012-12-14 15:07 ` Paolo Bonzini -- strict thread matches above, loose matches on Subject: below -- 2012-10-17 10:09 [Qemu-devel] [PATCH 0/3] chardev hotplug patch series Gerd Hoffmann 2012-10-17 10:09 ` [Qemu-devel] [PATCH 1/3] chardev: add error reporting for qemu_chr_new_from_opts Gerd Hoffmann 2012-10-17 16:22 ` Luiz Capitulino
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).