| Message ID | 20180907075948.26917-2-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Various qemu command line options help improvements | expand |
On 2018-09-07 09:59, Marc-André Lureau wrote: > qdev_device_help() is used from command line "-device help", or from > HMP "device_add". If used from command line, print help to stdout > (it is only printed on explicit demand). Good idea, it always bugged me that "-device help" behaves differently than "-help"! > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > include/monitor/monitor.h | 2 ++ > monitor.c | 16 +++++++++++++--- > qdev-monitor.c | 32 +++++++++++++++++++------------- > 3 files changed, 34 insertions(+), 16 deletions(-) > > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h > index 2ef5e04b37..e7667fda35 100644 > --- a/include/monitor/monitor.h > +++ b/include/monitor/monitor.h > @@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd); > void monitor_fdset_dup_fd_remove(int dup_fd); > int monitor_fdset_dup_fd_find(int dup_fd); > > +void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); The name is a little bit too generic for my taste ... but I also fail to come up with really good suggestions... maybe "mon_file_vprintf()" or something similar? Thomas
On 09/07/2018 02:59 AM, Marc-André Lureau wrote: > qdev_device_help() is used from command line "-device help", or from > HMP "device_add". If used from command line, print help to stdout > (it is only printed on explicit demand). > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > include/monitor/monitor.h | 2 ++ > monitor.c | 16 +++++++++++++--- > qdev-monitor.c | 32 +++++++++++++++++++------------- > 3 files changed, 34 insertions(+), 16 deletions(-) > > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h > index 2ef5e04b37..e7667fda35 100644 > --- a/include/monitor/monitor.h > +++ b/include/monitor/monitor.h > @@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd); > void monitor_fdset_dup_fd_remove(int dup_fd); > int monitor_fdset_dup_fd_find(int dup_fd); > > +void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); Perhaps name this monitor_vfprintf()? (That would match the fact that you are using it like normal vfprintf). > +++ b/qdev-monitor.c > @@ -104,22 +104,31 @@ static bool qdev_class_has_alias(DeviceClass *dc) > return (qdev_class_get_alias(dc) != NULL); > } > > +static void out_printf(const char *fmt, ...) > +{ > + va_list ap; > + > + va_start(ap, fmt); > + out_vprintf(stdout, fmt, ap); > + va_end(ap); > +} But this name seems reasonable. And I just now see that Thomas also complained about the monitor.h naming, but I like 'monitor_vfprintf' better than his 'mon_file_vprintf'. If an improved name is your only change, Reviewed-by: Eric Blake <eblake@redhat.com>
Hi On Fri, Sep 7, 2018 at 5:49 PM Eric Blake <eblake@redhat.com> wrote: > > On 09/07/2018 02:59 AM, Marc-André Lureau wrote: > > qdev_device_help() is used from command line "-device help", or from > > HMP "device_add". If used from command line, print help to stdout > > (it is only printed on explicit demand). > > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > > --- > > include/monitor/monitor.h | 2 ++ > > monitor.c | 16 +++++++++++++--- > > qdev-monitor.c | 32 +++++++++++++++++++------------- > > 3 files changed, 34 insertions(+), 16 deletions(-) > > > > diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h > > index 2ef5e04b37..e7667fda35 100644 > > --- a/include/monitor/monitor.h > > +++ b/include/monitor/monitor.h > > @@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd); > > void monitor_fdset_dup_fd_remove(int dup_fd); > > int monitor_fdset_dup_fd_find(int dup_fd); > > > > +void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); > > Perhaps name this monitor_vfprintf()? (That would match the fact that > you are using it like normal vfprintf). > > > +++ b/qdev-monitor.c > > @@ -104,22 +104,31 @@ static bool qdev_class_has_alias(DeviceClass *dc) > > return (qdev_class_get_alias(dc) != NULL); > > } > > > > +static void out_printf(const char *fmt, ...) > > +{ > > + va_list ap; > > + > > + va_start(ap, fmt); > > + out_vprintf(stdout, fmt, ap); > > + va_end(ap); > > +} > > But this name seems reasonable. > > And I just now see that Thomas also complained about the monitor.h > naming, but I like 'monitor_vfprintf' better than his 'mon_file_vprintf'. > > If an improved name is your only change, > Reviewed-by: Eric Blake <eblake@redhat.com> I'll change it to monitor_vfprintf, thanks > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3266 > Virtualization: qemu.org | libvirt.org >
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 2ef5e04b37..e7667fda35 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -47,4 +47,6 @@ int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd); void monitor_fdset_dup_fd_remove(int dup_fd); int monitor_fdset_dup_fd_find(int dup_fd); +void out_vprintf(FILE *stream, const char *fmt, va_list ap) GCC_FMT_ATTR(2, 0); + #endif /* MONITOR_H */ diff --git a/monitor.c b/monitor.c index 021c11b1bf..6ea4082ab5 100644 --- a/monitor.c +++ b/monitor.c @@ -4597,19 +4597,29 @@ static void monitor_readline_flush(void *opaque) } /* - * Print to current monitor if we have one, else to stderr. + * Print to current monitor if we have one, else to stream. * TODO should return int, so callers can calculate width, but that * requires surgery to monitor_vprintf(). Left for another day. */ -void error_vprintf(const char *fmt, va_list ap) +void out_vprintf(FILE *stream, const char *fmt, va_list ap) { if (cur_mon && !monitor_cur_is_qmp()) { monitor_vprintf(cur_mon, fmt, ap); } else { - vfprintf(stderr, fmt, ap); + vfprintf(stream, fmt, ap); } } +/* + * Print to current monitor if we have one, else to stderr. + * TODO should return int, so callers can calculate width, but that + * requires surgery to monitor_vprintf(). Left for another day. + */ +void error_vprintf(const char *fmt, va_list ap) +{ + out_vprintf(stderr, fmt, ap); +} + void error_vprintf_unless_qmp(const char *fmt, va_list ap) { if (cur_mon && !monitor_cur_is_qmp()) { diff --git a/qdev-monitor.c b/qdev-monitor.c index 61e0300991..1da324aba6 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -104,22 +104,31 @@ static bool qdev_class_has_alias(DeviceClass *dc) return (qdev_class_get_alias(dc) != NULL); } +static void out_printf(const char *fmt, ...) +{ + va_list ap; + + va_start(ap, fmt); + out_vprintf(stdout, fmt, ap); + va_end(ap); +} + static void qdev_print_devinfo(DeviceClass *dc) { - error_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc))); + out_printf("name \"%s\"", object_class_get_name(OBJECT_CLASS(dc))); if (dc->bus_type) { - error_printf(", bus %s", dc->bus_type); + out_printf(", bus %s", dc->bus_type); } if (qdev_class_has_alias(dc)) { - error_printf(", alias \"%s\"", qdev_class_get_alias(dc)); + out_printf(", alias \"%s\"", qdev_class_get_alias(dc)); } if (dc->desc) { - error_printf(", desc \"%s\"", dc->desc); + out_printf(", desc \"%s\"", dc->desc); } if (!dc->user_creatable) { - error_printf(", no-user"); + out_printf(", no-user"); } - error_printf("\n"); + out_printf("\n"); } static void qdev_print_devinfos(bool show_no_user) @@ -155,8 +164,7 @@ static void qdev_print_devinfos(bool show_no_user) continue; } if (!cat_printed) { - error_printf("%s%s devices:\n", i ? "\n" : "", - cat_name[i]); + out_printf("%s%s devices:\n", i ? "\n" : "", cat_name[i]); cat_printed = true; } qdev_print_devinfo(dc); @@ -278,13 +286,11 @@ int qdev_device_help(QemuOpts *opts) } for (prop = prop_list; prop; prop = prop->next) { - error_printf("%s.%s=%s", driver, - prop->value->name, - prop->value->type); + out_printf("%s.%s=%s", driver, prop->value->name, prop->value->type); if (prop->value->has_description) { - error_printf(" (%s)\n", prop->value->description); + out_printf(" (%s)\n", prop->value->description); } else { - error_printf("\n"); + out_printf("\n"); } }
qdev_device_help() is used from command line "-device help", or from HMP "device_add". If used from command line, print help to stdout (it is only printed on explicit demand). Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- include/monitor/monitor.h | 2 ++ monitor.c | 16 +++++++++++++--- qdev-monitor.c | 32 +++++++++++++++++++------------- 3 files changed, 34 insertions(+), 16 deletions(-)