In !30, Khem reports
that in a cross-compilation environment, nbdkit embeds the absolute
name of the cross-compiler into the resulting cc plugin, even though
running the plugin should probably be favoring the bare name 'cc' or
'gcc'.  This in turn leads to non-reproducible builds.  As the goal of
cross-compiling nbdkit is to produce a binary that behaves identically
regardless of the build environment used, this means we need to give
the user control over the defaults for CC and CFLAGS embedded into the
cc plugin.

However, instead of trying to munge the build environment variable as
suggested in that merge request, I found it cleaner to just add
additional precious variables to be set at configure time, as in:

./configure CC=/path/to/cross-compiler CC_PLUGIN_CC='ccache gcc' ...

Viewing './configure --help' now shows extra lines, such as:

  CC_PLUGIN_CC
              Value to hard-code into the cc plugin's default for CC, instead
              of $CC

Reported-by: Khem Raj
Fixes: !30


Signed-off-by: Eric Blake <eblake@redhat.com>
Message-ID: <20230818132748.1153048-1-eblake@redhat.com>
[eblake: drop doc changes, reword configure help output]
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

libvirt-ci needed yet another tweak: defaulting to installing 'awk'
everywhere broke FreeBSD (where awk is installed by default but under
a different package name).  This simplifies the container dependencies
to only pull in a package for awk when it is not already in by
default, with:

  ../libvirt-ci/bin/lcitool manifest ci/manifest.yml

using libvirt-ci @44315741 (libvirt/libvirt-ci!425

)

Signed-off-by: Eric Blake <eblake@redhat.com>

OpenSUSE Tumbleweed has decided that awk is optional in their
bare-bones image, but configure scripts built by autoconf require it.
Update our manifest to match the latest libvirt-ci change, with:

  ../libvirt-ci/bin/lcitool manifest ci/manifest.yml

using libvirt-ci @d3642d62 (libvirt/libvirt-ci!425

)

Signed-off-by: Eric Blake <eblake@redhat.com>

POSIX says <limits.h> on an XSI system must define PAGESIZE and
PAGE_SIZE as aliases to one another.  This triggered an Alpine Linux
build failure:

sparse.c:112: error: "PAGE_SIZE" redefined [-Werror]
  112 | #define PAGE_SIZE 32768
      |
In file included from /usr/include/fortify/stdlib.h:30,
                 from sparse.c:36:
/usr/include/limits.h:97: note: this is the location of the previous definition
   97 | #define PAGE_SIZE PAGESIZE
      |

The obvious fix is to rename our usage to avoid the names reserved by
POSIX (since we really want our pages to be a fixed size, even if the
platform offers a different native page size).

Signed-off-by: Eric Blake <eblake@redhat.com>

libvirt-ci has cycled out some older platforms in favor of some newer
ones.  Update our manifest to match, then regenerate the CI build
scripts with:

  ../libvirt-ci/bin/lcitool manifest ci/manifest.yml

using libvirt-ci @1219a68068

Signed-off-by: Eric Blake <eblake@redhat.com>

We share nbd-protocol.h with libnbd.  That project is taking the lead
on supporting NBD 64-bit extensions (see upstream nbd commits 36abf47d
and a9384e2f on the extension-ext-header branch[1]); it will be a
while longer before nbdkit can do so, because first we need to add a
v3 plugin protocol that lets plugins support 64-bit operations.

But in the short term, it doesn't hurt to add additional protocol
types, even if we aren't using them.

[1] https://github.com/NetworkBlockDevice/nbd/blob/extension-ext-header/doc/



Signed-off-by: Eric Blake <eblake@redhat.com>

nbd-protocol.h is shared with libnbd; pick up the changes made there
in commit befbb54814 to make it easier to manage block status
payloads, in preparation for supporting upstream NBD 64-bit
extensions.  Although support for 64-bit block status in nbdkit is
further down the road, it does not hurt to slightly rename our 32-bit
block status types in the short term.

Signed-off-by: Eric Blake <eblake@redhat.com>

Using --disable-vddk was not necessary since we already exclude !Linux
in the configure script.

Plain 'make' may be used instead of 'mingw64-make'.  The latter is a
Fedora-ism which defines various make variables like prefix, bindir,
etc, which is not necessary for nbdkit.

Thanks: Brian Carnes
See: !37

This error was temporarily disabled in commit ee8295fa with the
intent to add it back later.

Updates: commit ee8295fa

Brian Carnes authored Aug 15, 2023 and Richard W.M. Jones committed Aug 16, 2023

Also:

 - ensure partition filter only reads in full sectors at a time
 - set default sector size based on next's block_size min
 - update docs

Reviewed-by: Richard W.M. Jones <rjones@redhat.com>

Brian Carnes authored Aug 16, 2023 and Richard W.M. Jones committed Aug 16, 2023

this prepares for 512/4k sectors in a subsequent commit

Brian Carnes authored Aug 16, 2023 and Richard W.M. Jones committed Aug 16, 2023

* file plugin under windows detect 4k raw physical devices
* file plugin under windows communicate sectorsize via block_size min

Wine doesn't implement DeviceIoControl IOCTL_DISK_GET_LENGTH_INFO.
The error message that you get when you try this is quite
impenetrable, so try to improve it a little bit.

nbdkit: file[1]: error: \\.\D:: DeviceIoControl: 50

Happy with our choice of current fuzzers.

Fixes: commit a8e2648c

Brian Carnes authored Aug 13, 2023 and Richard W.M. Jones committed Aug 15, 2023

[RWMJ: Extracted parts of the original merge request here:
!37 ]

Instead of calling can_exit_with_parent, check nbdkit binary directly.

The diff across the two patches is:

  diff --git a/tests/Makefile.am b/tests/Makefile.am
  index b83514a5..cf37731f 100644
  --- a/tests/Makefile.am
  +++ b/tests/Makefile.am
  @@ -600,8 +600,6 @@ EXTRA_DIST += \
   check_LTLIBRARIES += libtest.la
   libtest_la_SOURCES = test.c test.h
   libtest_la_CFLAGS = $(WARNINGS_CFLAGS)
  -libtest_la_CPPFLAGS = -I$(top_srcdir)/common/utils
  -libtest_la_LIBADD = $(top_builddir)/common/utils/libutils.la

   # Basic connection test.
   LIBNBD_TESTS += test-connect
  diff --git a/tests/test.c b/tests/test.c
  index 6be10f12..42aa177a 100644
  --- a/tests/test.c
  +++ b/tests/test.c
  @@ -50,8 +50,6 @@
   #include <sys/wait.h>
   #endif

  -#include "exit-with-parent.h"
  -
   #include "test.h"

   #ifndef WIN32
  @@ -137,10 +135,15 @@ cleanup (void)
   int
   test_start_nbdkit (const char *arg, ...)
   {
  +  bool have_exit_with_parent;
     size_t i, len;
  -  struct test_nbdkit *kit = malloc (sizeof *kit);
  +  struct test_nbdkit *kit;
     bool exists;

  +  have_exit_with_parent =
  +    system ("nbdkit --exit-with-parent --version >/dev/null 2>&1") == 0;
  +
  +  kit = malloc (sizeof *kit);
     if (!kit) {
       perror ("malloc");
       return -1;
  @@ -171,7 +174,7 @@ test_start_nbdkit (const char *arg, ...)
       argv[i++] = kit->pidpath;
       argv[i++] = "-f";
       argv[i++] = "-v";
  -    if (can_exit_with_parent ())
  +    if (have_exit_with_parent)
         argv[i++] = "--exit-with-parent";
       argv[i++] = arg;

Fixes: commit c6299889

When compiling tests which use the test framework on macOS, we see
this error:

Undefined symbols for architecture arm64:
  "_nbdkit_debug", referenced from:
      _exit_with_parent_loop in libutils.a(libutils_la-exit-with-parent.o)
  "_nbdkit_error", referenced from:
      _exit_with_parent_loop in libutils.a(libutils_la-exit-with-parent.o)
  "_nbdkit_shutdown", referenced from:
      _exit_with_parent_loop in libutils.a(libutils_la-exit-with-parent.o)
ld: symbol(s) not found for architecture arm64

This happens because the exit-with-parent.c file in libutils.a on
macOS calls various nbdkit_* functions (and needs to, eg it must call
nbdkit_shutdown).  This is fine from the server, but doesn't work when
calling standalone test programs.

Anyway, there is a much better way to test for --exit-with-parent
functionality which I will add in the following commit.

This reverts commit c6299889.

Updates: commit 6aa8a5ea

This was broken for OCaml 4, after we added OCaml 5 support.  There
was no regression test for it.

After the prior commit, we should now work on OCaml < 5, so remove
test skips.

For OCaml 5, plugins should be linked with unix.cmxa and threads.cmxa.
In addition, in OCaml 5 these libraries are now located in
subdirectories of ocamldir so you must use -I +unix -I +threads.

For OCaml 4, plugins should only be linked with unix.cmxa.  If they
are linked with threads.cmxa then they will hang when forking because
this library adds enforcement of mutexes (ie. implements them using
pthread_mutex) and does not work correctly across fork.  (OCaml 5
always enforces mutexes and fixes the fork bug.)  Some discussion of
this bug here:
https://discuss.ocaml.org/t/fatal-error-fatal-error-during-lock-resource-deadlock-avoided/12457

Deal with this with a configure-time test, and modify how we link
examples and tests, and documentation.

After this change, with OCaml 5 ./configure should print:

  checking major version of OCaml... 5
  checking for what libraries are needed for OCaml plugins... -I +unix unix.cmxa -I +threads threads.cmxa

and for OCaml 4:

  checking major version of OCaml... 4
  checking for what libraries are needed for OCaml plugins... unix.cmxa

On OCaml 4 (only) we must also avoid using
caml_c_thread_{,un}register, since these are defined by threads.cmxa
(or rather, the C library that this pulls in).  This avoids the
following error:

  nbdkit: error: cannot open plugin './test-ocaml-plugin.so': plugins/ocaml/.libs/libnbdkitocaml.so.0: undefined symbol: caml_c_thread_unregister

This allows you to replace literal text in the manual page.  The
common use is to use this like AC_SUBST.  For example if @foo@ was
defined by ./configure you could use this in a Makefile.am:

  podwrapper.pl ... --replace "__FOO__:$(FOO)"

to replace __FOO__ in the manual page with @foo@ from a ./configure
AC_SUBST([FOO]) statement.

There was an undetected crash in OCaml plugins' .unload, where we
called caml_acquire_runtime_system.  This was only reproducible when
using the nbdkit -s option, which was only used by one test
(test-ocaml-errorcodes).

The crash didn't cause the test to fail since we don't check the exit
code of nbdkit when running it from nbd_connect_command as in this
particular test.

Looking at the OCaml runtime the crash was because for some reason
when .unload was called, we were in an unregistered thread.  OCaml
wants to allocate some thread local storage and requires you to
register each thread in the program.  If you don't do this, a common
symptom is Caml_check_caml_state crashes when it tries to read the
thread local storage, which is what happened here.  The program's main
thread is registered by default.

I am not clear why it was crashing in .unload since (with nbdkit -s)
we ought to be on the main thread (always registered), and other
callbacks which happen on the main thread (eg. .config_complete) also
call caml_acquire_runtime_system without crashing.

I don't fully understand the bug; nevertheless, it is safe and cheap
to always call caml_c_thread_register, even in the main thread (or
when we're supposed to be in the main thread) so let's do that.

When nbdkit loads a plugin, it uses dlopen, which on ELF systems will
call any constructors in the plugin.  Shortly after that, nbdkit will
call the plugin's 'plugin_init' function.

While all common systems have constructors, it is not standard, and
there's no reason to use one when we can just put the same code into
plugin_init.

There's no practical difference for plugins.

POSIX says test -a/-o are not required to work.  Use two test commands
instead.

Fixes: 32b395a7 ("Add CI for GitLab", v1.30.1)
Fixes: c09ae98f

 ("tests: Add configure --disable-libguestfs-tests flag", v1.19.11)
Signed-off-by: Eric Blake <eblake@redhat.com>

In my tests this is about 40% faster than zlib.

Fixes: commit 4c5e65c9

Map qcow2 L1/L2 table to extent information.

Refactor the code which traverses the L1/L2 table (metadata) from the
code which reads the data.

Can be used to query which events are supported in this build of the
filter.