Re: [PATCH v2 7/9] chardev/char: Preserve %errno in qemu_chr_write()

["Daniel P. Berrangé" <berrange@redhat.com>]

On Tue, Oct 28, 2025 at 06:00:33PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Oct 22, 2025 at 7:10 PM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
> >
> > qemu_chr_write() dispatches to ChardevClass::chr_write(),
> > and is expected to propagate the backend error, not some
> > unrelated one produce by "best effort" logfile or replay.
> > Preserve and return the relevant %errno.
> 
> Indeed.. imho we should avoid using errno, it's too easy to clutter.
> Even qemu mutex, which may use trace, may change it...
> 
> patch lgtm anyway
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Given you say 'qemu mutex, which may use trace, may change it...'
then surely this patch is broken....


> > @@ -154,6 +156,7 @@ static int qemu_chr_write_buffer(Chardev *s,
> >           */
> >          qemu_chr_write_log(s, buf, len);
> >      }
> > +    errno = saved_errno;
> >      qemu_mutex_unlock(&s->chr_write_lock);

^^^ This mutex_unlock call may clobber 'errno' that we've just tried
to restore.

> >
> >      return res;

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|