Hi
On Tue, Mar 24, 2026 at 5:54 PM Daniel P. Berrangé <[email protected]> wrote:
>
> On Tue, Mar 17, 2026 at 12:50:22PM +0400, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau <[email protected]>
> > ---
> > ui/vnc-jobs.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c
> > index 28a4738f1ec..ae925171fae 100644
> > --- a/ui/vnc-jobs.c
> > +++ b/ui/vnc-jobs.c
> > @@ -122,14 +122,14 @@ static void vnc_job_free(VncJob *job)
> >
> > void vnc_job_push(VncJob *job)
> > {
> > - vnc_lock_queue(queue);
> > if (QLIST_EMPTY(&job->rectangles)) {
>
> All the APIs which write to job->rectangles do that in a block
> that is protected by vnc_lock_queue, so narrowing this to allow
> lock-less access of job->rectangles feels dubious, without some
> justification in the commit message to explain why it is safe
> for job->rectangles to be unprotected, while 'queue->jobs' still
> needs protection.
The caller is vnc_update_client(). It creates the job and push damaged
rect. It shouldn't take the lock either. Only if it modified a queued
job it would need the lock, but it doesn't. I'll update the patch.
>
> > vnc_job_free(job);
> > } else {
> > + vnc_lock_queue(queue);
> > QTAILQ_INSERT_TAIL(&queue->jobs, job, next);
> > qemu_cond_broadcast(&queue->cond);
> > + vnc_unlock_queue(queue);
> > }
> > - vnc_unlock_queue(queue);
> > }
> >
> > static bool vnc_has_job_locked(VncState *vs)
> >
> > --
> > 2.53.0
> >
> >
>
> With regards,
> Daniel
> --
> |: https://berrange.com ~~ https://hachyderm.io/@berrange :|
> |: https://libvirt.org ~~ https://entangle-photo.org :|
> |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
>
>
--
Marc-André Lureau
