[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH v2 2/2] vnc: threaded VNC server
From: |
Corentin Chary |
Subject: |
[Qemu-devel] Re: [PATCH v2 2/2] vnc: threaded VNC server |
Date: |
Fri, 4 Jun 2010 16:55:00 +0200 |
On Fri, Jun 4, 2010 at 4:41 PM, Paolo Bonzini <address@hidden> wrote:
>> + vnc_lock_queue(queue);
>> + if (QTAILQ_EMPTY(&queue->jobs)) {
>> + qemu_cond_wait(&queue->cond,&queue->mutex);
>> + }
>> +
>> + /* If the queue is empty, it's an exit order */
>> + if (QTAILQ_EMPTY(&queue->jobs)) {
>> + vnc_unlock_queue(queue);
>> + return -1;
>> + }
>> +
>> + job = QTAILQ_FIRST(&queue->jobs);
>> + vnc_unlock_queue(queue);
>
> Wrong usage of condition variables...
Because it's not in a loop ? Like I said in the cover-letter (but this
should probably be in a comment here) you can't use multiple worker
on a single queue because of zlib streams. But if the issue is more a
style issue than the actual implementation, then I can use a loop and
an exit flag.
>> + flush = (job->vs->csock != -1&& job->vs->abording != true);
>
> and typo still there.
Ooops, I did it again.
>> +static void *vnc_worker_thread(void *arg)
>> +{
>> + VncJobQueue *queue = arg;
>
> Also, it's better (future proof) to call qemu_thread_self here.
Ok,
--
Corentin Chary
http://xf.iksaif.net
Re: [Qemu-devel] [PATCH v2 2/2] vnc: threaded VNC server, Avi Kivity, 2010/06/06