[PATCH v2 0/5] work around part table type misreporting by "parted"
by Laszlo Ersek
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 years, 5 months
[nbdkit PATCH v2 0/2] common/include/checked-overflow: provide fallback
by Laszlo Ersek
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 years, 5 months
[nbdkit PATCH 0/2] common/include/checked-overflow: provide fallback
by Laszlo Ersek
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
2 years, 5 months
[PATCH nbdkit 1/2] ocaml: Simplify NBDKit.set_error
by Richard W.M. Jones
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
2 years, 5 months
[PATCH] daemon/mkfs: disable creation of fake MBR partition table with "mkfs.fat"
by Laszlo Ersek
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 years, 5 months
[PATCH] xfs: Document lazy-counters setting cannot be changed in XFS version 5
by Richard W.M. Jones
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 years, 5 months
specifying a standard VGA video controller in OVF for oVirt
by Laszlo Ersek
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
2 years, 6 months
[PATCH libnbd 0/6] Enhance synch-parallel test
by Nir Soffer
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 years, 6 months
libguestfs build failure
by Laszlo Ersek
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 years, 6 months