Am 09.03.2011 um 12:42 schrieb Jan Kiszka:
> On 2011-03-09 11:41, Corentin Chary wrote:
>> The threaded VNC servers messed up with QEMU fd handlers without
>> any kind of locking, and that can cause some nasty race conditions.
>>
>> The IO-Thread provides appropriate locking primitives to avoid that.
>> This patch makes CONFIG_VNC_THREAD depends on CONFIG_IO_THREAD,
>> and add lock and unlock calls around the two faulty calls.
>>
>> Thanks to Jan Kiszka for helping me solve this issue.
>>
>> Cc: Jan Kiszka <jan.kiszka@xxxxxx>
>> Signed-off-by: Corentin Chary <corentin.chary@xxxxxxxxx>
>> ---
>> configure | 9 +++++++++
>> ui/vnc-jobs-async.c | 4 ++++
>> 2 files changed, 13 insertions(+), 0 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 5513d3e..c8c1ac1 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2455,6 +2455,15 @@ if test \( "$cpu" = "i386" -o "$cpu" = "x86_64" \) -a \
>> roms="optionrom"
>> fi
>>
>> +# VNC Thread depends on IO Thread
>> +if test "$vnc_thread" = "yes" -a "$io_thread" != "yes"; then
>> + echo
>> + echo "ERROR: VNC thread depends on IO thread which isn't enabled."
>> + echo "Please use --enable-io-thread if you want to enable it."
>> + echo
>> + exit 1
>> +fi
>> +
>>
>> echo "Install prefix $prefix"
>> echo "BIOS directory `eval echo $datadir`"
>> diff --git a/ui/vnc-jobs-async.c b/ui/vnc-jobs-async.c
>> index f596247..093c0d4 100644
>> --- a/ui/vnc-jobs-async.c
>> +++ b/ui/vnc-jobs-async.c
>> @@ -260,7 +260,9 @@ static int vnc_worker_thread_loop(VncJobQueue *queue)
>> goto disconnected;
>> }
>>
>> + qemu_mutex_lock_iothread();
>
> Doesn't this comes with a risk of an ABBA deadlock between output_mutex
> and the global lock? Here you take the global lock while holding
> output_mutex, but I bet there is also the other way around when vnc
> services are called from the main thread or a vcpu.
so after all i should stay with disabled vnc-thread in qemu-kvm until there is a solution?
br,
peter
>
>> vnc_write(job->vs, vs.output.buffer, vs.output.offset);
>> + qemu_mutex_unlock_iothread();
>>
>> disconnected:
>> /* Copy persistent encoding data */
>> @@ -269,7 +271,9 @@ disconnected:
>> vnc_unlock_output(job->vs);
>>
>> if (flush) {
>> + qemu_mutex_lock_iothread();
>> vnc_flush(job->vs);
>> + qemu_mutex_unlock_iothread();
>> }
>>
>> vnc_lock_queue(queue);
>
> The second hunk looks safe.
>
> Jan
>
> --
> Siemens AG, Corporate Technology, CT T DE IT 1
> Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
