* [Qemu-devel] [PATCH] char: don't exit on hmp 'chardev-add help'
@ 2017-07-17 10:52 Anton Nefedov
2017-07-17 11:19 ` Marc-André Lureau
2017-07-24 16:50 ` Paolo Bonzini
0 siblings, 2 replies; 3+ messages in thread
From: Anton Nefedov @ 2017-07-17 10:52 UTC (permalink / raw)
To: qemu-devel; +Cc: den, pbonzini, marcandre.lureau, dgilbert, Anton Nefedov
qemu_chr_new_from_opts() is used from both vl.c and hmp,
and it is quite confusing to see qemu suddenly exit after receiving a help
option in hmp.
Create a separate function for printing help and let different callers
react accordingly.
Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
---
include/chardev/char.h | 9 +++++++++
chardev/char.c | 27 +++++++++++++++++++--------
vl.c | 4 ++++
3 files changed, 32 insertions(+), 8 deletions(-)
diff --git a/include/chardev/char.h b/include/chardev/char.h
index 1604ea9..461962b 100644
--- a/include/chardev/char.h
+++ b/include/chardev/char.h
@@ -59,6 +59,15 @@ struct Chardev {
};
/**
+ * @qemu_chr_help:
+ *
+ * Print available chardevice backends if 'asked for help' in @opts
+ *
+ * Returns: true if there was a help option
+ */
+bool qemu_chr_help(QemuOpts *opts);
+
+/**
* @qemu_chr_new_from_opts:
*
* Create a new character backend from a QemuOpts list.
diff --git a/chardev/char.c b/chardev/char.c
index c34b44a..19833fe 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -604,14 +604,9 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, Error **errp)
return backend;
}
-Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
+bool qemu_chr_help(QemuOpts *opts)
{
- const ChardevClass *cc;
- Chardev *chr = NULL;
- ChardevBackend *backend = NULL;
- const char *name = chardev_alias_translate(qemu_opt_get(opts, "backend"));
- const char *id = qemu_opts_id(opts);
- char *bid = NULL;
+ const char *name = qemu_opt_get(opts, "backend");
if (name && is_help_option(name)) {
GString *str = g_string_new("");
@@ -620,7 +615,23 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
error_report("Available chardev backend types: %s", str->str);
g_string_free(str, true);
- exit(0);
+ return true;
+ }
+
+ return false;
+}
+
+Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
+{
+ const ChardevClass *cc;
+ Chardev *chr = NULL;
+ ChardevBackend *backend = NULL;
+ const char *name = chardev_alias_translate(qemu_opt_get(opts, "backend"));
+ const char *id = qemu_opts_id(opts);
+ char *bid = NULL;
+
+ if (qemu_chr_help(opts)) {
+ return NULL;
}
if (id == NULL) {
diff --git a/vl.c b/vl.c
index fb6b2ef..7cad539 100644
--- a/vl.c
+++ b/vl.c
@@ -2344,6 +2344,10 @@ static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
{
Error *local_err = NULL;
+ if (qemu_chr_help(opts)) {
+ exit(0);
+ }
+
qemu_chr_new_from_opts(opts, &local_err);
if (local_err) {
error_report_err(local_err);
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] char: don't exit on hmp 'chardev-add help'
2017-07-17 10:52 [Qemu-devel] [PATCH] char: don't exit on hmp 'chardev-add help' Anton Nefedov
@ 2017-07-17 11:19 ` Marc-André Lureau
2017-07-24 16:50 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Marc-André Lureau @ 2017-07-17 11:19 UTC (permalink / raw)
To: Anton Nefedov, qemu-devel; +Cc: pbonzini, den, dgilbert
Hi
On Mon, Jul 17, 2017 at 12:56 PM Anton Nefedov <anton.nefedov@virtuozzo.com>
wrote:
> qemu_chr_new_from_opts() is used from both vl.c and hmp,
> and it is quite confusing to see qemu suddenly exit after receiving a help
> option in hmp.
>
> Create a separate function for printing help and let different callers
> react accordingly.
>
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
>
---
> include/chardev/char.h | 9 +++++++++
> chardev/char.c | 27 +++++++++++++++++++--------
> vl.c | 4 ++++
> 3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 1604ea9..461962b 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -59,6 +59,15 @@ struct Chardev {
> };
>
> /**
> + * @qemu_chr_help:
> + *
> + * Print available chardevice backends if 'asked for help' in @opts
> + *
> + * Returns: true if there was a help option
> + */
> +bool qemu_chr_help(QemuOpts *opts);
> +
> +/**
> * @qemu_chr_new_from_opts:
> *
> * Create a new character backend from a QemuOpts list.
> diff --git a/chardev/char.c b/chardev/char.c
> index c34b44a..19833fe 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -604,14 +604,9 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
> Error **errp)
> return backend;
> }
>
> -Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
> +bool qemu_chr_help(QemuOpts *opts)
> {
> - const ChardevClass *cc;
> - Chardev *chr = NULL;
> - ChardevBackend *backend = NULL;
> - const char *name = chardev_alias_translate(qemu_opt_get(opts,
> "backend"));
> - const char *id = qemu_opts_id(opts);
> - char *bid = NULL;
> + const char *name = qemu_opt_get(opts, "backend");
>
> if (name && is_help_option(name)) {
> GString *str = g_string_new("");
> @@ -620,7 +615,23 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error
> **errp)
>
> error_report("Available chardev backend types: %s", str->str);
> g_string_free(str, true);
> - exit(0);
> + return true;
> + }
> +
> + return false;
> +}
> +
> +Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
> +{
> + const ChardevClass *cc;
> + Chardev *chr = NULL;
> + ChardevBackend *backend = NULL;
> + const char *name = chardev_alias_translate(qemu_opt_get(opts,
> "backend"));
> + const char *id = qemu_opts_id(opts);
> + char *bid = NULL;
> +
> + if (qemu_chr_help(opts)) {
> + return NULL;
> }
>
> if (id == NULL) {
> diff --git a/vl.c b/vl.c
> index fb6b2ef..7cad539 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2344,6 +2344,10 @@ static int chardev_init_func(void *opaque, QemuOpts
> *opts, Error **errp)
> {
> Error *local_err = NULL;
>
> + if (qemu_chr_help(opts)) {
> + exit(0);
> + }
> +
> qemu_chr_new_from_opts(opts, &local_err);
>
It is a bit intriguing to have qemu_chr_help() in two code paths here, but
since it should exit at first, it's not a big deal imho.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> if (local_err) {
> error_report_err(local_err);
> --
> 2.7.4
>
>
> --
Marc-André Lureau
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] char: don't exit on hmp 'chardev-add help'
2017-07-17 10:52 [Qemu-devel] [PATCH] char: don't exit on hmp 'chardev-add help' Anton Nefedov
2017-07-17 11:19 ` Marc-André Lureau
@ 2017-07-24 16:50 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2017-07-24 16:50 UTC (permalink / raw)
To: Anton Nefedov, qemu-devel; +Cc: den, marcandre.lureau, dgilbert
On 17/07/2017 12:52, Anton Nefedov wrote:
> qemu_chr_new_from_opts() is used from both vl.c and hmp,
> and it is quite confusing to see qemu suddenly exit after receiving a help
> option in hmp.
>
> Create a separate function for printing help and let different callers
> react accordingly.
>
> Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
> ---
> include/chardev/char.h | 9 +++++++++
> chardev/char.c | 27 +++++++++++++++++++--------
> vl.c | 4 ++++
> 3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/include/chardev/char.h b/include/chardev/char.h
> index 1604ea9..461962b 100644
> --- a/include/chardev/char.h
> +++ b/include/chardev/char.h
> @@ -59,6 +59,15 @@ struct Chardev {
> };
>
> /**
> + * @qemu_chr_help:
> + *
> + * Print available chardevice backends if 'asked for help' in @opts
> + *
> + * Returns: true if there was a help option
> + */
> +bool qemu_chr_help(QemuOpts *opts);
> +
> +/**
> * @qemu_chr_new_from_opts:
> *
> * Create a new character backend from a QemuOpts list.
> diff --git a/chardev/char.c b/chardev/char.c
> index c34b44a..19833fe 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -604,14 +604,9 @@ ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts, Error **errp)
> return backend;
> }
>
> -Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
> +bool qemu_chr_help(QemuOpts *opts)
> {
> - const ChardevClass *cc;
> - Chardev *chr = NULL;
> - ChardevBackend *backend = NULL;
> - const char *name = chardev_alias_translate(qemu_opt_get(opts, "backend"));
> - const char *id = qemu_opts_id(opts);
> - char *bid = NULL;
> + const char *name = qemu_opt_get(opts, "backend");
>
> if (name && is_help_option(name)) {
> GString *str = g_string_new("");
> @@ -620,7 +615,23 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
>
> error_report("Available chardev backend types: %s", str->str);
> g_string_free(str, true);
> - exit(0);
> + return true;
> + }
> +
> + return false;
> +}
> +
> +Chardev *qemu_chr_new_from_opts(QemuOpts *opts, Error **errp)
> +{
> + const ChardevClass *cc;
> + Chardev *chr = NULL;
> + ChardevBackend *backend = NULL;
> + const char *name = chardev_alias_translate(qemu_opt_get(opts, "backend"));
> + const char *id = qemu_opts_id(opts);
> + char *bid = NULL;
> +
> + if (qemu_chr_help(opts)) {
> + return NULL;
> }
>
> if (id == NULL) {
This is good, if only as a refactoring.
At this point, however, you don't need anymore the qemu_chr_help call below.
Instead, qemu_chr_new_from_opts now has three possible results:
- NULL, no error: help printed
- NULL, error: error
- non-NULL: chardev created
> diff --git a/vl.c b/vl.c
> index fb6b2ef..7cad539 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2344,6 +2344,10 @@ static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp)
> {
> Error *local_err = NULL;
>
> + if (qemu_chr_help(opts)) {
> + exit(0);
> + }
> +
> qemu_chr_new_from_opts(opts, &local_err);
So I think you can instead do:
if (!qemu_chr_new_from_opts(opts, &local_err)) {
if (!local_err) {
exit(0);
}
error_report_err(local_err);
exit(1);
}
return 0;
or something like that.
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-07-24 16:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-17 10:52 [Qemu-devel] [PATCH] char: don't exit on hmp 'chardev-add help' Anton Nefedov
2017-07-17 11:19 ` Marc-André Lureau
2017-07-24 16:50 ` Paolo Bonzini
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).