Stefan Hajnoczi <stefa...@redhat.com> writes: > On Mon, May 26, 2014 at 05:59:03PM +0200, Markus Armbruster wrote: >> Kevin Wolf <kw...@redhat.com> writes: >> >> > 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. >> >> Well, the answer used to be "no". Once upon a time because there was >> just a single thread, later on because only one thread ever executed >> "interesting" code. >> >> I'm *not* saying this should remain the case. I'm trying to find out >> what needs to be done around cur_mon when we break the assumption behind >> it. >> >> Would making cur_mon thread-local suffice? > > It would suffice. > > I think there is no code that invokes error_report() from another thread > on purpose (i.e. it knows the main thread is waiting), so making it > thread-local should not introduce a regression.
So, who's going to cook up the patch to make it thread-local?
