Re: [Qemu-devel] [PATCH v4] block: replace fprintf(stderr, ...) with error_report()

[Markus Armbruster]

Stefan Hajnoczi <address@hidden> writes:

> On Mon, May 26, 2014 at 09:44:03AM +0800, Le Tan wrote:
>> Replace fprintf(stderr,...) with error_report() in files block/*, block.c,
>> block-migration.c and blockdev.c. The trailing "\n"s of the @fmt argument
>> have been removed because @fmt of error_report() should not contain newline.
>> 
>> Signed-off-by: Le Tan <address@hidden>
>> ---
>>  block-migration.c      |    6 +--
>>  block.c                |    4 +-
>>  block/qcow2-refcount.c |  115 +++++++++++++++++++++---------------------
>>  block/qcow2.c          |   16 +++---
>>  block/raw-posix.c      |    8 ++-
>>  block/raw-win32.c      |    6 +--
>>  block/ssh.c            |    2 +-
>>  block/vdi.c            |   14 +++---
>>  block/vmdk.c           |   15 +++---
>>  block/vpc.c            |    4 +-
>>  block/vvfat.c | 129
>> ++++++++++++++++++++++++------------------------
>>  blockdev.c             |    6 +--
>>  12 files changed, 159 insertions(+), 166 deletions(-)
>
> There is one thing that worries me about this:
>
> error_report() assumes that the QEMU global mutex is held in order to
> access the "current monitor".

Global variable cur_mon, non-null while we're executing a monitor
command.

> This is problematic for code in the read/write/flush path (like qcow2
> refcount allocation) since it can be invoked from a virtio-blk
> data-plane thread (where the QEMU global mutex is not held).

error_report() reports to the current monitor when "in monitor context",
i.e. while executing a monitor command, i.e. while cur_mon isn't null.

Can we ever be in monitor context (cur_mon not null) without holding the
global mutex?

> For the time being, I prefer keeping the fprintf().  In the long term we
> need to decide:
> 1. Some functions should use Error * to report errors.
> 2. Some functions really produce their own output but it probably
> shouldn't go to the monitor.
> 3. The remaining functions may really be error_report() candidates.

0. Figure out what rules apply to cur_mon with respect to the global
mutex.