December 2018 - Libguestfs - libguestfs List Archives

[nbdkit PATCH] connections: Don't use uninit memory on early client EOF
by Eric Blake 21 Dec '18

21 Dec '18

Fuzzing with afl found a bug where a 27 byte client sequence can cause nbdkit to report a strange error message: $ printf %s $'000\1IHAVEOPT000\6'$'000\7'$'000\1x00' | tr 0 '\0' | ./nbdkit -s memory size=1m >/dev/null nbdkit: memory: error: client exceeded maximum number of options (32) The culprit? The client is hanging up on a message boundary, so conn->recv() returns 0 for EOF instead of -1 for read error, and our code blindly continues on in a for loop (re-)parsing the leftover data from the previous option. Let's fix things to uniformly quit trying to negotiate with a client that has early EOF at a message boundary, just as we do for one that quits mid-field, with the one difference that we treat a message boundary as a warning instead of an error because a client hanging up after an option response that it didn't like (rather than sending NBD_OPT_ABORT to inform the server that it won't be negotiating further) is a surprisingly common behavior among some existing clients. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- src/connections.c | 60 ++++++++++++++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 21 deletions(-) diff --git a/src/connections.c b/src/connections.c index 58ed6b0..577f466 100644 --- a/src/connections.c +++ b/src/connections.c @@ -600,6 +600,31 @@ send_newstyle_option_reply_info_export (struct connection *conn, return 0; } +/* Sub-function during _negotiate_handshake_newstyle, to uniformly handle + * a client hanging up on a message boundary. + */ +static int __attribute__ ((format (printf, 4, 5))) +conn_recv_full (struct connection *conn, void *buf, size_t len, + const char *fmt, ...) +{ + int r = conn->recv (conn, buf, len); + va_list args; + + if (r == -1) { + va_start (args, fmt); + nbdkit_verror (fmt, args); + va_end (args); + return -1; + } + if (r == 0) { + /* During negotiation, client EOF on message boundary is less + * severe than failure in the middle of the buffer. */ + debug ("client closed input socket, closing connection"); + return -1; + } + return r; +} + /* Sub-function of _negotiate_handshake_newstyle_options below. It * must be called on all non-error paths out of the options for-loop * in that function. @@ -639,10 +664,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn) const char *optname; for (nr_options = 0; nr_options < MAX_NR_OPTIONS; ++nr_options) { - if (conn->recv (conn, &new_option, sizeof new_option) == -1) { - nbdkit_error ("read: %m"); + if (conn_recv_full (conn, &new_option, sizeof new_option, + "read: %m") == -1) return -1; - } version = be64toh (new_option.version); if (version != NEW_VERSION) { @@ -675,10 +699,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn) switch (option) { case NBD_OPT_EXPORT_NAME: - if (conn->recv (conn, data, optlen) == -1) { - nbdkit_error ("read: %s: %m", name_of_nbd_opt (option)); + if (conn_recv_full (conn, data, optlen, + "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; - } /* Apart from printing it, ignore the export name. */ data[optlen] = '\0'; debug ("newstyle negotiation: %s: " @@ -715,10 +738,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn) if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) == -1) return -1; - if (conn->recv (conn, data, optlen) == -1) { - nbdkit_error ("read: %s: %m", name_of_nbd_opt (option)); + if (conn_recv_full (conn, data, optlen, + "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; - } continue; } @@ -738,10 +760,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn) if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_INVALID) == -1) return -1; - if (conn->recv (conn, data, optlen) == -1) { - nbdkit_error ("read: %s: %m", name_of_nbd_opt (option)); + if (conn_recv_full (conn, data, optlen, + "read: %s: %m", name_of_nbd_opt (option)) == -1) return -1; - } continue; } @@ -780,10 +801,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn) case NBD_OPT_INFO: case NBD_OPT_GO: optname = name_of_nbd_opt (option); - if (conn->recv (conn, data, optlen) == -1) { - nbdkit_error ("read: %s: %m", optname); + if (conn_recv_full (conn, data, optlen, + "read: %s: %m", optname) == -1) return -1; - } if (optlen < 6) { /* 32 bit export length + 16 bit nr info */ debug ("newstyle negotiation: %s option length < 6", optname); @@ -880,10 +900,9 @@ _negotiate_handshake_newstyle_options (struct connection *conn) /* Unknown option. */ if (send_newstyle_option_reply (conn, option, NBD_REP_ERR_UNSUP) == -1) return -1; - if (conn->recv (conn, data, optlen) == -1) { - nbdkit_error ("read: %m"); + if (conn_recv_full (conn, data, optlen, + "read: %m") == -1) return -1; - } } /* Note, since it's not very clear from the protocol doc, that the @@ -931,10 +950,9 @@ _negotiate_handshake_newstyle (struct connection *conn) } /* Client now sends us its 32 bit flags word ... */ - if (conn->recv (conn, &conn->cflags, sizeof conn->cflags) == -1) { - nbdkit_error ("read: %m"); + if (conn_recv_full (conn, &conn->cflags, sizeof conn->cflags, + "read: %m") == -1) return -1; - } conn->cflags = be32toh (conn->cflags); /* ... which we check for accuracy. */ debug ("newstyle negotiation: client flags: 0x%x", conn->cflags); -- 2.17.2

2 1

why virt-v2v in-place option not support in centos 7?
by Zhang Huan 17 Dec '18

17 Dec '18

Hi All, As the in-place help said, --in-place Do not create an output virtual machine in the target hypervisor. Instead, adjust the guest OS in the source VM to run in the input hypervisor. This mode is meant for integration with other toolsets, which take the responsibility of converting the VM configuration, providing for rollback in case of errors, transforming the storage, etc. I want to do v2v modifications in the source disk image, but the v2v give the error: virt-v2v -v -x -i disk /dev/vdh --in-place virt-v2v: error: --in-place cannot be used in RHEL 7 could any one give me some help? why —in-place option disabled in RHEL7, is there a older version enabled this feture? Thanks very much for help! Best Wishes.

2 2

[PATCH nbdkit v2 0/4] tests: Test export flags (eflags).
by Richard W.M. Jones 15 Dec '18

15 Dec '18

v1 was here: https://www.redhat.com/archives/libguestfs/2018-December/thread.html#00123 v2: - Document "-" instead of "script=-" and use it in the test; and verify this also works on FreeBSD; and verify that it doesn't depend on the particular behaviour of our wrapper script and should work with installed nbdkit too. - Fix handling of zero flags parameter. - Add "to stdout" for can_fua. - "In future" -> "In the future," in various places. - Make the test work on FreeBSD by avoiding use of endian swaps. - Use socat "cool-write" to avoid occasional EPIPE error seen in tests. - Retest on Linux & FreeBSD. - Extra commit for a small documentation change in nbdkit-sh-plugin. Rich.

2 6

[PATCH nbdkit 0/3] tests: Test export flags (eflags).
by Richard W.M. Jones 15 Dec '18

15 Dec '18

Some feature additions to the shell script plugin allow us to test the export flags field reasonably easily. Rich.

2 8

[nbdkit PATCH] maint: Adjust cleaning rules
by Eric Blake 13 Dec '18

13 Dec '18

'make distcheck' calls us out for leaving files behind after 'make distclean' that were not present in the tarball. Either these files are expensive enough that end users should not be required to regenerate them (so they should be distributed), or they should be cleaned when a user asks to get back to the pristine tarball state. Automake suggests this hierarchy of cleaning: mostlyclean: .o and other obvious build artifacts clean: everything that 'make' builds without rerunning configure distclean: everything that 'configure' builds maintainerclean: things that requires special tooling to rebuild By that definition, things that are expensive to rebuild, but which are built by make and not configure, belong in distclean rather than maintainerclean (this includes the TLS temporary files, as building them consumes host entropy); and things which are not terribly expensive to rebuild can live in clean. I didn't find any of the files called out as being hard enough to regenerate to make it worth including in the tarball, which would be the only reason to keep such files in maintainerclean. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- docs/Makefile.am | 2 +- tests/Makefile.am | 30 ++++++++++++++++++------------ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/docs/Makefile.am b/docs/Makefile.am index 1772c66..6b712e3 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -123,4 +123,4 @@ filter-links.pod: $(top_srcdir)/common-rules.mk $(srcdir)/make-links.sh filter 1 $(filters) > $@-t mv $@-t $@ -MAINTAINERCLEANFILES = plugin-links.pod filter-links.pod +DISTCLEANFILES = plugin-links.pod filter-links.pod diff --git a/tests/Makefile.am b/tests/Makefile.am index 4c7b59c..55db593 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -32,7 +32,8 @@ include $(top_srcdir)/common-rules.mk -MAINTAINERCLEANFILES = +CLEANFILES = +DISTCLEANFILES = EXTRA_DIST = \ make-pki.sh \ @@ -214,9 +215,10 @@ keys.psk: $(srcdir)/make-psk.sh SRCDIR=$(srcdir) $(srcdir)/make-psk.sh # Keys are expensive to recreate so only delete them when we do -# ‘make maintainer-clean’. -MAINTAINERCLEANFILES += keys.psk -maintainer-clean-local: +# ‘make distclean’. +DISTCLEANFILES += keys.psk +distclean-local: distclean-local-tls +distclean-local-tls: rm -rf pki #---------------------------------------------------------------------- @@ -226,7 +228,7 @@ if HAVE_PLUGINS # Common data shared by multiple tests check_DATA += file-data -MAINTAINERCLEANFILES += file-data +CLEANFILES += file-data file-data: rm -f $@ $@-t for f in `$(SEQ) 1 512`; do echo -ne '\x01\x02\x03\x04\x05\x06\x07\x08'; done > $@-t @@ -286,7 +288,7 @@ endif HAVE_LIBGUESTFS # common disk image shared with several tests if HAVE_GUESTFISH check_DATA += disk -MAINTAINERCLEANFILES += disk +CLEANFILES += disk disk: rm -f $@ test1.img @@ -340,7 +342,7 @@ if HAVE_GUESTFISH LIBGUESTFS_TESTS += test-ext2 check_DATA += ext2.img -MAINTAINERCLEANFILES += ext2.img +CLEANFILES += ext2.img ext2.img: disk rm -f $@ $@-t @@ -385,7 +387,7 @@ if HAVE_GUESTFISH LIBGUESTFS_TESTS += test-gzip check_DATA += disk.gz -MAINTAINERCLEANFILES += disk.gz +CLEANFILES += disk.gz test_gzip_SOURCES = test-gzip.c test.h test_gzip_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS) @@ -454,7 +456,7 @@ test_random_LDADD = libtest.la $(LIBGUESTFS_LIBS) # split files plugin test. check_DATA += split1 split2 split3 -MAINTAINERCLEANFILES += split1 split2 split3 +CLEANFILES += split1 split2 split3 split1: file-data rm -f $@ $@-t dd if=$< of=$@-t bs=1 count=100 @@ -528,6 +530,10 @@ test-ocaml-plugin.so: test_ocaml_plugin.cmx ../plugins/ocaml/libnbdkitocaml.la . -output-obj -runtime-variant _pic -o $@ \ NBDKit.cmx $< \ -cclib -L../plugins/ocaml/.libs -cclib -lnbdkitocaml +CLEANFILES += \ + test_ocaml_plugin.cmx \ + test_ocaml_plugin.cmi \ + test-ocaml-plugin.so endif HAVE_OCAML @@ -585,7 +591,7 @@ endif HAVE_RUBY # Shell (sh) plugin test. LIBGUESTFS_TESTS += test-shell check_DATA += test-shell.img -MAINTAINERCLEANFILES += test-shell.img +CLEANFILES += test-shell.img test_shell_SOURCES = test-lang-plugins.c test.h test_shell_CFLAGS = \ @@ -732,7 +738,7 @@ TESTS += test-nozero.sh # offset filter test. check_DATA += offset-data -MAINTAINERCLEANFILES += offset-data +CLEANFILES += offset-data LIBGUESTFS_TESTS += test-offset offset-data: @@ -765,7 +771,7 @@ if HAVE_GUESTFISH LIBGUESTFS_TESTS += test-xz check_DATA += disk.xz -MAINTAINERCLEANFILES += disk.xz +CLEANFILES += disk.xz test_xz_SOURCES = test-xz.c test.h test_xz_CFLAGS = $(WARNINGS_CFLAGS) $(LIBGUESTFS_CFLAGS) -- 2.17.2

2 2

[PATCH] v2v: update docs for VMware roles (RHBZ#1530967)
by Pino Toscano 13 Dec '18

13 Dec '18

Update the list of permissions needed for VMware vCenter 6.5. --- v2v/virt-v2v-input-vmware.pod | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/v2v/virt-v2v-input-vmware.pod b/v2v/virt-v2v-input-vmware.pod index 636652e50..bc17cd913 100644 --- a/v2v/virt-v2v-input-vmware.pod +++ b/v2v/virt-v2v-input-vmware.pod @@ -546,7 +546,8 @@ write to libvirt or any other supported target. Instead of using the vCenter Administrator role, you can create a custom non-administrator role to perform the conversion. You will -however need to give it a minimum set of permissions as follows: +however need to give it a minimum set of permissions as follows +(using VMware vCenter 6.5): =over 4 @@ -566,10 +567,12 @@ Enable (check) the following objects: - Validate session Virtual Machine: + Interaction: + - Guest operating system management by VIX API Provisioning: - Allow disk access - Allow read-only disk access - - Guest Operating system management by VIX API + - Allow virtual machine download =back -- 2.17.2

2 1

[PATCH v2 2/2] v2v: Copy static IP address information over for Windows guests (RHBZ#1626503).
by Richard W.M. Jones 11 Dec '18

11 Dec '18

v1 was here with much discussion: https://www.redhat.com/archives/libguestfs/2018-December/msg00048.html v2: - Fix the case where there are multiple interfaces. Note this does not preserve order correctly (see patch for comment on why that is a hard problem). - Preserve name servers. This patch is still for discussion only. I'd like to see what might be done to get this upstream in a way that will not change existing cloud-related uses of virt-v2v. Perhaps this should only be operational with a command line flag, or triggered only for particular input and output modes. For those interested, this is what the Powershell script ends up looking like: ---------------------------------------------------------------------- Set-PSDebug -Trace 1 # Wait for the netkvm (virtio-net) driver to become active. $adapters = @() While (-Not $adapters) { Start-Sleep -Seconds 5 $adapters = (Get-NetAdapter | Where DriverFileName -eq "netkvm.sys").InterfaceAlias | Sort-Object Write-Host "adapters = '$adapters'" } New-NetIPAddress -InterfaceAlias $adapters[0] -IPAddress "10.0.2.15" -DefaultGateway "10.0.2.2" -PrefixLength 8 Set-DnsClientServerAddress -InterfaceAlias $adapters[0] -ServerAddresses ("10.0.2.3") New-NetIPAddress -InterfaceAlias $adapters[1] -IPAddress "10.0.2.16" -DefaultGateway "10.0.2.2" -PrefixLength 8 Set-DnsClientServerAddress -InterfaceAlias $adapters[1] -ServerAddresses ("10.0.2.3") ---------------------------------------------------------------------- Rich.

1 2

v2v: -o rhv-upload: Upload via NBD
by Nir Soffer 11 Dec '18

11 Dec '18

I posted this patch for vdsm, adding NBD APIs: https://gerrit.ovirt.org/c/96079/ The main purpose of these APIs are enabling incremental restore, but they also enable a more efficient rhv-upload via NBD, and importing to thin disks, which is not possible with current solution. The same idea can work for KubeVirt or other targets, minimizing specific target code. Here is how rhv-upload can work using NBD: 1. rhv-upload plugin separated to pre and post scripts - pre - prepare disk and start image transfer - post - finialize image transfer, create vm, etc. 2. rhr-upload-pre plugin create a transfer with transport="nbd" POST /imagetransfers <image_transfer> <disk id="123"/> <direction>upload</direction> <format>raw</format> <transport>nbd</transport> </image_transfer> Engine does not implement <transport> yet, but this should be an easy change. This will use the new NBD APIs to export the disk using NBD over unix socket. We can support later also NBD over TLS/PSK. Engine will return NBD url in transfer_url: <transfer_url>nbd:unix:/run/vdsm/nbd/<transfer_uuid>.sock</tansfer_url> 3. v2v use the trasfer_url to start qem-img with the NBD server: nbdkit (vddk) -> qemu-img convert -> qemu-nbd Note that nbdkit is removed from the rhv side of the pipeline. This is expected to improve the throughput significantly, since imageio is not very good with lot of small requests generated by qemu-img. 4. rhv-upload-post script invoked to complete the transfer What do you think? Nir

2 2

[nbdkit PATCH] build: Allow 'make install' into non-root --prefix: bash-completion
by Eric Blake 08 Dec '18

08 Dec '18

In general, autotooled packages are supposed to allow './configure --prefix=$HOME/subdir' as a way to then get 'make install' to run as non-root. In fact, 'make distcheck' tests that this scenario works; alas, we fail due to: /usr/bin/install -c -m 644 ../../../bash/nbdkit '/usr/share/bash-completion/completions' /usr/bin/install: cannot remove '/usr/share/bash-completion/completions/nbdkit': Permission denied The culprit? We use pkg-config to ask bash-completion where it would install user completion scripts. bash-completion.pc states: prefix=/usr completionsdir=${prefix}/share/bash-completion/completions but pkg-config --variable defaults to flattening the ${prefix} in our use of the PKG_CHECK_VAR() macro, which in turn means that './configure --prefix=$HOME/subdir' still uses an absolute path pointing to a root-owned directory (rather than one relative to our desired ${prefix}) in our definition of bashcompsdir. The solution? Tell pkg-config to NOT flatten the prefix variable. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- I'm posting this one for review, particularly since it means someone running 'make install' with a non-root --prefix will now be stuck with bash completion libraries that won't be loaded by default (the user will have to probably edit their ~/.bashrc to load things), but that still seems better than the alternative of failing to install because the installation wants to override root-owned files in spite of the non-root --prefix. 'make distcheck' still doesn't pass for me; the next failure is due to attempts to overwrite root-owned ocaml files. Similar symptoms as what this patch addressed for bashcompsdir, but there we aren't using pkg-config --variable. Instaed, our definition of ocamllibdir (based on ${OCAMLLIB} in m4/ocaml.m4) is coming from 'ocamlc -where', and I have no idea how to coerce that to print a relative pathname, or even if it is common to have ocaml libraries installed in a non-root home directory but which can still be picked up by ocaml at runtime. configure.ac | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 33ed4d5..8c97ffb 100644 --- a/configure.ac +++ b/configure.ac @@ -337,7 +337,15 @@ PKG_CHECK_MODULES([BASH_COMPLETION], [bash-completion >= 2.0], [ bash_completion=yes AC_MSG_CHECKING([for bash-completions directory]) m4_ifdef([PKG_CHECK_VAR],[ - PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir]) + dnl PKG_CHECK_VAR(bashcompdir, [bash-completion], [completionsdir]) + dnl Unfortunately, PKG_CHECK_VAR flattens references to ${prefix}, + dnl which in turn means './configure --prefix=...' to a location + dnl writable by non-root still fails when it comes to installing + dnl the bash completions. + dnl So here, we just run the same command as the macro would, but + dnl with an override for prefix to keep 'make distcheck' happy. + bashcompdir=`$PKG_CONFIG --define-variable=prefix='${prefix}' \ + --variable=completionsdir bash-completion 2>/dev/null` ]) AS_IF([test -z "$bashcompdir"], [ bashcompdir="${sysconfdir}/bash_completion.d" -- 2.17.2

2 4

[nbdkit PATCH] build: Install ocaml files relative to --prefix
by Eric Blake 07 Dec '18

07 Dec '18

Rather than always trying to install ocaml files into $(OCAMLLIBS), which is likely to be root-owned and therefore fail during a './configure --prefix=$HOME/subdir', we instead choose to always install relative to $(prefix) and let distro packagers take steps post-install to move the distro's pre-built copy into the correct location for the distro. This fixes a 'make distcheck' failure. Signed-off-by: Eric Blake <eblake(a)redhat.com> --- plugins/ocaml/Makefile.am | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/ocaml/Makefile.am b/plugins/ocaml/Makefile.am index b95f255..bbde5e1 100644 --- a/plugins/ocaml/Makefile.am +++ b/plugins/ocaml/Makefile.am @@ -39,7 +39,7 @@ EXTRA_DIST = \ if HAVE_OCAML -ocamllibdir = $(OCAMLLIB) +ocamllibdir = $(libdir)/ocaml ocamllib_DATA = NBDKit.mli NBDKit.cmi NBDKit.cmx NBDKit.o NBDKit.cmi: NBDKit.mli -- 2.17.2

2 2