- Rebased to libvirt-10.3.0 (RHEL-29642) - The rebase also fixes the following bugs: RHEL-25993, RHEL-7101, RHEL-7461, RHEL-16333, RHEL-7343 RHEL-28808, RHEL-32880, RHEL-35879, RHEL-34112, RHEL-30373 RHEL-23416, RHEL-23608, RHEL-26276, RHEL-22728 Resolves: RHEL-16333, RHEL-22728, RHEL-23416, RHEL-23608, RHEL-25993 Resolves: RHEL-26276, RHEL-28808, RHEL-29642, RHEL-30373, RHEL-32880 Resolves: RHEL-34112, RHEL-35879, RHEL-7101, RHEL-7343, RHEL-7461
83 lines
3.1 KiB
Diff
83 lines
3.1 KiB
Diff
From 52de75b7ad95b76d7ddaba12a4d9a0e196d9de64 Mon Sep 17 00:00:00 2001
|
|
Message-ID: <52de75b7ad95b76d7ddaba12a4d9a0e196d9de64.1715249019.git.jdenemar@redhat.com>
|
|
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
|
|
Date: Wed, 8 May 2024 11:50:09 +0100
|
|
Subject: [PATCH] tests: fix hang in virshtest 'read-big-pipe' case
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=UTF-8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
The virshtest program testPipeFeeder method is doing this:
|
|
|
|
mkfifo("test.fifo", 0600) ;
|
|
|
|
int fd = open("test.fifo", O_RDWR);
|
|
|
|
char buf[...];
|
|
memset(buf, 'a', sizeof(buf));
|
|
write(fd, buf, sizeof(buf)) == sizeof(buf));
|
|
close(fd);
|
|
|
|
while the the 'virsh' child process then ends up doing:
|
|
|
|
fd = open("test.fifo", O_RDONLY);
|
|
read(fd, buf, sizeof(buf)) == sizeof(buf));
|
|
close(fd);
|
|
|
|
The 'virsh' code hangs on open() on at least ppc64 and some other
|
|
arches. It can be provoked to hang even on x86 by reducing the size of
|
|
the buffer. It can be prevented from hanging on ppc64 by increasing the
|
|
size of the buffer.
|
|
|
|
What is happening is a result of differing page sizes, altering the
|
|
overall pipe capacity size, since pipes on linux default to 16 pages
|
|
in size and thus have architecture specific capacity when measured
|
|
in bytes.
|
|
|
|
* On x86, testPipeFeeder opens R+W, tries to write 140kb and
|
|
write() blocks because the pipe is full. This gives time for
|
|
virsh to start up, and it can open the pipe for O_RDONLY
|
|
since testPipeFeeder still has it open for write. Everything
|
|
works as intended.
|
|
|
|
* On ppc64, testPipeFeeder opens R+W, tries to write 140kb
|
|
and write() succeeds because the larger 64kb page size
|
|
resulted in greater buffer capacity for the pipe. It thus
|
|
quickly closes the pipe, removing the writer, and triggering
|
|
discard of all the unread data. Now virsh starts up, tries
|
|
to open the pipe for O_RDONLY and blocks waiting for a new
|
|
writer to open it, which will never happen. Meson kills
|
|
the test after 30 seconds.
|
|
|
|
NB, every now & then, it will not block because virsh starts
|
|
up quickly enough that testPipeFeeder has not yet closed the
|
|
write end of the pipe, giving the illusion of correctness.
|
|
|
|
The key flaw here is that it should not have been using O_RDWR
|
|
in testPipeFeeder. Synchronization is required such that both
|
|
virsh and testPipeFeeder have their respective ends of the pipe
|
|
open before any data is sent. This is trivially arranged by
|
|
using O_WRONLY in testPipeFeeder.
|
|
|
|
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
|
|
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
|
|
---
|
|
tests/virshtest.c | 2 +-
|
|
1 file changed, 1 insertion(+), 1 deletion(-)
|
|
|
|
diff --git a/tests/virshtest.c b/tests/virshtest.c
|
|
index a1ae481316..7a7797647c 100644
|
|
--- a/tests/virshtest.c
|
|
+++ b/tests/virshtest.c
|
|
@@ -145,7 +145,7 @@ testPipeFeeder(void *opaque)
|
|
g_autofree char *doc = g_new0(char, emptyspace + xmlsize + 1);
|
|
VIR_AUTOCLOSE fd = -1;
|
|
|
|
- if ((fd = open(pipepath, O_RDWR)) < 0) {
|
|
+ if ((fd = open(pipepath, O_WRONLY)) < 0) {
|
|
fprintf(stderr, "\nfailed to open pipe '%s': %s\n", pipepath, g_strerror(errno));
|
|
return;
|
|
}
|
|
--
|
|
2.45.0
|