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