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(-)
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(-)
diff --git a/chardev/char-fd.c b/chardev/char-fd.c
index d2c4923359..00a225bc48 100644
--- a/chardev/char-fd.c
+++ b/chardev/char-fd.c
@@ -198,7 +198,7 @@ int qmp_chardev_open_file_source(char *src, int flags, Error **errp)
{
int fd = -1;
- fd = RETRY_ON_EINTR(qemu_open_old(src, flags, 0666));
+ fd = qemu_open_old(src, flags, 0666);
if (fd == -1) {
error_setg_file_open(errp, errno, src);
}
diff --git a/chardev/char-pipe.c b/chardev/char-pipe.c
index 5ad30bcc59..c9dc793434 100644
--- a/chardev/char-pipe.c
+++ b/chardev/char-pipe.c
@@ -131,8 +131,8 @@ static void qemu_chr_open_pipe(Chardev *chr,
filename_in = g_strdup_printf("%s.in", filename);
filename_out = g_strdup_printf("%s.out", filename);
- fd_in = RETRY_ON_EINTR(qemu_open_old(filename_in, O_RDWR | O_BINARY));
- fd_out = RETRY_ON_EINTR(qemu_open_old(filename_out, O_RDWR | O_BINARY));
+ fd_in = qemu_open_old(filename_in, O_RDWR | O_BINARY);
+ fd_out = qemu_open_old(filename_out, O_RDWR | O_BINARY);
g_free(filename_in);
g_free(filename_out);
if (fd_in < 0 || fd_out < 0) {
diff --git a/os-posix.c b/os-posix.c
index 43f9a43f3f..7401e7da6b 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -291,7 +291,7 @@ void os_setup_post(void)
error_report("not able to chdir to /: %s", strerror(errno));
exit(1);
}
- fd = RETRY_ON_EINTR(qemu_open_old("/dev/null", O_RDWR));
+ fd = qemu_open_old("/dev/null", O_RDWR);
if (fd == -1) {
exit(1);
}
diff --git a/util/osdep.c b/util/osdep.c
index 770369831b..13a09d0dd5 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -295,9 +295,9 @@ static int qemu_open_cloexec(const char *name, int flags, mode_t mode)
{
int ret;
#ifdef O_CLOEXEC
- ret = open(name, flags | O_CLOEXEC, mode);
+ ret = RETRY_ON_EINTR(open(name, flags | O_CLOEXEC, mode));
#else
- ret = open(name, flags, mode);
+ ret = RETRY_ON_EINTR(open(name, flags, mode));
if (ret >= 0) {
qemu_set_cloexec(ret);
}
--
2.45.2Hi 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.
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.
© 2016 - 2024 Red Hat, Inc.