Re: [Qemu-devel] [PATCH] qemu-error: remove dependency of stubs on monitor

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> Leave the implementation of error_printf, error_printf_unless_qmp
> and error_vprintf to libqemustub.a and monitor.c, so that we can
> remove the monitor_printf and monitor_vprintf stubs.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>         This should help shutting up the vmstate unit tests.

Why does this make it any easier than my patch?
You're still going to need to add something stub specific to turn
the output on and off.

Dave

>  monitor.c            | 38 ++++++++++++++++++++++++++++++++++++++
>  stubs/Makefile.objs  |  2 +-
>  stubs/error-printf.c | 26 ++++++++++++++++++++++++++
>  stubs/mon-printf.c   | 11 -----------
>  util/qemu-error.c    | 38 --------------------------------------
>  5 files changed, 65 insertions(+), 50 deletions(-)
>  create mode 100644 stubs/error-printf.c
>  delete mode 100644 stubs/mon-printf.c
> 
> diff --git a/monitor.c b/monitor.c
> index b73a999..dab910f 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -3957,6 +3957,44 @@ static void monitor_readline_flush(void *opaque)
>      monitor_flush(opaque);
>  }
>  
> +/*
> + * 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)
> +{
> +    if (cur_mon && !monitor_cur_is_qmp()) {
> +        monitor_vprintf(cur_mon, fmt, ap);
> +    } else {
> +        vfprintf(stderr, fmt, ap);
> +    }
> +}
> +
> +/*
> + * Print to current monitor if we have one, else to stderr.
> + * TODO just like error_vprintf()
> + */
> +void error_printf(const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    va_start(ap, fmt);
> +    error_vprintf(fmt, ap);
> +    va_end(ap);
> +}
> +
> +void error_printf_unless_qmp(const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    if (!monitor_cur_is_qmp()) {
> +        va_start(ap, fmt);
> +        error_vprintf(fmt, ap);
> +        va_end(ap);
> +    }
> +}
> +
>  static void __attribute__((constructor)) monitor_lock_init(void)
>  {
>      qemu_mutex_init(&monitor_lock);
> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs
> index c5850e8..044bb47 100644
> --- a/stubs/Makefile.objs
> +++ b/stubs/Makefile.objs
> @@ -9,6 +9,7 @@ stub-obj-y += clock-warp.o
>  stub-obj-y += cpu-get-clock.o
>  stub-obj-y += cpu-get-icount.o
>  stub-obj-y += dump.o
> +stub-obj-y += error-printf.o
>  stub-obj-y += fdset-add-fd.o
>  stub-obj-y += fdset-find-fd.o
>  stub-obj-y += fdset-get-fd.o
> @@ -22,7 +23,6 @@ stub-obj-y += is-daemonized.o
>  stub-obj-y += machine-init-done.o
>  stub-obj-y += migr-blocker.o
>  stub-obj-y += mon-is-qmp.o
> -stub-obj-y += mon-printf.o
>  stub-obj-y += monitor-init.o
>  stub-obj-y += notify-event.o
>  stub-obj-y += qtest.o
> diff --git a/stubs/error-printf.c b/stubs/error-printf.c
> new file mode 100644
> index 0000000..19f7dd0
> --- /dev/null
> +++ b/stubs/error-printf.c
> @@ -0,0 +1,26 @@
> +#include "qemu/osdep.h"
> +#include "qemu-common.h"
> +#include "qemu/error-report.h"
> +
> +void error_vprintf(const char *fmt, va_list ap)
> +{
> +    vfprintf(stderr, fmt, ap);
> +}
> +
> +void error_printf(const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    va_start(ap, fmt);
> +    error_vprintf(fmt, ap);
> +    va_end(ap);
> +}
> +
> +void error_printf_unless_qmp(const char *fmt, ...)
> +{
> +    va_list ap;
> +
> +    va_start(ap, fmt);
> +    error_vprintf(fmt, ap);
> +    va_end(ap);
> +}
> diff --git a/stubs/mon-printf.c b/stubs/mon-printf.c
> deleted file mode 100644
> index e7c1e0c..0000000
> --- a/stubs/mon-printf.c
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -#include "qemu/osdep.h"
> -#include "qemu-common.h"
> -#include "monitor/monitor.h"
> -
> -void monitor_printf(Monitor *mon, const char *fmt, ...)
> -{
> -}
> -
> -void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
> -{
> -}
> diff --git a/util/qemu-error.c b/util/qemu-error.c
> index 1ef3566..dffd781 100644
> --- a/util/qemu-error.c
> +++ b/util/qemu-error.c
> @@ -14,44 +14,6 @@
>  #include "monitor/monitor.h"
>  #include "qemu/error-report.h"
>  
> -/*
> - * 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)
> -{
> -    if (cur_mon && !monitor_cur_is_qmp()) {
> -        monitor_vprintf(cur_mon, fmt, ap);
> -    } else {
> -        vfprintf(stderr, fmt, ap);
> -    }
> -}
> -
> -/*
> - * Print to current monitor if we have one, else to stderr.
> - * TODO just like error_vprintf()
> - */
> -void error_printf(const char *fmt, ...)
> -{
> -    va_list ap;
> -
> -    va_start(ap, fmt);
> -    error_vprintf(fmt, ap);
> -    va_end(ap);
> -}
> -
> -void error_printf_unless_qmp(const char *fmt, ...)
> -{
> -    va_list ap;
> -
> -    if (!monitor_cur_is_qmp()) {
> -        va_start(ap, fmt);
> -        error_vprintf(fmt, ap);
> -        va_end(ap);
> -    }
> -}
> -
>  static Location std_loc = {
>      .kind = LOC_NONE
>  };
> -- 
> 2.7.4
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK