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 <tamlokv...@gmail.com>
> ---
>  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".

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).

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.

Stefan

Reply via email to