Re: [Qemu-devel] Re: segmentation fault in qemu-kvm-0.14.0

[Jan Kiszka]

On 2011-03-09 10:54, Corentin Chary wrote:
> Re-reading:
> 
>>> So we are calling a IOHandlerRecord::fd_write handler that is NULL.
>>> Looking at qemu_set_fd_handler2, this may happen if that function is
>>> called for an existing io-handler entry with non-NULL write handler,
>>> passing a NULL write and a non-NULL read handler. And all this without
>>> the global mutex held.
> 
> When using the vnc thread, nothing in the vnc thread will never be
> called directly by an IO-handler. So I don't really how in would
> trigger this.
> And since there is a lock for accessing the output buffer (and the
> network actually), there should not be any race condition either.
> 
> So the real issue, is that qemu_set_fd_handler2() is called outside of
> the main thread by those two vnc_write() and vnc_flush() calls,
> without any kind of locking.

Yes, that's what I was referring to.

> 
>> In upstream qemu, the latter - if it exists (which is not the case in
>> non-io-thread mode).
>> In qemu-kvm, those locks are yet unused. Rather the code in qemu-kvm.c
>> implements the global lock.
> 
> So there is currently no lock for that when io-thread is disabled :/.
> Spice also seems to project this kind of thing with
> qemu_mutex_lock_iothread().
> 
> Maybe qemu_mutex_lock_iothread() should also be defined when
> CONFIG_VNC_THREAD=y ?
> 
>> But I'm also not sure about vnc_send_framebuffer_update. Someone has to
>> go through the threaded vnc code again, very thoroughly. It looks fragile.
> 
> while vnc-thread is enabled vnc_send_framebuffer_update() will always
> call vnc_write() with csock = -1 in a temporary buffer. Check
> vnc_async_encoding_start() and vnc_async_encoding_end(), they provide
> a kind of "sandbox" that prevent the thread to write anything the
> main-thread will use. You can also see that as a "transaction": the
> thread compute the update in a temporary buffer, and only send it to
> the network (real vnc_write calls with csock correctly set) once it's
> successfully finished.
> 
> The is only two functions calls that break this isolation are the two
> that I pointed out earlier.

Probably the best way is to make vnc stop fiddling with
qemu_set_fd_handler2, specifically in threaded mode. Why does it need to
set/reset the write handler all the time?

Jan

signature.asc
Description: OpenPGP digital signature