Re: [PATCH] util: retry open() when it gets interrupted by a signal

[Daniel P . Berrangé]

On Wed, Jul 31, 2024 at 03:32:52PM +0100, Peter Maydell wrote:
> On Wed, 31 Jul 2024 at 15:11, Daniel P. Berrangé <berrange@redhat.com> wrote:
> >
> > On Wed, Jul 31, 2024 at 03:25:24PM +0200, Philipp Reisner wrote:
> > > As with many syscalls, open() might be interrupted by a signal.
> > >
> > > The experienced logfile entry is:
> > >
> > > qemu-system-x86_64: -device 
> > > virtio-blk-pci,bus=pci.0,addr=0x7,drive=libvirt-2-format,id=virtio-disk0,bootindex=2,write-cache=on,serial=1b990c4d13b74a4e90ea:
> > >  Could not open '/dev/drbd1003': Interrupted system call

What is the actual signal you are seeing that impacts QEMU
in this way ?

> > > Retry it until it is not interrupted by a signal.
> >
> > As you say, many syscalls can be interruptted by signals, so
> > special casing open() isn't really a solution - its just
> > addressing one specific instance you happened to see.
> >
> > If there are certain signals that we don't want to have a
> > fatal interruption for, it'd be better to set SA_RESTART
> > with sigaction, which will auto-restart a large set of
> > syscalls, while allowing other signals to be fatal.
> 
> This is why we have the RETRY_ON_EINTR() macro, right?
> 
> Currently we have some places that call qemu_open_old() inside
> RETRY_ON_EINTR -- we should decide whether we want to
> handle EINTR inside the qemu_open family of functions,
> or make the caller deal with it, and put the macro uses
> in the right place consistently.

It is incredibly arbitrary where we use RETRY_ON_EINTR, which I think
points towards it being a sub-optimal solution to the general problem.

> 
> I agree that it would be nicer if we could use SA_RESTART,
> but presumably there's a reason why we don't. (At any
> rate code that's shared with the user-mode emulation
> has to be EINTR-resistant, because we can't force the
> user-mode guest code to avoid registering signal handlers
> that aren't SA_RESTART.)

For user mode emulation isn't it valid to just propagage the
EINTR back up to the application, since EINTR is a valid errno
they have to be willing to handle unless the app has itself
use SA_RESTART.

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 :|