November 2021 - Libguestfs - libguestfs List Archives

[PATCH v2 0/5] work around part table type misreporting by "parted"
by Laszlo Ersek 25 Nov '21

25 Nov '21

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1931821 v1: https://listman.redhat.com/archives/libguestfs/2021-November/msg00211.html In version 2, "list-filesystems" works with the bogus MBR partition table, as long as the FAT variant in use is FAT16 or FAT32. (Parted completely chokes on bogus MBR + FAT12, but that's arguably a rare corner case with libguestfs.) See also the patch-level Notes sections. Thanks, Laszlo Laszlo Ersek (5): daemon/mkfs: disable creation of fake MBR partition table with "mkfs.fat" daemon/9p: fix wrong pathname in error message daemon/parted: simplify print_partition_table() prototype daemon/parted: work around part table type misreporting by "parted" daemon/listfs: don't call "sgdisk -i" on bogus MBR partition table entry daemon/9p.c | 4 ++-- daemon/listfs.ml | 5 +++++ daemon/mkfs.c | 27 +++++++++++++++++++++++++++ daemon/parted.c | 17 ++++++----------- daemon/parted.ml | 13 ++++++++----- daemon/utils.ml | 44 ++++++++++++++++++++++++++++++++++++++++++++ daemon/utils.mli | 10 ++++++++++ 7 files changed, 102 insertions(+), 18 deletions(-) base-commit: 9fda9110e6a7afc1d418665f8f3538d87b5c8a5b -- 2.19.1.3.g30247aa5d201

2 15

[nbdkit PATCH v2 0/2] common/include/checked-overflow: provide fallback
by Laszlo Ersek 25 Nov '21

25 Nov '21

v1: https://listman.redhat.com/archives/libguestfs/2021-November/msg00245.html Fixed the buggy assert()s in the second patch; re-ran "make check". Thanks & sorry, Laszlo Laszlo Ersek (2): utils/vector: pass only unsigned arguments to (ADD|MUL)_OVERFLOW common/include/checked-overflow: provide fallback common/include/checked-overflow.h | 160 ++++++++++++++++++++++-- common/utils/Makefile.am | 8 +- common/utils/test-checked-overflow.c | 177 +++++++++++++++++++++++++++ common/utils/vector.c | 2 +- configure.ac | 6 + 5 files changed, 338 insertions(+), 15 deletions(-) create mode 100644 common/utils/test-checked-overflow.c base-commit: b9a2c12da886decf662f4e2bbcd1699a200676aa -- 2.19.1.3.g30247aa5d201

2 3

[nbdkit PATCH 0/2] common/include/checked-overflow: provide fallback
by Laszlo Ersek 25 Nov '21

25 Nov '21

The second patch is the relevant one; the first patch is just a small tweak to an existent ADD_OVERFLOW() invocation in "common/utils/vector.c" -- because we'll need to pass unsigned arguments to ADD_OVERFLOW() (and MUL_OVERFLOW()) going forward. Thanks, Laszlo Laszlo Ersek (2): utils/vector: pass only unsigned arguments to (ADD|MUL)_OVERFLOW common/include/checked-overflow: provide fallback common/include/checked-overflow.h | 160 ++++++++++++++++++++++-- common/utils/Makefile.am | 8 +- common/utils/test-checked-overflow.c | 177 +++++++++++++++++++++++++++ common/utils/vector.c | 2 +- configure.ac | 6 + 5 files changed, 338 insertions(+), 15 deletions(-) create mode 100644 common/utils/test-checked-overflow.c base-commit: b9a2c12da886decf662f4e2bbcd1699a200676aa -- 2.19.1.3.g30247aa5d201

1 3

[PATCH nbdkit 1/2] ocaml: Simplify NBDKit.set_error
by Richard W.M. Jones 24 Nov '21

24 Nov '21

Using the function code_of_unix_error from <caml/unixsupport.h> we can greatly simplify this function. This function was added in OCaml 4.01 which is ≤ 4.03 that we currently require, and also ≤ 4.02.2 that was the minimum version supported since the OCaml plugin was added in 2014. See: https://github.com/ocaml/ocaml/issues/4812 This does require a small change ot how OCaml plugins are linked -- we now need to link them with the standard OCaml Unix library (unix.cmxa). This commit also adds a comprehensive end-to-end test of error codes. --- plugins/cc/nbdkit-cc-plugin.pod | 4 +- plugins/ocaml/nbdkit-ocaml-plugin.pod | 2 +- plugins/ocaml/Makefile.am | 2 +- tests/Makefile.am | 22 ++++-- plugins/ocaml/NBDKit.ml | 25 +------ plugins/ocaml/bindings.c | 22 +----- tests/test-cc-ocaml.sh | 2 +- tests/cc_shebang.ml | 2 +- tests/test-ocaml-errorcodes.c | 97 +++++++++++++++++++++++++++ tests/test_ocaml_errorcodes_plugin.ml | 33 +++++++++ .gitignore | 1 + 11 files changed, 158 insertions(+), 54 deletions(-) diff --git a/plugins/cc/nbdkit-cc-plugin.pod b/plugins/cc/nbdkit-cc-plugin.pod index 0fe0d9ea..be4019f9 100644 --- a/plugins/cc/nbdkit-cc-plugin.pod +++ b/plugins/cc/nbdkit-cc-plugin.pod @@ -89,7 +89,7 @@ C<CC=g++> as a parameter to exec nbdkit. =head2 Using this plugin with OCaml nbdkit cc CC=ocamlopt \ - CFLAGS="-output-obj -runtime-variant _pic NBDKit.cmx -cclib -lnbdkitocaml" \ + CFLAGS="-output-obj -runtime-variant _pic unix.cmxa NBDKit.cmx -cclib -lnbdkitocaml" \ source.ml OCaml plugin scripts can be created using this trick: @@ -97,7 +97,7 @@ OCaml plugin scripts can be created using this trick: (*/.)>/dev/null 2>&1 exec nbdkit cc "$0" \ CC=ocamlopt \ - CFLAGS="-output-obj -runtime-variant _pic NBDKit.cmx -cclib -lnbdkitocaml" \ + CFLAGS="-output-obj -runtime-variant _pic unix.cmxa NBDKit.cmx -cclib -lnbdkitocaml" \ "$@" *) (* followed by OCaml code for the plugin here *) diff --git a/plugins/ocaml/nbdkit-ocaml-plugin.pod b/plugins/ocaml/nbdkit-ocaml-plugin.pod index 293f8143..efeb2240 100644 --- a/plugins/ocaml/nbdkit-ocaml-plugin.pod +++ b/plugins/ocaml/nbdkit-ocaml-plugin.pod @@ -53,7 +53,7 @@ using this command: ocamlopt.opt -output-obj -runtime-variant _pic \ -o nbdkit-myplugin-plugin.so \ - NBDKit.cmx myplugin.ml \ + unix.cmxa NBDKit.cmx myplugin.ml \ -cclib -lnbdkitocaml You can then use C<nbdkit-myplugin-plugin.so> as an nbdkit plugin (see diff --git a/plugins/ocaml/Makefile.am b/plugins/ocaml/Makefile.am index 1082fc0a..fcf3396d 100644 --- a/plugins/ocaml/Makefile.am +++ b/plugins/ocaml/Makefile.am @@ -81,7 +81,7 @@ noinst_SCRIPTS = nbdkit-ocamlexample-plugin.so nbdkit-ocamlexample-plugin.so: example.cmx libnbdkitocaml.la NBDKit.cmi NBDKit.cmx $(OCAMLOPT) $(OCAMLOPTFLAGS) \ -output-obj -runtime-variant _pic -o $@ \ - NBDKit.cmx $< \ + unix.cmxa NBDKit.cmx $< \ -cclib -L.libs -cclib -lnbdkitocaml example.cmx: example.ml NBDKit.cmi NBDKit.cmx $(OCAMLOPT) $(OCAMLOPTFLAGS) -c $< -o $@ diff --git a/tests/Makefile.am b/tests/Makefile.am index 4f63f9ab..20f5b06a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -1065,9 +1065,8 @@ EXTRA_DIST += test-zero.sh if HAVE_OCAML LIBGUESTFS_TESTS += test-ocaml +LIBNBD_TESTS += test-ocaml-errorcodes -# This is somewhat different from the other tests because we have -# to build an actual plugin here. test_ocaml_SOURCES = test-ocaml.c test.h test_ocaml_CFLAGS = \ $(WARNINGS_CFLAGS) \ @@ -1075,15 +1074,30 @@ test_ocaml_CFLAGS = \ $(NULL) test_ocaml_LDADD = libtest.la $(LIBGUESTFS_LIBS) -check_SCRIPTS += test-ocaml-plugin.so +test_ocaml_errorcodes_SOURCES = test-ocaml-errorcodes.c +test_ocaml_errorcodes_CFLAGS = $(WARNINGS_CFLAGS) $(LIBNBD_CFLAGS) +test_ocaml_errorcodes_LDADD = $(LIBNBD_LIBS) + +check_SCRIPTS += \ + test-ocaml-plugin.so \ + test-ocaml-errorcodes-plugin.so + test-ocaml-plugin.so: test_ocaml_plugin.cmx ../plugins/ocaml/libnbdkitocaml.la ../plugins/ocaml/NBDKit.cmi ../plugins/ocaml/NBDKit.cmx $(OCAMLOPT) $(OCAMLOPTFLAGS) -I ../plugins/ocaml \ -output-obj -runtime-variant _pic -o $@ \ - NBDKit.cmx $< \ + unix.cmxa NBDKit.cmx $< \ -cclib -L../plugins/ocaml/.libs -cclib -lnbdkitocaml test_ocaml_plugin.cmx: test_ocaml_plugin.ml ../plugins/ocaml/NBDKit.cmi ../plugins/ocaml/NBDKit.cmx $(OCAMLOPT) $(OCAMLOPTFLAGS) -I ../plugins/ocaml -c $< -o $@ +test-ocaml-errorcodes-plugin.so: test_ocaml_errorcodes_plugin.cmx ../plugins/ocaml/libnbdkitocaml.la ../plugins/ocaml/NBDKit.cmi ../plugins/ocaml/NBDKit.cmx + $(OCAMLOPT) $(OCAMLOPTFLAGS) -I ../plugins/ocaml \ + -output-obj -runtime-variant _pic -o $@ \ + unix.cmxa NBDKit.cmx $< \ + -cclib -L../plugins/ocaml/.libs -cclib -lnbdkitocaml +test_ocaml_errorcodes_plugin.cmx: test_ocaml_errorcodes_plugin.ml ../plugins/ocaml/NBDKit.cmi ../plugins/ocaml/NBDKit.cmx + $(OCAMLOPT) $(OCAMLOPTFLAGS) -I ../plugins/ocaml -c $< -o $@ + endif HAVE_OCAML EXTRA_DIST += \ diff --git a/plugins/ocaml/NBDKit.ml b/plugins/ocaml/NBDKit.ml index c9ce31b5..d28992c8 100644 --- a/plugins/ocaml/NBDKit.ml +++ b/plugins/ocaml/NBDKit.ml @@ -220,30 +220,7 @@ let register_plugin plugin = (* Bindings to nbdkit server functions. *) -external _set_error : int -> unit = "ocaml_nbdkit_set_error" [@@noalloc] - -let set_error unix_error = - (* There's an awkward triple translation going on here, because - * OCaml Unix.error codes, errno on the host system, and NBD_* - * errnos are not all the same integer value. Plus we cannot - * read the host system errno values from OCaml. - *) - let nbd_error = - match unix_error with - | Unix.EPERM -> 1 - | Unix.EIO -> 2 - | Unix.ENOMEM -> 3 - | Unix.EINVAL -> 4 - | Unix.ENOSPC -> 5 - | Unix.ESHUTDOWN -> 6 - | Unix.EOVERFLOW -> 7 - | Unix.EOPNOTSUPP -> 8 - | Unix.EROFS -> 9 - | Unix.EFBIG -> 10 - | _ -> 4 (* EINVAL *) in - - _set_error nbd_error - +external set_error : Unix.error -> unit = "ocaml_nbdkit_set_error" [@@noalloc] external parse_size : string -> int64 = "ocaml_nbdkit_parse_size" external parse_bool : string -> bool = "ocaml_nbdkit_parse_bool" external read_password : string -> string = "ocaml_nbdkit_read_password" diff --git a/plugins/ocaml/bindings.c b/plugins/ocaml/bindings.c index a6d57084..ba95fb4a 100644 --- a/plugins/ocaml/bindings.c +++ b/plugins/ocaml/bindings.c @@ -42,6 +42,7 @@ #include <caml/memory.h> #include <caml/mlvalues.h> #include <caml/threads.h> +#include <caml/unixsupport.h> #define NBDKIT_API_VERSION 2 #include <nbdkit-plugin.h> @@ -54,26 +55,7 @@ NBDKIT_DLL_PUBLIC value ocaml_nbdkit_set_error (value nv) { - int err; - - switch (Int_val (nv)) { - /* Host errno values that will map to NBD protocol values */ - case 1: err = EPERM; break; - case 2: err = EIO; break; - case 3: err = ENOMEM; break; - case 4: err = EINVAL; break; - case 5: err = ENOSPC; break; - case 6: err = ESHUTDOWN; break; - case 7: err = EOVERFLOW; break; - case 8: err = EOPNOTSUPP; break; - /* Other errno values that server/protocol.c treats specially */ - case 9: err = EROFS; break; - case 10: err = EFBIG; break; - default: abort (); - } - - nbdkit_set_error (err); - + nbdkit_set_error (code_of_unix_error (nv)); return Val_unit; } diff --git a/tests/test-cc-ocaml.sh b/tests/test-cc-ocaml.sh index e47bf26f..9ba7ee98 100755 --- a/tests/test-cc-ocaml.sh +++ b/tests/test-cc-ocaml.sh @@ -58,6 +58,6 @@ cleanup_fn rm -f $out rm -f $out nbdkit -U - cc $script a=1 b=2 c=3 \ - CC="$OCAMLOPT" CFLAGS="-output-obj -runtime-variant _pic -I $SRCDIR/../plugins/ocaml NBDKit.cmx -cclib -lnbdkitocaml" \ + CC="$OCAMLOPT" CFLAGS="-output-obj -runtime-variant _pic -I $SRCDIR/../plugins/ocaml unix.cmxa NBDKit.cmx -cclib -lnbdkitocaml" \ --run 'nbdinfo --size $uri' > $out test "$(cat $out)" -eq $((512 * 2048)) diff --git a/tests/cc_shebang.ml b/tests/cc_shebang.ml index 05036284..a6c79039 100755 --- a/tests/cc_shebang.ml +++ b/tests/cc_shebang.ml @@ -4,7 +4,7 @@ # shell as an impossible command which is ignored. The line below is # run by the shell and ignored by OCaml. -exec nbdkit cc "$0" CC=ocamlopt CFLAGS="-output-obj -runtime-variant _pic NBDKit.cmx -cclib -lnbdkitocaml" "$@" +exec nbdkit cc "$0" CC=ocamlopt CFLAGS="-output-obj -runtime-variant _pic unix.cmxa NBDKit.cmx -cclib -lnbdkitocaml" "$@" *) open Printf diff --git a/tests/test-ocaml-errorcodes.c b/tests/test-ocaml-errorcodes.c new file mode 100644 index 00000000..25397f48 --- /dev/null +++ b/tests/test-ocaml-errorcodes.c @@ -0,0 +1,97 @@ +/* nbdkit + * Copyright (C) 2013-2021 Red Hat Inc. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * + * * Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * * Neither the name of Red Hat nor the names of its contributors may be + * used to endorse or promote products derived from this software without + * specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* See also test_ocaml_errorcodes_plugin.ml */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <errno.h> +#include <assert.h> + +#include <libnbd.h> + +int +main (int argc, char *argv[]) +{ + struct nbd_handle *nbd; + char buf[512]; + + nbd = nbd_create (); + if (nbd == NULL) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + char *args[] = { + "nbdkit", "-s", "--exit-with-parent", + "./test-ocaml-errorcodes-plugin.so", NULL + }; + if (nbd_connect_command (nbd, args) == -1) { + fprintf (stderr, "%s\n", nbd_get_error ()); + exit (EXIT_FAILURE); + } + + /* Reading at various sector offsets in this plugin should produce + * predictable error codes. + */ + assert (nbd_pread (nbd, buf, 512, 0, 0) == 0); + + assert (nbd_pread (nbd, buf, 512, 1*512, 0) == -1); + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ()); + fflush (stdout); + assert (nbd_get_errno () == EPERM); + + assert (nbd_pread (nbd, buf, 512, 2*512, 0) == -1); + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ()); + fflush (stdout); + assert (nbd_get_errno () == EIO); + + assert (nbd_pread (nbd, buf, 512, 3*512, 0) == -1); + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ()); + fflush (stdout); + assert (nbd_get_errno () == ENOMEM); + + assert (nbd_pread (nbd, buf, 512, 4*512, 0) == -1); + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ()); + fflush (stdout); + assert (nbd_get_errno () == ESHUTDOWN); + + assert (nbd_pread (nbd, buf, 512, 5*512, 0) == -1); + printf ("%s %d\n", nbd_get_error (), nbd_get_errno ()); + fflush (stdout); + assert (nbd_get_errno () == EINVAL); + + nbd_close (nbd); + exit (EXIT_SUCCESS); +} diff --git a/tests/test_ocaml_errorcodes_plugin.ml b/tests/test_ocaml_errorcodes_plugin.ml new file mode 100644 index 00000000..6a628794 --- /dev/null +++ b/tests/test_ocaml_errorcodes_plugin.ml @@ -0,0 +1,33 @@ +open Unix + +let open_connection _ = () + +let get_size () = Int64.of_int (6 * 512) + +let pread () count offset _ = + let count = Int32.to_int count + and offset = Int64.to_int offset in + + (* Depending on the sector requested (offset), return a different + * error code. + *) + match offset / 512 with + | 0 -> (* good, return data *) String.make count '\000' + | 1 -> NBDKit.set_error EPERM; failwith "EPERM" + | 2 -> NBDKit.set_error EIO; failwith "EIO" + | 3 -> NBDKit.set_error ENOMEM; failwith "ENOMEM" + | 4 -> NBDKit.set_error ESHUTDOWN; failwith "ESHUTDOWN" + | 5 -> NBDKit.set_error EINVAL; failwith "EINVAL" + | _ -> assert false + +let plugin = { + NBDKit.default_callbacks with + NBDKit.name = "test-ocaml-errorcodes"; + version = NBDKit.version (); + + open_connection = Some open_connection; + get_size = Some get_size; + pread = Some pread; +} + +let () = NBDKit.register_plugin plugin diff --git a/.gitignore b/.gitignore index 91cbc810..4e2ae75d 100644 --- a/.gitignore +++ b/.gitignore @@ -146,6 +146,7 @@ plugins/*/*.3 /tests/test-nbd /tests/test-null /tests/test-ocaml +/tests/test-ocaml-errorcodes /tests/test-offset /tests/test-oldstyle /tests/test-old-plugins-*.sh -- 2.32.0

3 22

[PATCH] daemon/mkfs: disable creation of fake MBR partition table with "mkfs.fat"
by Laszlo Ersek 23 Nov '21

23 Nov '21

Search the usage output of "mkfs.fat" for "--mbr[="; cache the result for further invocations. If the option is supported, pass "--mbr=n" to "mkfs.fat". This will prevent the creation of a bogus partition table whose first (and only) entry describes a partition that contains the partition table. (Such a bogus partition table breaks "parted", which is a tool used by libguestfs extensively, both internally and in public libguestfs APIs.) Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1931821 Signed-off-by: Laszlo Ersek <lersek(a)redhat.com> --- Notes: Testing (per <https://bugzilla.redhat.com/show_bug.cgi?id=1931821#c0>): $ virt-make-fs -vx --format=raw --size=200M --type=vfat --label=TEST \ website ~/test_vfat.img > [...] > creating vfat filesystem on /dev/sda ... > libguestfs: trace: mkfs "vfat" "/dev/sda" "label:TEST" > [...] > commandrvf: stdout=n stderr=y flags=0x0 > commandrvf: mkfs.fat > Usage: mkfs.fat [OPTIONS] TARGET [BLOCKS] > Create FAT filesystem in TARGET, which can be a block device or file. Use only > up to BLOCKS 1024 byte blocks if specified. With the -C option, file TARGET will be > created with a size of 1024 bytes times BLOCKS, which must be specified. > > Options: > -a Disable alignment of data structures > -A Toggle Atari variant of the filesystem > -b SECTOR Select SECTOR as location of the FAT32 backup boot sector > -c Check device for bad blocks before creating the filesystem > -C Create file TARGET then create filesystem in it > -D NUMBER Write BIOS drive number NUMBER to boot sector > -f COUNT Create COUNT file allocation tables > -F SIZE Select FAT size SIZE (12, 16 or 32) > -g GEOM Select disk geometry: heads/sectors_per_track > -h NUMBER Write hidden sectors NUMBER to boot sector > -i VOLID Set volume ID to VOLID (a 32 bit hexadecimal number) > -I Ignore and disable safety checks > -l FILENAME Read bad blocks list from FILENAME > -m FILENAME Replace default error message in boot block with contents of FILENAME > -M TYPE Set media type in boot sector to TYPE > --mbr[=y|n|a] Fill (fake) MBR table with one partition which spans whole disk > -n LABEL Set volume name to LABEL (up to 11 characters long) > --codepage=N use DOS codepage N to encode label (default: 850) > -r COUNT Make room for at least COUNT entries in the root directory > -R COUNT Set minimal number of reserved sectors to COUNT > -s COUNT Set number of sectors per cluster to COUNT > -S SIZE Select a sector size of SIZE (a power of two, at least 512) > -v Verbose execution > --variant=TYPE Select variant TYPE of filesystem (standard or Atari) > > --invariant Use constants for randomly generated or time based values > --offset=SECTOR Write the filesystem at a specific sector into the device file. > --help Show this help message and exit > [...] > commandrvf: stdout=n stderr=y flags=0x0 > commandrvf: mkfs -t vfat -I --mbr=n -n TEST /dev/sda $ guestfish -a ~/test_vfat.img > ><fs> run > ><fs> list-filesystems > /dev/sda: vfat daemon/mkfs.c | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/daemon/mkfs.c b/daemon/mkfs.c index ba90cef2111c..4cb216cabe8e 100644 --- a/daemon/mkfs.c +++ b/daemon/mkfs.c @@ -30,6 +30,14 @@ #define MAX_ARGS 64 +enum fat_mbr_option { + FMO_UNCHECKED, + FMO_DOESNT_EXIST, + FMO_EXISTS, +}; + +static enum fat_mbr_option fat_mbr_option = FMO_UNCHECKED; + /* Takes optional arguments, consult optargs_bitmask. */ int do_mkfs (const char *fstype, const char *device, int blocksize, @@ -101,6 +109,25 @@ do_mkfs (const char *fstype, const char *device, int blocksize, STREQ (fstype, "msdos")) ADD_ARG (argv, i, "-I"); + /* Prevent mkfs.fat from creating a bogus partition table (RHBZ#1931821). */ + if (STREQ (fstype, "fat") || STREQ (fstype, "vfat") || + STREQ (fstype, "msdos")) { + if (fat_mbr_option == FMO_UNCHECKED) { + CLEANUP_FREE char *usage_err = NULL; + + fat_mbr_option = FMO_DOESNT_EXIST; + /* Invoking either version 3 of version 4 of mkfs.fat without any options + * will make it (a) print a usage summary to stderr, (b) exit with status + * 1. + */ + r = commandr (NULL, &usage_err, "mkfs.fat", (char *)NULL); + if (r == 1 && strstr (usage_err, "--mbr[=") != NULL) + fat_mbr_option = FMO_EXISTS; + } + if (fat_mbr_option == FMO_EXISTS) + ADD_ARG (argv, i, "--mbr=n"); + } + /* Process blocksize parameter if set. */ if (optargs_bitmask & GUESTFS_MKFS_BLOCKSIZE_BITMASK) { if (blocksize <= 0 || !is_power_of_2 (blocksize)) { -- 2.19.1.3.g30247aa5d201

2 1

[PATCH] xfs: Document lazy-counters setting cannot be changed in XFS version 5
by Richard W.M. Jones 23 Nov '21

23 Nov '21

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2024022 --- generator/actions_core.ml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/generator/actions_core.ml b/generator/actions_core.ml index 5933282dcf..226fb860a0 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -7665,7 +7665,10 @@ can modify parameters. Some of the parameters of a mounted filesystem can be examined and modified using the C<guestfs_xfs_info> and -C<guestfs_xfs_growfs> calls." }; +C<guestfs_xfs_growfs> calls. + +Beginning with XFS version 5, it is no longer possible to modify +the lazy-counters setting (ie. C<lazycounter> parameter has no effect)." }; { defaults with name = "xfs_repair"; added = (1, 19, 36); -- 2.32.0

2 3

specifying a standard VGA video controller in OVF for oVirt
by Laszlo Ersek 18 Nov '21

18 Nov '21

Hi! Please make your email reader's window as wide as possible, for reading this email. This email is long, but it does contain an actual question. Watch out for the question mark please. I'm almost ready to post version 1 of the virt-v2v patch set for RHBZ#1961107. The purpose of this BZ is that virt-v2v generate a standard VGA video controller in *all* output (= converted) domains, replacing both Cirrus and QXL. The argument for VGA is captured in both the BZ and the commit messages of my patches. The missing piece is how to populate the OVF xml fragment that oVirt gets from virt-v2v. (0) For reference, this is the virt-v2v snippet that produces the fragment (file "lib/create_ovf.ml" at commit ab55f9432e77): 678 (* We always add a qxl device when outputting to RHV. 679 * See RHBZ#1213701 and RHBZ#1211231 for the reasoning 680 * behind that. 681 *) 682 let qxl_resourcetype = 683 match ovf_flavour with 684 | OVirt -> 32768 (* RHBZ#1598715 *) 685 | RHVExportStorageDomain -> 20 in 686 e "Item" [] [ 687 e "rasd:Caption" [] [PCData "Graphical Controller"]; 688 e "rasd:InstanceId" [] [PCData (uuidgen ())]; 689 e "rasd:ResourceType" [] [PCData (string_of_int qxl_resourcetype)]; 690 e "Type" [] [PCData "video"]; 691 e "rasd:VirtualQuantity" [] [PCData "1"]; 692 e "rasd:Device" [] [PCData "qxl"]; 693 ] Note that the *only* reference to QXL is on the last line, in the <rasd:Device> element. The "qxl_resourcetype" variable *looks* like it is related to QXL; in fact, it is not. It is instead the generic identifier for the *monitor* (not video controller) resource type in oVirt. In oVirt, there is some compat stuff around this (more on that later); for now, suffice it to say that the values 32768 and 20 are *both* independent of QXL (and standard VGA). The OVF fragment is parsed by oVirt in the following code snippets. This is my first encounter with the ovirt-engine repository, so please bear with me. - Repository: https://github.com/oVirt/ovirt-engine.git - Branch: master (at commit daca2ca6cd91) (1) In file "backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/utils/VmDeviceType.java", we have the following enum definitions (excerpt): 3 public enum VmDeviceType { 4 FLOPPY("floppy", "14"), 5 DISK("disk", "17"), 6 LUN("lun"), 7 CDROM("cdrom", "15"), 8 INTERFACE("interface", "10"), 9 BRIDGE("bridge", "3"), 10 VIDEO("video", "20"), 11 USB("usb", "23"), 12 CONTROLLER("controller", "23"), 13 REDIR("redir", "23"), 14 SPICEVMC("spicevmc", "23"), 15 QXL("qxl"), 16 CIRRUS("cirrus"), 17 VGA("vga"), 18 SPICE("spice"), 19 VNC("vnc"), 20 SOUND("sound"), 21 ICH6("ich6"), 22 AC97("ac97"), 23 MEMBALLOON("memballoon"), 24 CHANNEL("channel"), 25 SMARTCARD("smartcard"), 26 BALLOON("balloon"), 27 CONSOLE("console"), 28 VIRTIO("virtio"), 29 WATCHDOG("watchdog"), 30 VIRTIOSCSI("virtio-scsi"), 31 VIRTIOSERIAL("virtio-serial"), 32 HOST_DEVICE("hostdev"), 33 MEMORY("memory"), 34 PCI("pci"), 35 IDE("ide"), 36 SATA("sata"), 37 ICH9("ich9"), 38 TPM("tpm"), 39 BOCHS("bochs"), 40 OTHER("other", "0"), 41 UNKNOWN("unknown", "-1"); 42 43 private String name; 44 private String ovfResourceType; 45 46 VmDeviceType(String name) { 47 this.name = name; 48 } 49 50 VmDeviceType(String name, String ovfResourceType) { 51 this.name = name; 52 this.ovfResourceType = ovfResourceType; 53 } 54 55 public String getName() { 56 return name; 57 } 58 59 /** 60 * This method maps OVF Resource Types to oVirt devices. 61 */ 62 public static VmDeviceType getoVirtDevice(int resourceType) { 63 for (VmDeviceType deviceType : values()) { 64 if (deviceType.ovfResourceType != null && Integer.parseInt(deviceType.ovfResourceType) == resourceType) { 65 return deviceType; 66 } 67 } 68 return UNKNOWN; 69 } (2) In file "backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/businessentities/DisplayType.java", we have the following enum definitions (excerpt): 5 public enum DisplayType { 6 @Deprecated 7 cirrus(VmDeviceType.CIRRUS), 8 qxl(VmDeviceType.QXL), 9 vga(VmDeviceType.VGA), 10 bochs(VmDeviceType.BOCHS), 11 none(null); // For Headless VM, this type means that the VM will run without any display (VIDEO) device (3) In file "backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfHardware.java", we have the following resource type constants (excerpt): 10 /** The OVF specification does not include Monitor devices, 11 * so we use a constant that is supposed to be unused */ 12 public static final String Monitor = "32768"; 13 /** We must keep using '20' for oVirt's OVFs for backward/forward compatibility */ 14 public static final String OVIRT_Monitor = "20"; This is related to my above note about "qxl_resourcetype" in the virt-v2v OCaml source code. And: 16 public static final String Graphics = "24"; 17 /** In the OVF specification 24 should be used for Graphic devices, but we 18 * must keep using '26' for oVirt's OVFs for backward/forward compatibility */ 19 public static final String OVIRT_Graphics = "26"; However, this resource type seems mostly unused in oVirt, and therefore irrelevant for this discussion. I'm including these enum constants just so I can point that out! (4) In file "backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfOvirtReader.java", we have the following override (= specialization) logic: 365 protected String adjustHardwareResourceType(String resourceType) { 366 switch(resourceType) { 367 case OvfHardware.OVIRT_Monitor: 368 return OvfHardware.Monitor; This is what implements the compatibility resource type mapping discussed above. Again, it is *unrelated* to QXL vs. VGA. 369 case OvfHardware.OVIRT_Graphics: 370 return OvfHardware.Graphics; And, again, this is irrelevant here; including it just for completeness. 371 default: 372 return super.adjustHardwareResourceType(resourceType); 373 } 374 } (5) In file "backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfProperties.java", we many (symbolic) constants for various XML element names in the OVF format; such as (excerpt): 5 String VMD_DEVICE = "Device"; 6 String VMD_TYPE = "Type"; 12 String VMD_RESOURCE_TYPE = "rasd:ResourceType"; 13 String VMD_SUB_RESOURCE_TYPE = "rasd:ResourceSubType"; 14 String VMD_VIRTUAL_QUANTITY = "rasd:VirtualQuantity"; 22 String VMD_ID = "rasd:InstanceId"; Note that I selected mostly those that cover the OVF snippet generated by virt-v2v, with the code quoted at the top, under bullet (0). (6) The main oVirt logic that's relevant to us now is in file "backend/manager/modules/utils/src/main/java/org/ovirt/engine/core/utils/ovf/OvfReader.java": 851 private void setDeviceByResource(XmlNode node, VmDevice vmDevice) { 852 String resourceType = selectSingleNode(node, VMD_RESOURCE_TYPE, _xmlNS).innerText; 853 XmlNode resourceSubTypeNode = selectSingleNode(node, VMD_SUB_RESOURCE_TYPE, _xmlNS); 854 if (resourceSubTypeNode == null) { 855 // we need special handling for Monitor to define it as vnc or spice 856 if (OvfHardware.Monitor.equals(adjustHardwareResourceType(resourceType))) { 857 // get number of monitors from VirtualQuantity in OVF 858 if (selectSingleNode(node, VMD_VIRTUAL_QUANTITY, _xmlNS) != null 859 && !StringUtils.isEmpty(selectSingleNode(node, 860 VMD_VIRTUAL_QUANTITY, 861 _xmlNS).innerText)) { 862 int virtualQuantity = 863 Integer.parseInt( 864 selectSingleNode(node, VMD_VIRTUAL_QUANTITY, _xmlNS).innerText); 865 if (virtualQuantity > 1) { 866 vmDevice.setDevice(VmDeviceType.QXL.getName()); 867 } else { 868 // get first supported display device 869 List<Pair<GraphicsType, DisplayType>> supportedGraphicsAndDisplays = 870 osRepository.getGraphicsAndDisplays(vmBase.getOsId(), new Version(getVersion())); 871 if (!supportedGraphicsAndDisplays.isEmpty()) { 872 DisplayType firstDisplayType = supportedGraphicsAndDisplays.get(0).getSecond(); 873 vmDevice.setDevice(firstDisplayType.getDefaultVmDeviceType().getName()); 874 } else { 875 vmDevice.setDevice(VmDeviceType.QXL.getName()); 876 } 877 } 878 } else { // default to spice if quantity not found 879 vmDevice.setDevice(VmDeviceType.QXL.getName()); 880 } 881 } else { 882 vmDevice.setDevice(VmDeviceType.getoVirtDevice(Integer.parseInt(resourceType)).getName()); 883 } 884 } else if (OvfHardware.Network.equals(resourceType)) { Here's what it does, in English (as far as I can tell): - If the <rasd:ResourceSubType> element is *present*, then we move to line 884. Not interesting for our case. - On line 856, the monitor resource type adjustment occurs that I described in bullets (3) and (4). What I want to point out here is that the snippet that virt-v2v generates satisfies this -- in other words, virt-v2v provides a "monitor" resource --, but otherwise it has no bearing on QXL vs. VGA. - Next, the <rasd:VirtualQuantity> element is consulted. If this element does not exist, or is empty, then oVirt picks QXL (line 879). If the number of displays is larger than 1 (= multi-head setup), the same happens (line 866). In our case, neither case matches, as virt-v2v generates exactly 1 display head (see bullet (0), line 691). So further conditions are checked, below. - Next, if "OS info" knows anything about the converted OS's supported video controllers, then the first such controller is picked (line 873). Otherwise, oVirt picks QXL (line 875). - Importantly, line 882 would be reachable if virt-v2v *did not* provide a "monitor" resource at all. However, even in that case, we could not express VGA. That's because the getoVirtDevice() method -- see bullet (1), line 62 -- would have to locate the VGA entry of enum VmDeviceType, and that entry's "ovfResourceType" field is null. See line 17 under bullet (1). Ultimately, the "rasd:Device" (VMD_DEVICE) element generated by virt-v2v, with contents "qxl" (see bullet (0) line 692), plays *no role* at all, and the current oVirt logic does not allow virt-v2v to choose VGA in any way. The "qxl_resourcetype" selection in virt-v2v only activates the "monitor" resource handling, but does not affect the video controller. Here's my proposal (or more precisely, request): Can the oVirt developers please enhance the setDeviceByResource() method in "OvfReader.java" such that it parse VMD_DEVICE as well, and if that one says "vga", then the method please pick "VmDeviceType.VGA"? Because, in this case, the virt-v2v change for me would be relatively easy: - I'd rename the "qxl_resourcetype" variable to "monitor_resourcetype" -- this misnomer should in fact be fixed regardless of the QXL->VGA replacement; - I'd change the contents of the <rasd:Device> element from "qxl" to "vga", and remove the QXL-related comment too. (This would likely mean the insertion of two small patches into my virt-v2v series.) Thank you, Laszlo

3 4

[PATCH nbdkit v2 0/2] ocaml: Simplify NBDKit.set_error
by Richard W.M. Jones 18 Nov '21

18 Nov '21

v1 was here: https://listman.redhat.com/archives/libguestfs/2021-November/msg00176.html This version addresses all or very nearly all of the feedback. I already posted v2 1/2, but including it again here since it is necessary for the 2/2 to make sense. Rich.

3 5

[PATCH libnbd 0/6] Enhance synch-parallel test
by Nir Soffer 15 Nov '21

15 Nov '21

I'm working on an application using the sync API. In my tests I see best read throughput with 2 nbd connections which is unexpected. The best way to investigate this is a benchmark using various combinations of request size and number of connections. I found that tests/synch-parallel already does mostly what I need. This series fixes this test and enhances it to make request size and number of connections configurable. Additional features I plan to add: - Configurable run time - for benchmarking, running 10 seconds is too fast. When running on a laptop, the first benchmark is usually faster since the CPU take time to heat and when it heats (I see 96c in my laptop) it slows down. We need to run 30 or 60 seconds for more consistent results. For CI, 10 seconds is way too slow, running for 0.1 seconds is enough to ensure the program still runs. - Configurable I/O type - (read, write, read-write). Testing manually show higher throughput for read-only test (3.9g/s) compared with 1.1g/s read and 1.1g/s write when reading and writing. Issues: - The test scripts is much slower now (120 seconds instead of 10). We need to to separate the benchmark, running many combinations (synch-parallel-bench.sh) and the test, running one combination (sync-parallel.sh). - tests/aio-parallel test seems to have the same issues and needs similar changes. Nir Soffer (6): tests/synch-parallel: Show thread run time tests/synch-parallel: Fix request loop time limit tests/synch-parallel: Remove unneeded memcpy tests/synch-parallel: Show throughput in MiB/s tests/synch-parallel: Test multiple request sizes tests/synch-parallel: Test multiple number of connections tests/synch-parallel.c | 121 +++++++++++++++++++++++++++++----------- tests/synch-parallel.sh | 17 ++++-- 2 files changed, 100 insertions(+), 38 deletions(-) -- 2.31.1

2 8

libguestfs build failure
by Laszlo Ersek 12 Nov '21

12 Nov '21

Hi, commit b536c61a6df3 ("m4: Remove test for OCaml Bytes module", 2021-11-09) removed AM_CONDITIONAL([HAVE_BYTES_COMPAT_ML ... from "m4/guestfs-ocaml.m4". However, in the common submodule, "mlstdutils/Makefile.am" still contains lines such as: if HAVE_BYTES_COMPAT_ML This causes autoreconf in libguestfs to fail with: common/mlstdutils/Makefile.am:30: error: HAVE_BYTES_COMPAT_ML does not appear in AM_CONDITIONAL My libguestfs checkout is current master (commit 4fe8df48a723), with the matching common submodule checkout (62ccce447228). Substituting a constant for HAVE_BYTES_COMPAT_ML in the "common" submodule is not obviously a solution, as guestfs-tools still has AM_CONDITIONAL([HAVE_BYTES_COMPAT_ML ..., in "configure.ac" (at commit 7fdf82407d36). For now, so that I can progress with something else, I guess I'll try checking out libguestfs at an earlier commit than b536c61a6df3. Thanks, Laszlo

2 3