Laszlo Ersek
2021-Sep-20 05:23 UTC
[Libguestfs] [PATCH 0/4] libguestfs modifications in order to pass "make check"
In my Fedora 34 environment, with all libguestfs features enabled except
the Erlang and Haskell bindings, "make check" passes if:
- these patches are applied on top of current master (f47e0bb67254), and
- the btrfs-progs package is downgraded to 5.13.1-1.fc34.x86_64,
according to <https://bugzilla.redhat.com/show_bug.cgi?id=2005529>.
Thanks,
Laszlo
Laszlo Ersek (4):
test-9p: fix the base directory that's exported to the guest
test-md-and-lvm-devices: work around RAID0 regression in Linux
v3.14/v5.4
tests: xfs: remove lazy-counter disablement test
Go bindings: fix "C array of strings" -- char** -- allocation
generator/actions_core.ml | 6 ----
generator/golang.ml | 46 +++++++++++++++++++++++------
tests/9p/test-9p.sh | 2 +-
tests/md/test-md-and-lvm-devices.sh | 12 +++++---
4 files changed, 46 insertions(+), 20 deletions(-)
--
2.19.1.3.g30247aa5d201
Laszlo Ersek
2021-Sep-20 05:23 UTC
[Libguestfs] [PATCH 1/4] test-9p: fix the base directory that's exported to the guest
In commit 6d32773e8118 ("tests: Run the tests in parallel.",
2021-03-18),
the "abs_srcdir" macro value that the 9p test would see changed from
".../tests/9p" to just ".../tests" -- the last component got
dropped.
(Said commit updated some "abs_srcdir"-based references accordingly,
for
example under "tests/disks", but "tests/9p/test-9p.sh" was
missed.)
Therefore, the guest-visible location of the "/test-9p.sh" file
changed to
"/9p/test-9p.sh", and a non-recursive listing of the guest-visible
root
directory would not return the file. Thus, the test fails now.
Restore the host-side base directory to ".../tests/9p".
Fixes: 6d32773e811882f78dbd8c2a39a2b7a9c3cfca7c
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
tests/9p/test-9p.sh | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tests/9p/test-9p.sh b/tests/9p/test-9p.sh
index b4bdbe56e077..4fd5de7fdafe 100755
--- a/tests/9p/test-9p.sh
+++ b/tests/9p/test-9p.sh
@@ -45,7 +45,7 @@ guestfish <<EOF
sparse test-9p.img 1M
config -device '$virtio_9p,fsdev=test9p,mount_tag=test9p'
-config -fsdev
'local,id=test9p,path=${abs_srcdir},security_model=passthrough'
+config -fsdev
'local,id=test9p,path=${abs_srcdir}/9p,security_model=passthrough'
run
--
2.19.1.3.g30247aa5d201
Laszlo Ersek
2021-Sep-20 05:23 UTC
[Libguestfs] [PATCH 2/4] test-md-and-lvm-devices: work around RAID0 regression in Linux v3.14/v5.4
The "test-md-and-lvm-devices" test case creates, among other things, a RAID0 array (md127) that spans two *differently sized* block devices (sda1: 20MB, lv0: 16MB). In Linux v3.14, the layout of such arrays was changed incompatibly and undetectably. If an array were created with a pre-v3.14 kernel and assembled on a v3.14+ kernel, or vice versa, data could be corrupted. In Linux v5.4, a mitigation was added, requiring the user to specify the layout version of such RAID0 arrays explicitly, as a module parameter. If the user fails to specify a layout version, the v5.4+ kernel refuses to assemble such arrays. This is why "test-md-and-lvm-devices" currently fails, with any v5.4+ appliance kernel. Until we implement a more general solution (see the bugzilla link below), work around the issue by sizing sda1 and lv0 identically. For this, increase the size of sdb1 to 24MB: when one 4MB extent is spent on LVM metadata, the resultant lv0 size (20MB) will precisely match the size of sda1. This workaround only affects sizes, and does not interfere with the original purpose of this test case, which is to test various *stackings* between disk partitions, software RAID (md), and LVM logical volumes. Related: https://bugzilla.redhat.com/show_bug.cgi?id=2005485 Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- tests/md/test-md-and-lvm-devices.sh | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/md/test-md-and-lvm-devices.sh b/tests/md/test-md-and-lvm-devices.sh index 5e82e3a4ff69..54f1c3bfb3f5 100755 --- a/tests/md/test-md-and-lvm-devices.sh +++ b/tests/md/test-md-and-lvm-devices.sh @@ -53,7 +53,7 @@ trap cleanup INT QUIT TERM EXIT # sda2: 20M PV (vg1) # sda3: 20M MD (md125) # -# sdb1: 20M PV (vg0) +# sdb1: 24M PV (vg0) [*] # sdb2: 20M PV (vg2) # sdb3: 20M MD (md125) # @@ -66,6 +66,9 @@ trap cleanup INT QUIT TERM EXIT # vg3 : VG (md125) # lv3 : LV (vg3) # +# [*] The reason for making sdb1 4M larger than sda1 is that the LVM metadata +# will consume one 4MB extent, and we need lv0 to offer exactly as much space +# as sda1 does, for combining them in md127. Refer to RHBZ#2005485. guestfish <<EOF # Add 2 empty disks @@ -79,9 +82,10 @@ part-add /dev/sda p 64 41023 part-add /dev/sda p 41024 81983 part-add /dev/sda p 81984 122943 part-init /dev/sdb mbr -part-add /dev/sdb p 64 41023 -part-add /dev/sdb p 41024 81983 -part-add /dev/sdb p 81984 122943 +part-add /dev/sdb p 64 49215 +part-add /dev/sdb p 49216 90175 +part-add /dev/sdb p 90176 131135 + # Create volume group and logical volume on sdb1 pvcreate /dev/sdb1 -- 2.19.1.3.g30247aa5d201
Laszlo Ersek
2021-Sep-20 05:23 UTC
[Libguestfs] [PATCH 3/4] tests: xfs: remove lazy-counter disablement test
According to xfs_admin(8):> -c 0|1 Enable (1) or disable (0) lazy-counters in the filesys? > tem. > > Lazy-counters may not be disabled on Version 5 su? > perblock filesystems (i.e. those with metadata CRCs en? > abled). > > [...]According to mkfs.xfs(1):> -m global_metadata_options > Section Name: [metadata] > These options specify metadata format options that ei? > ther apply to the entire filesystem or aren't easily > characterised by a specific functionality group. The > valid global_metadata_options are: > > [...] > > crc=value > This is used to create a filesystem which > maintains and checks CRC information in all > metadata objects on disk. The value is ei? > ther 0 to disable the feature, or 1 to en? > able the use of CRCs. > > [...] > > By default, mkfs.xfs will enable metadata > CRCs.Consistently with the above, the first "xfs_admin" test case in "generator/actions_core.ml", which attempts to disable lazy counters, always fails:> 534/550 test_xfs_admin_0 > libguestfs: error: xfs_admin: /dev/sda1: Cannot disable lazy-counters on V5 fsWe can resolve this test failure in three ways: (1) Extend do_mkfs() [daemon/mkfs.c], possibly even introduce do_mkfs_xfs(), and permit the caller to specify "-m crc=0" for mkfs.xfs. Then use this option when the temporary filesystem is created in the XFS test that disables lazy counters. (2) Extend the "guestfs_int_xfsinfo" structure in the libguestfs-common project, with an "xfs_crc" field. Extend parse_xfs_info() [daemon/xfs.c] to populate the field from "meta-data=...crc=[01]". Modify the test case to check the following post-condition: xfs_crc || xfs_lazycount == 0 instead of the current xfs_lazycount == 0 effectively ignoring "xfs_lazycount" when "xfs_crc" is set. (3) Remove the test altogether that attempts to disable lazy counters after filesystem creation. Given that new XFS filesystems are created with metadata CRCs enabled by default, and several XFS features depend on metadata CRCs being enabled, this patch implements option (3). Signed-off-by: Laszlo Ersek <lersek at redhat.com> --- generator/actions_core.ml | 6 ------ 1 file changed, 6 deletions(-) diff --git a/generator/actions_core.ml b/generator/actions_core.ml index bb602ee02dfc..5933282dcf90 100644 --- a/generator/actions_core.ml +++ b/generator/actions_core.ml @@ -7644,12 +7644,6 @@ with zeroes)." }; style = RErr, [String (Device, "device")], [OBool "extunwritten"; OBool "imgfile"; OBool "v2log"; OBool "projid32bit"; OBool "lazycounter"; OString "label"; OString "uuid"]; optional = Some "xfs"; tests = [ - InitEmpty, Always, TestResult ( - [["part_disk"; "/dev/sda"; "mbr"]; - ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; - ["xfs_admin"; "/dev/sda1"; ""; ""; ""; ""; "false"; "NOARG"; "NOARG"]; - ["mount"; "/dev/sda1"; "/"]; - ["xfs_info"; "/"]], "ret->xfs_lazycount == 0"), []; InitEmpty, Always, TestResultString ( [["part_disk"; "/dev/sda"; "mbr"]; ["mkfs"; "xfs"; "/dev/sda1"; ""; "NOARG"; ""; ""; "NOARG"]; -- 2.19.1.3.g30247aa5d201
Laszlo Ersek
2021-Sep-20 05:23 UTC
[Libguestfs] [PATCH 4/4] Go bindings: fix "C array of strings" -- char** -- allocation
The current "arg_string_list" and "free_string_list"
implementations go
back to commit b6f01f32606d ("Add Go (language) bindings.",
2013-07-03).
There are two problems with them:
- "free_string_list" doesn't actually free anything,
- at the *first* such g.Internal_test() call site that passes an
Ostringlist member inside the Optargs argument, namely:
> g.Internal_test ("abc",
> string_addr ("def"),
> []string{},
> false,
> 0,
> 0,
> "123",
> "456",
> []byte{'a', 'b', 'c',
'\000', 'a', 'b', 'c'},
> &guestfs.OptargsInternal_test{Ostringlist_is_set:
true,
> Ostringlist: []string{}
> }
> )
the "golang/run-bindtests" case crashes:
> panic: runtime error: cgo argument has Go pointer to Go pointer
>
> goroutine 1 [running]:
> libguestfs.org/guestfs.(*Guestfs).Internal_test.func7(0xc000018180,
> 0xadfb60, 0xadfb80, 0xc000010048, 0x0, 0x0, 0x0, 0xae3e10, 0xae3e30,
> 0xade3a0, ...)
> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0xa9
> libguestfs.org/guestfs.(*Guestfs).Internal_test(0xc000018180, 0x4ee3a5,
> 0x3, 0xc000061be8, 0xc000061af8, 0x0, 0x0, 0xc000061a00, 0x0, 0x0, ...)
> golang/src/libguestfs.org/guestfs/guestfs.go:6729 +0x3c9
> main.main()
> golang/bindtests/bindtests.go:77 +0x151e
> exit status 2
> FAIL run-bindtests (exit status: 1)
It turns out that a C-language array containing C-language pointers can
only be allocated in C:
https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
Rewrite the "arg_string_list" and "free_string_list"
functions almost
entirely in C.
Signed-off-by: Laszlo Ersek <lersek at redhat.com>
---
generator/golang.ml | 46 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/generator/golang.ml b/generator/golang.ml
index d328abe4ed08..5db478e8a494 100644
--- a/generator/golang.ml
+++ b/generator/golang.ml
@@ -52,6 +52,39 @@ _go_guestfs_create_flags (unsigned flags)
{
return guestfs_create_flags (flags);
}
+
+//
+// A C-language array of C-language pointers needs to be allocated in C.
+//
https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
+//
+typedef char *pChar;
+
+pChar *allocStringArray (size_t nmemb)
+{
+ pChar *array;
+
+ array = malloc (sizeof (pChar) * (nmemb + 1));
+ array[nmemb] = NULL;
+ return array;
+}
+
+void storeString (pChar *array, size_t idx, pChar string)
+{
+ array[idx] = string;
+}
+
+void freeStringArray (pChar *array)
+{
+ pChar *position;
+ pChar string;
+
+ position = array;
+ while ((string = *position) != NULL) {
+ free (string);
+ position++;
+ }
+ free (array);
+}
*/
import \"C\"
@@ -148,12 +181,11 @@ func (g *Guestfs) Close () *GuestfsError {
* C strings and golang []string.
*/
func arg_string_list (xs []string) **C.char {
- r := make ([]*C.char, 1 + len (xs))
+ r := C.allocStringArray (C.size_t (len (xs)))
for i, x := range xs {
- r[i] = C.CString (x)
+ C.storeString (r, C.size_t (i), C.CString (x));
}
- r[len (xs)] = nil
- return &r[0]
+ return (**C.char) (r)
}
func count_string_list (argv **C.char) int {
@@ -167,11 +199,7 @@ func count_string_list (argv **C.char) int {
}
func free_string_list (argv **C.char) {
- for *argv != nil {
- //C.free (*argv)
- argv = (**C.char) (unsafe.Pointer (uintptr (unsafe.Pointer (argv)) +
- unsafe.Sizeof (*argv)))
- }
+ C.freeStringArray ((*C.pChar) (argv))
}
func return_string_list (argv **C.char) []string {
--
2.19.1.3.g30247aa5d201