Message ID | 20230908013535.990731-27-eblake@redhat.com |
---|---|
State | New |
Headers | show |
Series | [PULL,01/13] qemu-iotests/197: use more generic commands for formats other than qcow2 | expand |
Please resolve the following CI failure: https://gitlab.com/qemu-project/qemu/-/jobs/5045998355 ninja: job failed: cc -m64 -mcx16 -Iqemu-nbd.p -I. -I.. -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/p11-kit-1 -I/usr/include/glib-2.0 -I/usr/lib/glib-2.0/include -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -isystem /builds/qemu-project/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/qemu-project/qemu -iquote /builds/qemu-project/qemu/include -iquote /builds/qemu-project/qemu/host/include/x86_64 -iquote /builds/qemu-project/qemu/host/include/generic -iquote /builds/qemu-project/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -MD -MQ qemu-nbd.p/qemu-nbd.c.o -MF qemu-nbd.p/qemu-nbd.c.o.d -o qemu-nbd.p/qemu-nbd.c.o -c ../qemu-nbd.c In file included from /usr/include/fortify/stdio.h:23, from ../include/qemu/osdep.h:110, from ../qemu-nbd.c:19: ../qemu-nbd.c: In function 'nbd_client_thread': ../qemu-nbd.c:340:39: error: expected identifier before '(' token 340 | nbd_client_release_pipe(opts->stderr); | ^~~~~~ ../qemu-nbd.c: In function 'main': ../qemu-nbd.c:605:10: error: expected identifier before '(' token 605 | .stderr = STDOUT_FILENO, | ^~~~~~ ../qemu-nbd.c:962:22: error: expected identifier before '(' token 962 | opts.stderr = dup(STDERR_FILENO); | ^~~~~~ ../qemu-nbd.c:963:26: error: expected identifier before '(' token 963 | if (opts.stderr < 0) { | ^~~~~~ ../qemu-nbd.c:1200:38: error: expected identifier before '(' token 1200 | nbd_client_release_pipe(opts.stderr); | ^~~~~~ On Thu, 7 Sept 2023 at 21:37, Eric Blake <eblake@redhat.com> wrote: > > From: "Denis V. Lunev" <den@openvz.org> > > Closing stderr earlier is good for daemonized qemu-nbd under ssh > earlier, but breaks the case where -v is being used to track what is > happening in the server, as in iotest 233. > > When we know we are verbose, we should preserve original stderr and > restore it once the setup stage is done. This commit restores the > original behavior with -v option. In this case original output > inside the test is kept intact. > > Reported-by: Kevin Wolf <kwolf@redhat.com> > Signed-off-by: Denis V. Lunev <den@openvz.org> > CC: Eric Blake <eblake@redhat.com> > CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> > CC: Hanna Reitz <hreitz@redhat.com> > CC: Mike Maslenkin <mike.maslenkin@gmail.com> > Fixes: 5c56dd27a2 ("qemu-nbd: fix regression with qemu-nbd --fork run over ssh") > Message-ID: <20230906093210.339585-7-den@openvz.org> > Reviewed-by: Eric Blake <eblake@redhat.com> > Tested-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > qemu-nbd.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 7c4e22def17..1cdc41ed292 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -255,18 +255,23 @@ struct NbdClientOpts { > char *device; > char *srcpath; > SocketAddress *saddr; > + int stderr; > bool fork_process; > bool verbose; > }; > > -static void nbd_client_release_pipe(void) > +static void nbd_client_release_pipe(int old_stderr) > { > /* Close stderr so that the qemu-nbd process exits. */ > - if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) { > + if (dup2(old_stderr, STDERR_FILENO) < 0) { > error_report("Could not release pipe to parent: %s", > strerror(errno)); > exit(EXIT_FAILURE); > } > + if (old_stderr != STDOUT_FILENO && close(old_stderr) < 0) { > + error_report("Could not release qemu-nbd: %s", strerror(errno)); > + exit(EXIT_FAILURE); > + } > } > > #if HAVE_NBD_DEVICE > @@ -332,7 +337,7 @@ static void *nbd_client_thread(void *arg) > fprintf(stderr, "NBD device %s is now connected to %s\n", > opts->device, opts->srcpath); > } else { > - nbd_client_release_pipe(); > + nbd_client_release_pipe(opts->stderr); > } > > if (nbd_client(fd) < 0) { > @@ -597,6 +602,7 @@ int main(int argc, char **argv) > .device = NULL, > .srcpath = NULL, > .saddr = NULL, > + .stderr = STDOUT_FILENO, > }; > > #ifdef CONFIG_POSIX > @@ -951,6 +957,16 @@ int main(int argc, char **argv) > > close(stderr_fd[0]); > > + /* Remember parent's stderr if we will be restoring it. */ > + if (opts.verbose /* fork_process is set */) { > + opts.stderr = dup(STDERR_FILENO); > + if (opts.stderr < 0) { > + error_report("Could not dup original stderr: %s", > + strerror(errno)); > + exit(EXIT_FAILURE); > + } > + } > + > ret = qemu_daemon(1, 0); > saved_errno = errno; /* dup2 will overwrite error below */ > > @@ -1181,7 +1197,7 @@ int main(int argc, char **argv) > } > > if (opts.fork_process) { > - nbd_client_release_pipe(); > + nbd_client_release_pipe(opts.stderr); > } > > state = RUNNING; > -- > 2.41.0 > >
On 9/8/23 13:03, Stefan Hajnoczi wrote: > Please resolve the following CI failure: > > https://gitlab.com/qemu-project/qemu/-/jobs/5045998355 > > ninja: job failed: cc -m64 -mcx16 -Iqemu-nbd.p -I. -I.. -Iqapi -Itrace > -Iui -Iui/shader -I/usr/include/p11-kit-1 -I/usr/include/glib-2.0 > -I/usr/lib/glib-2.0/include -fdiagnostics-color=auto -Wall > -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong > -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings > -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls > -Wold-style-declaration -Wold-style-definition -Wtype-limits > -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers > -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined > -Wimplicit-fallthrough=2 -Wmissing-format-attribute > -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi > -isystem /builds/qemu-project/qemu/linux-headers -isystem > linux-headers -iquote . -iquote /builds/qemu-project/qemu -iquote > /builds/qemu-project/qemu/include -iquote > /builds/qemu-project/qemu/host/include/x86_64 -iquote > /builds/qemu-project/qemu/host/include/generic -iquote > /builds/qemu-project/qemu/tcg/i386 -pthread -D_GNU_SOURCE > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing > -fno-common -fwrapv -fPIE -MD -MQ qemu-nbd.p/qemu-nbd.c.o -MF > qemu-nbd.p/qemu-nbd.c.o.d -o qemu-nbd.p/qemu-nbd.c.o -c ../qemu-nbd.c > In file included from /usr/include/fortify/stdio.h:23, > from ../include/qemu/osdep.h:110, > from ../qemu-nbd.c:19: > ../qemu-nbd.c: In function 'nbd_client_thread': > ../qemu-nbd.c:340:39: error: expected identifier before '(' token > 340 | nbd_client_release_pipe(opts->stderr); > | ^~~~~~ > ../qemu-nbd.c: In function 'main': > ../qemu-nbd.c:605:10: error: expected identifier before '(' token > 605 | .stderr = STDOUT_FILENO, > | ^~~~~~ > ../qemu-nbd.c:962:22: error: expected identifier before '(' token > 962 | opts.stderr = dup(STDERR_FILENO); > | ^~~~~~ > ../qemu-nbd.c:963:26: error: expected identifier before '(' token > 963 | if (opts.stderr < 0) { > | ^~~~~~ > ../qemu-nbd.c:1200:38: error: expected identifier before '(' token > 1200 | nbd_client_release_pipe(opts.stderr); > | ^~~~~~ quite interesting and surprising. Tried to reproduce with ./configure --target-list=avr-softmmu,loongarch64-softmmu,mips64-softmmu,mipsel-softmmu --enable-werror --disable-docs --enable-fdt=system locally but without success - the code is compiled fine. Is there any way to get into the Jenkins environment? Den
On 9/8/23 13:24, Denis V. Lunev wrote: > On 9/8/23 13:03, Stefan Hajnoczi wrote: >> Please resolve the following CI failure: >> >> https://gitlab.com/qemu-project/qemu/-/jobs/5045998355 >> >> ninja: job failed: cc -m64 -mcx16 -Iqemu-nbd.p -I. -I.. -Iqapi -Itrace >> -Iui -Iui/shader -I/usr/include/p11-kit-1 -I/usr/include/glib-2.0 >> -I/usr/lib/glib-2.0/include -fdiagnostics-color=auto -Wall >> -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong >> -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings >> -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls >> -Wold-style-declaration -Wold-style-definition -Wtype-limits >> -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers >> -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined >> -Wimplicit-fallthrough=2 -Wmissing-format-attribute >> -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi >> -isystem /builds/qemu-project/qemu/linux-headers -isystem >> linux-headers -iquote . -iquote /builds/qemu-project/qemu -iquote >> /builds/qemu-project/qemu/include -iquote >> /builds/qemu-project/qemu/host/include/x86_64 -iquote >> /builds/qemu-project/qemu/host/include/generic -iquote >> /builds/qemu-project/qemu/tcg/i386 -pthread -D_GNU_SOURCE >> -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing >> -fno-common -fwrapv -fPIE -MD -MQ qemu-nbd.p/qemu-nbd.c.o -MF >> qemu-nbd.p/qemu-nbd.c.o.d -o qemu-nbd.p/qemu-nbd.c.o -c ../qemu-nbd.c >> In file included from /usr/include/fortify/stdio.h:23, >> from ../include/qemu/osdep.h:110, >> from ../qemu-nbd.c:19: >> ../qemu-nbd.c: In function 'nbd_client_thread': >> ../qemu-nbd.c:340:39: error: expected identifier before '(' token >> 340 | nbd_client_release_pipe(opts->stderr); >> | ^~~~~~ >> ../qemu-nbd.c: In function 'main': >> ../qemu-nbd.c:605:10: error: expected identifier before '(' token >> 605 | .stderr = STDOUT_FILENO, >> | ^~~~~~ >> ../qemu-nbd.c:962:22: error: expected identifier before '(' token >> 962 | opts.stderr = dup(STDERR_FILENO); >> | ^~~~~~ >> ../qemu-nbd.c:963:26: error: expected identifier before '(' token >> 963 | if (opts.stderr < 0) { >> | ^~~~~~ >> ../qemu-nbd.c:1200:38: error: expected identifier before '(' token >> 1200 | nbd_client_release_pipe(opts.stderr); >> | ^~~~~~ > > quite interesting and surprising. Tried to reproduce with > > ./configure > --target-list=avr-softmmu,loongarch64-softmmu,mips64-softmmu,mipsel-softmmu > --enable-werror --disable-docs --enable-fdt=system > > locally but without success - the code is compiled fine. > > Is there any way to get into the Jenkins environment? > > Den The only possible reason I could imagine is that 'stderr' word is defined under by pre-processor. Den
On Fri, Sep 08, 2023 at 01:42:00PM +0200, Denis V. Lunev wrote: > On 9/8/23 13:24, Denis V. Lunev wrote: > > On 9/8/23 13:03, Stefan Hajnoczi wrote: > > > Please resolve the following CI failure: > > > > > > https://gitlab.com/qemu-project/qemu/-/jobs/5045998355 > > > > > > ../qemu-nbd.c: In function 'nbd_client_thread': > > > ../qemu-nbd.c:340:39: error: expected identifier before '(' token > > > 340 | nbd_client_release_pipe(opts->stderr); > > > | ^~~~~~ > The only possible reason I could imagine is that 'stderr' > word is defined under by pre-processor. Indeed, that is a common implementation; the obvious fix is to use a different name. v2 coming up with that change made.
diff --git a/qemu-nbd.c b/qemu-nbd.c index 7c4e22def17..1cdc41ed292 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -255,18 +255,23 @@ struct NbdClientOpts { char *device; char *srcpath; SocketAddress *saddr; + int stderr; bool fork_process; bool verbose; }; -static void nbd_client_release_pipe(void) +static void nbd_client_release_pipe(int old_stderr) { /* Close stderr so that the qemu-nbd process exits. */ - if (dup2(STDOUT_FILENO, STDERR_FILENO) < 0) { + if (dup2(old_stderr, STDERR_FILENO) < 0) { error_report("Could not release pipe to parent: %s", strerror(errno)); exit(EXIT_FAILURE); } + if (old_stderr != STDOUT_FILENO && close(old_stderr) < 0) { + error_report("Could not release qemu-nbd: %s", strerror(errno)); + exit(EXIT_FAILURE); + } } #if HAVE_NBD_DEVICE @@ -332,7 +337,7 @@ static void *nbd_client_thread(void *arg) fprintf(stderr, "NBD device %s is now connected to %s\n", opts->device, opts->srcpath); } else { - nbd_client_release_pipe(); + nbd_client_release_pipe(opts->stderr); } if (nbd_client(fd) < 0) { @@ -597,6 +602,7 @@ int main(int argc, char **argv) .device = NULL, .srcpath = NULL, .saddr = NULL, + .stderr = STDOUT_FILENO, }; #ifdef CONFIG_POSIX @@ -951,6 +957,16 @@ int main(int argc, char **argv) close(stderr_fd[0]); + /* Remember parent's stderr if we will be restoring it. */ + if (opts.verbose /* fork_process is set */) { + opts.stderr = dup(STDERR_FILENO); + if (opts.stderr < 0) { + error_report("Could not dup original stderr: %s", + strerror(errno)); + exit(EXIT_FAILURE); + } + } + ret = qemu_daemon(1, 0); saved_errno = errno; /* dup2 will overwrite error below */ @@ -1181,7 +1197,7 @@ int main(int argc, char **argv) } if (opts.fork_process) { - nbd_client_release_pipe(); + nbd_client_release_pipe(opts.stderr); } state = RUNNING;