Re: [Qemu-devel] [PATCH v1 0/8] Remove some of the fprintf(stderr, "*

[Alistair Francis]

On Tue, Sep 26, 2017 at 2:05 AM, Stefan Hajnoczi <address@hidden> wrote:
> On Mon, Sep 25, 2017 at 05:08:34PM -0700, Alistair Francis wrote:
>>
>> Continue on improving QEMUs logging/error messages by removing more
>> fprintf()'s.
>>
>> Unfortunatley my Coccinelle skills aren't that great so it's all done in
>> some nasty regex and a little bit of manual work.
>>
>>
>>
>> Alistair Francis (8):
>>   Replace all occurances of __FUNCTION__ with __func__
>>   tests: Replace fprintf(stderr, "*\n" with error_report()
>>   hw: Replace fprintf(stderr, "*\n" with error_report()
>>   block: Replace fprintf(stderr, "*\n" with error_report()
>>   util: Replace fprintf(stderr, "*\n" with error_report()
>>   ui: Replace fprintf(stderr, "*\n" with error_report()
>>   tcg: Replace fprintf(stderr, "*\n" with error_report()
>>   target: Replace fprintf(stderr, "*\n" with error_report()
>>
>>  audio/audio_int.h                      |   2 +-
>>  block/file-posix.c                     |   6 +-
>>  block/file-win32.c                     |   3 +-
>>  block/linux-aio.c                      |   5 +-
>>  block/parallels.c                      |   7 ++-
>>  block/qcow2-cluster.c                  |   2 +-
>>  block/qcow2-refcount.c                 |  95 +++++++++++++++----------------
>>  block/qcow2.c                          |   8 +--
>>  block/quorum.c                         |   5 +-
>>  block/ssh.c                            |   4 +-
>>  block/vdi.c                            |  14 +++--
>>  block/vpc.c                            |   5 +-
>>  block/vvfat.c                          |  99 
>> ++++++++++++++++++---------------
>
> Please don't convert fprintfs to error_report in the block layer yet.
> error_report() uses the cur_mon global variable in a way that is not
> thread-safe:
>
>   void error_vprintf(const char *fmt, va_list ap)
>   {
>       if (cur_mon && !monitor_cur_is_qmp()) {
>           monitor_vprintf(cur_mon, fmt, ap);
>
> This is a time-of-check-to-time-of-use race condition.
> monitor_cur_is_qmp() itself also suffers from a race condition.
>
> I guess we haven't seen issues because error_report() is rarely called.

Thanks, I didn't know that. I'll drop the block patches then.

Thanks,
Alistair

>
> Stefan