qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Paolo Bonzini
Subject: Re: [Qemu-devel] [PATCH] qemu-error: remove dependency of stubs on monitor
Date: Mon, 24 Oct 2016 13:52:59 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0


On 24/10/2016 12:34, Dr. David Alan Gilbert wrote:
> * Paolo Bonzini (address@hidden) 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 <address@hidden>
>> ---
>>         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.

It makes it possible to override the functions independent of the rest
of util/qemu-error.c. You can implement the functions in the test, simply as

    had_stderr_output = true;

and then assert that had_stderr_output is false or true depending on the
test.

(It's also a useful starting point to fix the cur_mon race).

Consider this an RFC.  error_printf probably should stay in qemu-error.c
since it can always call error_vprintf.

Paolo

> 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 / address@hidden / Manchester, UK
> 



reply via email to

[Prev in Thread] Current Thread [Next in Thread]