Commit d5b6f1df5f ("daemon: Allow parts of the daemon and APIs to be
written in OCaml.", 2017) contained a bug where in any OCaml function
that returns int64_t, the result was truncated to an int. This
particularly affected part_get_gpt_attributes as that returns large 64
bit numbers, but probably affects other functions too, undetected.
Fixes: commit d5b6f1df5f
(cherry picked from commit 882ef4d93a)
(cherry picked from commit 285b8fa92b)
This was failing with recent Linux:
libguestfs: error: btrfs_subvolume_snapshot: /dir/test3: /dir/test6: ERROR: cannot snapshot '/sysroot/dir/test3': Invalid argument
I tried to change the test to use 1/1000 instead, but that fails with
a different error which I don't understand at all.
As we're not meant to be testing btrfs here, only that libguestfs can
translate between the guestfs API and btrfs commands and we know it
can do that, I simply deleted the sub-test entirely.
(cherry picked from commit c7fe9fd917)
(cherry picked from commit f970f093fb)
This was an accident of the parted implementation, and wasn't really
used anywhere. Remove it.
(cherry picked from commit 2811e42b43)
(cherry picked from commit cbfe36b7c7)
Since OCaml 4.07 (released 2018-07-10) the always-loaded standard
library module has been called Stdlib. The old Pervasives module was
finally removed in OCaml 5.
$ perl -pi.bak -e 's/Pervasives\./Stdlib./g' -- `git ls-files`
OCaml >= 4.07 is now required.
Also update the common submodule with:
commit d61cd820b49e403848d15c5deaccbf8dd7045370
Author: Jürgen Hötzel
Date: Sat May 20 18:16:40 2023 +0200
Add support for OCaml 5.0
(cherry picked from commit 3cb094083e)
The presence of this file complicates ./configure and you also have to
remember to update it on each release. Replace it with a simple
RELEASE_DATE set in ./configure when the version is updated.
I also had to make a minor change to the generator which was using
this file both to check it was run from the source directory and to
get an exclusive lock. We now use podwrapper.pl.in for this.
(cherry picked from commit f68752462e)
Run this command across the source:
perl -pi.bak -e 's/(20[012][0-9])-20[12][012]/$1-2023/g' `git ls-files`
and remove changes to po{,-docs}/*.po{,t} (these will be regenerated
later when we run 'make dist').
Add an API to return the build ID of the guest. This to allow a
future change to be able to distinguish between Windows 10 and Windows 11
which can only be done using the build ID.
For Windows we can read the CurrentBuildNumber key from the registry.
For Linux there happens to be a BUILD_ID field in /etc/os-release.
I've never seen a Linux distro that actually uses this.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
The file(1) manual suggests using -S (disable seccomp) with -z since
the set of system calls provided by the seccomp policy does not allow
the subprocess to run. This is obvious when you use file -z on a
compressed file on a Linux distro that enables file's seccomp policy
(Arch does this, Fedora does not):
$ file -zbsL lib-i586.so.zst
Bad system call
I also fixed some incorrect text in the manual.
Thanks: Toolybird for pointing to this fix
Reported-by: David Runge
Fixes: https://github.com/libguestfs/libguestfs/issues/100
I cannot reproduce the originally reported error:
libguestfs: error: mkfs: xfs: /dev/VG/LV: Filesystem must be larger than 300MB.
Thanks: David Runge
Related: https://github.com/libguestfs/libguestfs/issues/100
Many warnings such as:
src/session.c: In function 'guestfs_session_internal_test_rstruct':
src/session.c:14755:7: warning: the comparison will always evaluate as 'true' for the address of 'pv_uuid' will never be NULL [-Waddress]
14755 | if (ret->pv_uuid) memcpy (s->pv_uuid, ret->pv_uuid, sizeof (s->pv_uuid));
| ^~~
In file included from src/session.c:40:
../include/guestfs.h:551:8: note: 'pv_uuid' declared here
551 | char pv_uuid[32]; /* this is NOT nul-terminated, be careful when printing */
| ^~~~~~~
Deprecation warnings include:
src/optargs-xfs_growfs.c: In function 'guestfs_xfs_growfs_init':
src/optargs-xfs_growfs.c:311:13: warning: Deprecated pre-processor symbol: replace with "G_ADD_PRIVATE"
311 | o->priv = GUESTFS_XFS_GROWFS_GET_PRIVATE (o);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is based on the same change made here:
https://github.com/nzjrs/osm-gps-map/pull/78/files
The macOS rpcgen actually generates calls to xdr_uint64_t but doesn't
define them. Despite this we can just use xdr_int64_t instead since
it's just byte swapping.
This is required so we can determine the file architecture of
zstd-compressed Linux kernel modules as used by OpenSUSE and maybe
other distros in future.
Note that zstd becomes a required package, but it is widely available
in current Linux distros.
The package names come from https://pkgs.org/download/zstd and my own
research.
Starting with PHP8, arginfo is mandatory for PHP extensions. This patch
updates the generator for the PHP bindings to generate the arginfo
structures, using the Zend API macros. Only basic arginfo is added,
without full documentation of argument and return types, in order to
ensure compatibility with as many versions of PHP as possible.
The documentation currently says that the user should avoid passing
"--selinux-relabel" on the command line if the guest does not support
SELinux. However, the "is_selinux_guest" helper function in
"common/mlcustomize/SELinux_relabel.ml" already turns "--selinux-relabel"
into a no-op if some key SELinux files are absent from the guest, so there
is no need to caution the user.
This change is relevant because the subsequent patches will turn on
"--selinux-relabel" by default, and therefore "is_selinux_guest" will grow
in importance.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1554735
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2075718
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220510102757.14466-2-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
Representing "iface" in the "drive_create_data" and "drive" structures is
now useless; the direct backend ignores "iface", while the libvirt one
rejects it unless it is empty. Unify both backends -- make them both
ignore "iface". (Which only relaxes the libvirt backend, so it cannot
cause compatibility problems.) This lets us remove the fields. Update the
documentation as well.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1844341
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220504134155.11832-3-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Currently the guestfs_readdir() API can not list long directories, due to
it sending back the whole directory listing in a single guestfs protocol
response, which is limited to GUESTFS_MESSAGE_MAX (approx. 4MB) in size.
Introduce the "internal_readdir" action, for transferring the directory
listing from the daemon to the library through a FileOut parameter.
Rewrite guestfs_readdir() on top of this new internal function:
- The new "internal_readdir" action is a daemon action. Do not repurpose
the "readdir" proc_nr (138) for "internal_readdir", as some distros ship
the binary appliance to their users, and reusing the proc_nr could
create a mismatch between library & appliance with obscure symptoms.
Replace the old proc_nr (138) with a new proc_nr (511) instead; a
mismatch would then produce a clear error message. Assume the new action
will first be released in libguestfs-1.48.2.
- Turn "readdir" from a daemon action into a non-daemon one. Call the
daemon action guestfs_internal_readdir() manually, receive the FileOut
parameter into a temp file, then deserialize the dirents array from the
temp file.
This patch sneakily fixes an independent bug, too. In the pre-patch
do_readdir() function [daemon/readdir.c], when readdir() returns NULL, we
don't distinguish "end of directory stream" from "readdir() failed". This
rewrite fixes this problem -- I didn't see much value separating out the
fix for the original do_readdir().
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1674392
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20220502085601.15012-2-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Before commit 3a00c4d179 ("Remove inspection from the C library and
switch to daemon/OCaml implementation") in 2017 the name parameter
passed to add_drive was used by inspection to override the device name
that is determined from fstab. None of our tools ever actually used
this parameter, and when the inspection code was moved inside the
daemon we stopped using this hint field at all.
So it's no longer used, and likely hasn't been used ever. Therefore
document that the field does nothing.
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
cdrtools writes "CDROM" into the Volume Identifier field in the PVD,
whereas genisoimage and xorriso write "ISOIMAGE". Recognise either
string as valid in the test.
Fixes: https://github.com/libguestfs/libguestfs/issues/79
Reported-by: David Runge
This was a feature that allowed you to add drives to the appliance
after launching it. It was complicated to implement, and only worked
for the libvirt backend (not "direct", which is the default backend).
It also turned out to be a bad idea. The original concept was that
appliance creation was slow, so to examine multiple guests you should
launch the handle once then hot-add the disks from each guest in turn
to manipulate them. However this is terrible from a security point of
view, especially for multi-tenant, because the drives from one guest
might compromise the appliance and thus the filesystems/drives from
subsequent guests.
It also turns out that hotplugging is very slow. Nowadays appliance
creation should be faster than hotplugging.
The main use case for this was virt-df, but virt-df no longer uses it
after we discovered the problems outlined above.
These APIs were an experimental feature for passing through 9p
filesystems from the host to the libguestfs appliance. It was never
possible to use this without hacking the qemu command line of the
appliance to add such drives by hand. It also didn't fit the
libguestfs model very well. And 9p is generally deprecated in
upstream qemu.
Note that for ABI reasons these APIs are not actually removed, they
have been changed so that they always return an error. These APIs
were actually hard-removed from all versions of RHEL.
See-also: https://bugzilla.redhat.com/921710
User-Mode Linux was an alternative hypervisor that could run the
appliance, instead of using qemu. It had many limitations including
lack of network, and UML support in Linux has been semi-broken for a
long time. It was also slower than KVM on baremeal in general and had
various corner cases which were much slower including the emulated
serial port which made bulk uploads and downloads painful. Also of
course it lacked qemu-specific features like qcow2 or any
network-backed disk, so many disk images could not be opened this way.
This was never supported in RHEL.
See-also: https://bugzilla.redhat.com/1144197
This experimental feature allowed you (in theory) to connect to an
existing instance of the libguestfs daemon. (Again, in theory) it
allowed you to attach to running guests. This didn't work well in
practice. If you want to do this, install qemu-guest-agent inside
your guest instead.
This also disables the --live options in guestfish and guestmount.
(The option now prints an error).
This was never supported in RHEL.
The daemon tests relied on this connection method to perform tests on
a bare daemon, so this removes those tests. They were not especially
valuable.
See-also: https://bugzilla.redhat.com/798980
In some places in the generator we were still generating "noalloc".
It was hidden from the previous regexp I used to replace these because
of string escaping.
Updates: commit a69cde79ca
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=2030709
Thanks: label@rockylinux.org
---
RWMJ notes: I fixed the original patch so it compiled. This patch
sets osinfo to "rocky8", which doesn't exist in the osinfo db yet.
Arguably we might want to set this to "centos8", but we can see what
libosinfo decides to do. Here is partial virt-inspector output on a
Rocky Linux disk image:
$ ./run virt-inspector -a disk.img
<?xml version="1.0"?>
<operatingsystems>
<operatingsystem>
<root>/dev/rl/root</root>
<name>linux</name>
<arch>x86_64</arch>
<distro>rocky</distro>
<product_name>Rocky Linux 8.5 (Green Obsidian)</product_name>
<major_version>8</major_version>
<minor_version>5</minor_version>
<package_format>rpm</package_format>
<package_management>dnf</package_management>
<hostname>localhost.localdomain</hostname>
<osinfo>rocky8</osinfo>
<mountpoints>
<mountpoint dev="/dev/rl/root">/</mountpoint>
<mountpoint dev="/dev/sda1">/boot</mountpoint>
</mountpoints>
<filesystems>
<filesystem dev="/dev/rl/root">
<type>xfs</type>
<uuid>fed8331f-9f25-40cd-883e-090cd640559d</uuid>
</filesystem>
<filesystem dev="/dev/rl/swap">
<type>swap</type>
<uuid>6da2c121-ea7d-49ce-98a3-14a37fceaadd</uuid>
</filesystem>
<filesystem dev="/dev/sda1">
<type>xfs</type>
<uuid>4efafe61-2d20-4d93-8055-537e09bfd033</uuid>
</filesystem>
</filesystems>
The current "arg_string_list" and "free_string_list" implementations go
back to commit b6f01f3260 ("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)
In Daniel P. Berrangé's words [1],
> You're allowed to pass a Go pointer to C via CGo, but the memory that
> points to is not allowed to contained further Go pointers. So the struct
> fields must strictly use a C pointer.
One pattern to solve the problem has been shown on stackoverflow [2].
Thus, rewrite the "arg_string_list" and "free_string_list" functions
almost entirely in C, following that example.
While this approach is not the most idiomatic Go, as a solution exists
without C helper functions [3], it should still be acceptable, at least as
an incremental improvement, for letting "golang/run-bindtests" pass.
[1] https://listman.redhat.com/archives/libguestfs/2021-September/msg00118.html
[2] https://stackoverflow.com/questions/35924545/golang-cgo-panic-runtime-error-cgo-argument-has-go-pointer-to-go-pointer
[3] https://listman.redhat.com/archives/libguestfs/2021-September/msg00106.html
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: "Richard W.M. Jones" <rjones@redhat.com>
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20210921192939.32468-1-lersek@redhat.com>
Tested-by: "Richard W.M. Jones" <rjones@redhat.com>
Acked-by: "Richard W.M. Jones" <rjones@redhat.com>
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 fs
We 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@redhat.com>
Message-Id: <20210920052335.3358-4-lersek@redhat.com>
Acked-by: Richard W.M. Jones <rjones@redhat.com>
When creating and returning a Python struct we were adding fields from
the C struct, but did not reduce the ref count on the temporary value
after it had been moved to the struct, resulting in a memory leak.
Reported-by: 朱丹 <zhudan24@huawei.com>
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1985912
warning: panic message is not a string literal
--> src/bin/event_leak.rs:9:30
|
9 | Err(e) => panic!(format!(" could not create handle {:?}", e)),
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `#[warn(non_fmt_panic)]` on by default
= note: this is no longer accepted in Rust 2021
= note: this warning originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
./guestfs.go:4945:16: conversion from _Ctype_char (int8) to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?)
./guestfs.go:7320:16: conversion from _Ctype_char (int8) to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?)
./guestfs.go:7335:16: conversion from _Ctype_char (int8) to string yields a string of one rune, not a string of digits (did you mean fmt.Sprint(x)?)
This gnulib feature abstracts away threads, locks and TLS, and also
allowed libguestfs to be linked with or without pthread. However
since pthread these days is part of glibc and so every program is
using pthread, and we want to get rid of gnulib as a dependency, just
use pthread directly.
Currently the guestfs_isoinfo and guestfs_isoinfo_device APIs run
isoinfo inside the appliance to extract the information.
isoinfo is part of genisoimage which is somewhat dead upstream.
xorriso is supposedly the new thing. (For a summary of the situation
see: https://wiki.debian.org/genisoimage).
This commit rewrites the parsing from C to OCaml to make it easier to
deal with, and allows you to use either isoinfo or xorriso.
Mostly the same fields are available from either tool, but xorriso is
a bit more awkward to parse.
