[Libguestfs] libnbd | Failed pipeline for master | bab30812

2023-05-09 Thread GitLab
Pipeline #862248012 has failed! Project: libnbd ( https://gitlab.com/nbdkit/libnbd ) Branch: master ( https://gitlab.com/nbdkit/libnbd/-/commits/master ) Commit: bab30812 ( https://gitlab.com/nbdkit/libnbd/-/commit/bab308126517f3ac480cf15dbcf32bdef9058f54 ) Commit Message: tests: Add a test

Re: [Libguestfs] [PATCH libnbd v2] tests: Add a test of nbd_{set, get}_socket_activation_name

2023-05-09 Thread Richard W.M. Jones
This was pushed as commit bab3081265 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-builder quickly builds VMs from scratch http://libguestfs.org/virt-builder.1.html

Re: [Libguestfs] [PATCH nbdkit v3 6/6] server: debug: Escape debug strings

2023-05-09 Thread Richard W.M. Jones
Thanks for everyone's feedback. I pushed this series with several enhancements as: 5fbf93a92..2f7ca818f Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows

Re: [Libguestfs] [PATCH nbdkit v3 6/6] server: debug: Escape debug strings

2023-05-09 Thread Richard W.M. Jones
On Tue, May 09, 2023 at 01:36:13PM -0500, Eric Blake wrote: > On Tue, May 09, 2023 at 03:51:21PM +0100, Richard W.M. Jones wrote: > > Debug strings contain all kinds of information including some under > > user control. Previously we simply sent everything to stderr, but > > this is potentially

Re: [Libguestfs] [PATCH nbdkit v3 6/6] server: debug: Escape debug strings

2023-05-09 Thread Eric Blake
On Tue, May 09, 2023 at 03:51:21PM +0100, Richard W.M. Jones wrote: > Debug strings contain all kinds of information including some under > user control. Previously we simply sent everything to stderr, but > this is potentially insecure, as well as not dealing well with > non-printable

Re: [Libguestfs] [PATCH nbdkit v3 5/6] server: debug: Fix error handling

2023-05-09 Thread Richard W.M. Jones
On Tue, May 09, 2023 at 01:25:18PM -0500, Eric Blake wrote: > On Tue, May 09, 2023 at 03:51:20PM +0100, Richard W.M. Jones wrote: > > Preserve errno even if the function fails. We need to set > > errno = err again on every exit path out of the function. > > > > Check if close_memstream failed

Re: [Libguestfs] [PATCH nbdkit v3 3/6] common/utils: Add C string quoting function

2023-05-09 Thread Richard W.M. Jones
On Tue, May 09, 2023 at 01:18:13PM -0500, Eric Blake wrote: > On Tue, May 09, 2023 at 03:51:18PM +0100, Richard W.M. Jones wrote: > > eg. { '\r', '\n' } -> { '\\', 'n', '\\', 'r' } > > --- > > common/utils/utils.h | 1 + > > common/utils/quote.c | 58

Re: [Libguestfs] [PATCH nbdkit v3 5/6] server: debug: Fix error handling

2023-05-09 Thread Eric Blake
On Tue, May 09, 2023 at 03:51:20PM +0100, Richard W.M. Jones wrote: > Preserve errno even if the function fails. We need to set > errno = err again on every exit path out of the function. > > Check if close_memstream failed and go to the error path if so. This > is most important on Windows

Re: [Libguestfs] [PATCH nbdkit v3 4/6] server: debug: Return earlier if not verbose

2023-05-09 Thread Eric Blake
On Tue, May 09, 2023 at 03:51:19PM +0100, Richard W.M. Jones wrote: > In particular return before we have allocated 'str' with > attribute((cleanup)) so that we don't waste time calling free (NULL) > on the hot-ish path out of the function. > --- > server/debug.c | 6 +++--- > 1 file changed, 3

Re: [Libguestfs] [PATCH nbdkit v3 3/6] common/utils: Add C string quoting function

2023-05-09 Thread Eric Blake
On Tue, May 09, 2023 at 03:51:18PM +0100, Richard W.M. Jones wrote: > eg. { '\r', '\n' } -> { '\\', 'n', '\\', 'r' } > --- > common/utils/utils.h | 1 + > common/utils/quote.c | 58 > 2 files changed, 59 insertions(+) > > + > +/* Print str to fp,

Re: [Libguestfs] [PATCH nbdkit v3 2/6] server: debug: Don't emit ANSI colour codes on Windows

2023-05-09 Thread Eric Blake
On Tue, May 09, 2023 at 03:51:17PM +0100, Richard W.M. Jones wrote: > At least under Wine they are not recognized and you just see literal > "[0m" etc appearing in the output. Maybe that means the problem is that isatty() under Wine is returning 1 even when it shouldn't? But avoiding the tty

[Libguestfs] [PATCH nbdkit v3 6/6] server: debug: Escape debug strings

2023-05-09 Thread Richard W.M. Jones
Debug strings contain all kinds of information including some under user control. Previously we simply sent everything to stderr, but this is potentially insecure, as well as not dealing well with non-printable characters. Escape these strings when printing. --- server/debug.c | 38

[Libguestfs] [PATCH nbdkit v3 5/6] server: debug: Fix error handling

2023-05-09 Thread Richard W.M. Jones
Preserve errno even if the function fails. We need to set errno = err again on every exit path out of the function. Check if close_memstream failed and go to the error path if so. This is most important on Windows where close_memstream is what actually allocates the memory. But even on Unix,

[Libguestfs] [PATCH nbdkit v3 3/6] common/utils: Add C string quoting function

2023-05-09 Thread Richard W.M. Jones
eg. { '\r', '\n' } -> { '\\', 'n', '\\', 'r' } --- common/utils/utils.h | 1 + common/utils/quote.c | 58 2 files changed, 59 insertions(+) diff --git a/common/utils/utils.h b/common/utils/utils.h index 42288a5cd..1e6d8972b 100644 ---

[Libguestfs] [PATCH nbdkit v3 4/6] server: debug: Return earlier if not verbose

2023-05-09 Thread Richard W.M. Jones
In particular return before we have allocated 'str' with attribute((cleanup)) so that we don't waste time calling free (NULL) on the hot-ish path out of the function. --- server/debug.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/server/debug.c b/server/debug.c index

[Libguestfs] [PATCH nbdkit v3 1/6] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-09 Thread Richard W.M. Jones
This is not secure so should not be used routinely. Also we do not attempt to filter environment variables, so even ones containing multiple lines or special characters are all sent to nbdkit_debug. The reason for adding this is to allow for debugging the new nbd_set_socket_activation_name(3)

[Libguestfs] [PATCH nbdkit v3 2/6] server: debug: Don't emit ANSI colour codes on Windows

2023-05-09 Thread Richard W.M. Jones
At least under Wine they are not recognized and you just see literal "[0m" etc appearing in the output. Updates: commit 5a011e02375912f2ad88de1ebc6d609e29d38f71 Reviewed-by: Laszlo Ersek --- server/debug.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/server/debug.c b/server/debug.c

[Libguestfs] [PATCH nbdkit v3 0/6] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-09 Thread Richard W.M. Jones
Add R-b tags. Split the last patch into 3 parts, fixing the error handling issues raised by Laszlo in the previous review. Rich. ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit v2 4/4] server: debug: Escape debug strings

2023-05-09 Thread Richard W.M. Jones
On Tue, May 09, 2023 at 03:05:41PM +0200, Laszlo Ersek wrote: [...] > Then, in the post-patch version, the following catches my eye: > > fp_inner = open_memstream (_inner, _inner); > if (fp_inner == NULL) { > fail: > /* Try to emit what we can. */ > errno = err; > vfprintf

Re: [Libguestfs] [PATCH nbdkit v2 3/4] common/utils: Add C string quoting function

2023-05-09 Thread Richard W.M. Jones
On Tue, May 09, 2023 at 02:28:40PM +0200, Laszlo Ersek wrote: > On 5/9/23 11:51, Richard W.M. Jones wrote: > > eg. { '\r', '\n' } -> { '\\', 'n', '\\', 'r' } > > --- > > common/utils/utils.h | 1 + > > common/utils/quote.c | 58 > > 2 files changed,

Re: [Libguestfs] [PATCH libnbd v2] tests: Add a test of nbd_{set, get}_socket_activation_name

2023-05-09 Thread Laszlo Ersek
On 5/9/23 11:48, Richard W.M. Jones wrote: > Also test that the expected environment variable is set when > connecting to nbdkit. > > This test requires nbdkit >= 1.35.2 which added support for -D > nbdkit.environ=1 to dump the environment, and will be skipped with > other versions. > --- >

Re: [Libguestfs] [PATCH nbdkit v2 4/4] server: debug: Escape debug strings

2023-05-09 Thread Laszlo Ersek
On 5/9/23 11:51, Richard W.M. Jones wrote: > Debug strings contain all kinds of information including some under > user control. Previously we simply sent everything to stderr, but > this is potentially insecure, as well as not dealing well with > non-printable characters. Escape these strings

Re: [Libguestfs] [PATCH nbdkit v2 3/4] common/utils: Add C string quoting function

2023-05-09 Thread Laszlo Ersek
On 5/9/23 11:51, Richard W.M. Jones wrote: > eg. { '\r', '\n' } -> { '\\', 'n', '\\', 'r' } > --- > common/utils/utils.h | 1 + > common/utils/quote.c | 58 > 2 files changed, 59 insertions(+) > > diff --git a/common/utils/utils.h

Re: [Libguestfs] [PATCH nbdkit v2 2/4] server: debug: Don't emit ANSI colour codes on Windows

2023-05-09 Thread Laszlo Ersek
On 5/9/23 11:51, Richard W.M. Jones wrote: > At least under Wine they are not recognized and you just see literal > "[0m" etc appearing in the output. > > Updates: commit 5a011e02375912f2ad88de1ebc6d609e29d38f71 > --- > server/debug.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git

Re: [Libguestfs] [PATCH nbdkit v2 1/4] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-09 Thread Laszlo Ersek
On 5/9/23 11:51, Richard W.M. Jones wrote: > This is not secure so should not be used routinely. Also we do not > attempt to filter environment variables, so even ones containing > multiple lines or special characters are all sent to nbdkit_debug. > > The reason for adding this is to allow for

[Libguestfs] [PATCH nbdkit v2 4/4] server: debug: Escape debug strings

2023-05-09 Thread Richard W.M. Jones
Debug strings contain all kinds of information including some under user control. Previously we simply sent everything to stderr, but this is potentially insecure, as well as not dealing well with non-printable characters. Escape these strings when printing. --- server/debug.c | 52

[Libguestfs] [PATCH nbdkit v2 3/4] common/utils: Add C string quoting function

2023-05-09 Thread Richard W.M. Jones
eg. { '\r', '\n' } -> { '\\', 'n', '\\', 'r' } --- common/utils/utils.h | 1 + common/utils/quote.c | 58 2 files changed, 59 insertions(+) diff --git a/common/utils/utils.h b/common/utils/utils.h index 42288a5cd..1e6d8972b 100644 ---

[Libguestfs] [PATCH nbdkit v2 0/4] Add -D nbdkit.environ=1, Windows fixes and quoting issues

2023-05-09 Thread Richard W.M. Jones
This grew a bit from last time ... The basic change to allow dumping the environment is the same, except I enabled this for Windows (and checked that it does in fact work -- Windows has environment variables, who knew?) The second patch fixes a problem I noticed while testing the above on

[Libguestfs] [PATCH nbdkit v2 1/4] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-09 Thread Richard W.M. Jones
This is not secure so should not be used routinely. Also we do not attempt to filter environment variables, so even ones containing multiple lines or special characters are all sent to nbdkit_debug. The reason for adding this is to allow for debugging the new nbd_set_socket_activation_name(3)

[Libguestfs] [PATCH nbdkit v2 2/4] server: debug: Don't emit ANSI colour codes on Windows

2023-05-09 Thread Richard W.M. Jones
At least under Wine they are not recognized and you just see literal "[0m" etc appearing in the output. Updates: commit 5a011e02375912f2ad88de1ebc6d609e29d38f71 --- server/debug.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/server/debug.c b/server/debug.c index 1b5ddaafe..2231d40a0

[Libguestfs] [PATCH libnbd v2] tests: Add a test of nbd_{set, get}_socket_activation_name

2023-05-09 Thread Richard W.M. Jones
Also test that the expected environment variable is set when connecting to nbdkit. This test requires nbdkit >= 1.35.2 which added support for -D nbdkit.environ=1 to dump the environment, and will be skipped with other versions. --- .gitignore | 1 + tests/Makefile.am

[Libguestfs] [PATCH libnbd v2] tests: Add a test of nbd_{set, get}_socket_activation_name

2023-05-09 Thread Richard W.M. Jones
I added a test of the LISTEN_FDNAMES=unknown case as well, otherwise only minor cosmetic changes from last time. Rich. ___ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs

Re: [Libguestfs] [PATCH nbdkit] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-09 Thread Richard W.M. Jones
On Tue, May 09, 2023 at 08:56:47AM +0200, Laszlo Ersek wrote: > On 5/8/23 23:33, Richard W.M. Jones wrote: > > On Mon, May 08, 2023 at 08:54:48AM +0200, Laszlo Ersek wrote: > >> On 5/7/23 12:43, Richard W.M. Jones wrote: > >>> This is not secure so should not be used routinely. Also we do not >

Re: [Libguestfs] [PATCH nbdkit] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-09 Thread Laszlo Ersek
On 5/8/23 23:52, Eric Blake wrote: > On Mon, May 08, 2023 at 10:33:25PM +0100, Richard W.M. Jones wrote: >>> >>> I was surprised not to see any assignment to the new global variable, >>> but then I found "server/debug-flags.c"; that's some serious wizardry. >> >> With the side effect, as it

Re: [Libguestfs] [PATCH nbdkit] server: Add -D nbdkit.environ=1 to dump the environment

2023-05-09 Thread Laszlo Ersek
On 5/8/23 23:33, Richard W.M. Jones wrote: > On Mon, May 08, 2023 at 08:54:48AM +0200, Laszlo Ersek wrote: >> On 5/7/23 12:43, Richard W.M. Jones wrote: >>> This is not secure so should not be used routinely. Also we do not >>> attempt to filter environment variables, so even ones containing >>>