Re: [PATCH v3] util: use RETRY_ON_EINTR() on open() more consistently

[Philipp Reisner]

Hi Philippe and everyone else on the thread,

In the meantime, I learned that we can cause the kernel to restart the
open() syscall by returning ERESTARTSYS from the kernel-level block
device driver side. It was our mistake from the DRBD side that we
returned EINTR instead.

The proposed patch to qemu is okay but not necessary. I will no
further pursuit the inclusion of this patch.

Sorry for the noise.

Best regards,
 Philipp



On Thu, Aug 1, 2024 at 2:48 PM Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Hi Philipp,
>
> On 31/7/24 17:17, Philipp Reisner wrote:
> > As with many syscalls, open() might be interrupted by a signal.
> >
> > The call trace
> > img_open_file()
> >    blk_new_open()
> >      raw_open()
> >        raw_open_common()
> >          qemu_open()
> >            qemu_open_internal()
> >              qemu_open_cloexec()
> >
> > Ended up in calling open() without a retry loop around it.
> >
> > 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
> >
> > Add the RETRY_ON_EINTR() in qemu_open_cloexec() and remove it on
> > call-sites using qemu_open_old().
> >
> > Signed-off-by: Philipp Reisner <philipp.reisner@linbit.com>
> > ---
> >   chardev/char-fd.c   | 2 +-
> >   chardev/char-pipe.c | 4 ++--
> >   os-posix.c          | 2 +-
> >   util/osdep.c        | 4 ++--
> >   4 files changed, 6 insertions(+), 6 deletions(-)
>
> If you need to repost, please set the next version in your patch
> (which will be v4). Also avoid replying to previous version /
> discussions and your patch could be missed, see
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#when-resending-patches-add-a-version-tag:
>
>    Send each new revision as a new top-level thread, rather than
>    burying it in-reply-to an earlier revision, as many reviewers
>    are not looking inside deep threads for new patches.
>
> Also please Cc qemu-block@nongnu.org since the block layer is
> involved via the blk_new_open() call.
>
> Regards,
>
> Phil.