Am 26.05.2014 um 17:02 hat Markus Armbruster geschrieben: > Stefan Hajnoczi <stefa...@gmail.com> 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 <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". > > Global variable cur_mon, non-null while we're executing a monitor > command.
The important part here is that it's indeed global and not thread-local. > > 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? The right question is: Can a thread (= the main loop thread) ever be in monitor context while another thread (= dataplane thread) is executing block driver code and doesn't hold the global mutex? If I understand dataplane correctly, the whole point of it is that the answer to this is yes. Kevin > > 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.
