include/chardev/char.h | 9 +++++++++ chardev/char.c | 27 +++++++++++++++++++-------- vl.c | 4 ++++ 3 files changed, 32 insertions(+), 8 deletions(-)
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
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
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
© 2016 - 2024 Red Hat, Inc.