Re: [Libguestfs] [libnbd PATCH 1/5] Fix test requirements
On Fri, May 07, 2021 at 07:10:49AM -0500, Eric Blake wrote:
On 5/6/21 6:30 AM, Martin Kletzander wrote:
> Some tests have extra dependencies that may be missing in minimal
> deployments (e.g. hexdump), some require kernel modules or support (nbd, fuse)
> and others need qemu, but were written to only work with qemu-kvm binary.
>
> All these were fixed to my best knowledge, but of course they are still not
> perfect.
>
> Signed-off-by: Martin Kletzander <mkletzan(a)redhat.com>
> ---
> +# requires_qemu
> +#
> +# Check that qemu-kvm or qemu-system-$(uname -m) works. If not, skip the test.
> +# If it does, then export the appropriate binary name in $QEMU_BINARY.
> +requires_qemu ()
> +{
> + local qemu_bin
> + for qemu_bin in "qemu-kvm" "qemu-system-$(uname -m)"; do
> + ( "$qemu_bin" --version ) </dev/null >/dev/null 2>&1
|| continue
> +
> + export QEMU_BINARY="$qemu_bin"
> + break
> + done
> +
> + if test -z "$QEMU_BINARY"; then
> + echo "$0: ‘$*’ failed with error code $?"
$* is empty in the typical case of calling 'requires_qemu' without
arguments, so you may want to reword this message.
Oh yes, I had to write this roughly 7 times because I always wrote it
into functions.sh by mistake and then ran the build rewriting it. I'll
just change it to "No usable qemu binary found", maybe attach the list
of the ones that were tried.
> + echo "$0: test prerequisite is missing or not
working"
> + exit 77
> + fi
> +}
> +
> # Tests that run under check-root should use this.
> requires_root ()
> {
> +++ b/copy/copy-block-to-nbd.sh
> @@ -23,6 +23,7 @@ set -x
>
> requires_root
> requires nbdkit --exit-with-parent --version
> +requires test -e /sys/module/nbd
Is it enough that the file exists, or do we also want to test that it
can be read? Then again, since we used requires_root, we should have
privileges to use the file if it exists.
I am not sure. I am just checking that the module exists in the kernel,
but I do not know how to reliably check that it is usable. As I wrote
in the cover letter, maybe I need to check for some capability or
permission, but I did not figure out exactly which one. In the end this
was better than nothing as it strictly eliminates tests that would
failed just because there is no nbd support in the kernel (unloaded
module).
> +++ b/fuse/test-file-mode.sh
> @@ -27,6 +27,7 @@ requires fusermount3 --version
> requires nbdkit --exit-with-parent --version
> requires cmp --version
> requires dd --version
> +requires test -e /dev/fuse
Another spot where -r might make more sense than -e?
Again, with this one I am not sure this particular file is used to do
fuse. I tried checking /sys/modules/fuse, but that did not help as it
existed in the container, but anything with fuse just failed. This was
the only one I could come up with and I would be very glad for any
better ideas.
Overall, makes sense to me.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 833 bytes)