Intermittent test failures in virt-filesystems showed that when using
the pvs_full API, the pv_name field in the returned list of structures
was not being reverse translated. As a result internal partition
names could appear in the output of virt-filesystems.
See: https://listman.redhat.com/archives/libguestfs/2023-July/032058.html
The just added --chown option previously used StringPair, splitting
the argument as ‘UID.GID:FILENAME’. However this will not work if we
ever extend this with the ability to use user or group names, since
they may contain dot (but not colon). Add a new StringTriplet type
and split the argument string three ways. The new option becomes:
virt-customize ... --chown UID:GID:FILENAME
Include the following commit from the common submodule:
commit e70d89a58dae068be2e19c7c21558707261af96a
Author: Richard W.M. Jones <rjones@redhat.com>
Date: Sat Jul 15 16:42:06 2023 +0100
customize: Update generated files for --chown with StringTriplet
Updates: commit d8e48bff21
On QEMU 7.2.0+, if "passt" is available, ask QEMU for passt ("stream")
rather than SLIRP ("user") networking.
For this, we need to run passt ourselves. Given that passt daemonizes by
default, start it with our traditional function guestfs_int_cmd_run(). Ask
passt to save its PID file, because in case something goes wrong before
we're completely sure the appliance (i.e. QEMU) is up and running, we'll
need to kill passt, the *grandchild*, ourselves.
Pass "--one-off" to passt (same as libvirt). This way, once we have proof
that QEMU has connected to passt (because the appliance shows signs of
life), we need not clean up passt ourselves -- once QEMU exits, passt will
see an EOF on the unix domain socket, and exit as well.
Passt is way more flexible than SLIRP, and passt normally intends to
imitate the host environment in the guest as much as possible. This means
that, when switching from SLIRP to passt, the guest would see changes to
the following:
- guest IP address,
- guest subnet mask,
- host (= gateway) IP address,
- host (= gateway) MAC address.
Extract the SLIRP defaults into the new macros NETWORK_GW_IP and
NETWORK_GW_MAC, and pass them explicitly to passt. In particular,
"tests/rsync/test-rsync.sh" fails without setting the host address
(NETWORK_GW_IP) properly.
(These artifacts can be verified in the appliance with "virt-rescue
--network", by running "ip addr", "ip route", and "ip neighbor" at the
virt-rescue prompt. There are four scenarios: two libguest backends, times
passt being installed or not installed.)
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2184967
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20230714132213.96616-8-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Introduce a small function for creating pathnames for PID files.
guestfs_int_make_pid_path() is something of an amalgamation of
guestfs_int_make_temp_path() [1] and guestfs_int_create_socketname() [2]:
- it creates a pathname under sockdir, like [2],
- it uses the handle's unique counter, like [1],
- it takes a name like both [1] and [2], but the name is not size-limited
like in [2], plus we hardcode the suffix from [1] as ".pid".
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2184967
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20230714132213.96616-7-lersek@redhat.com>
Consider the following inverted call tree (effectively a dependency tree
-- callees are at the top and near the left margin):
lazy_make_tmpdir() [lib/tmpdirs.c]
guestfs_int_lazy_make_tmpdir() [lib/tmpdirs.c]
guestfs_int_make_temp_path() [lib/tmpdirs.c]
guestfs_int_lazy_make_sockdir() [lib/tmpdirs.c]
guestfs_int_create_socketname() [lib/launch.c]
lazy_make_tmpdir() is our common workhorse / helper function that
centralizes the mkdtemp() function call.
guestfs_int_lazy_make_tmpdir() and guestfs_int_lazy_make_sockdir() are the
next level functions, both calling lazy_make_tmpdir(), just feeding it
different dirname generator functions, and different "is_runtime_dir"
qualifications. These functions create temp dirs for various, more
specific, purposes (see the manual and "lib/guestfs-internal.h" for more
details).
On a yet higher level are guestfs_int_make_temp_path() and
guestfs_int_create_socketname() -- they serve for creating *entries* in
those specific temp directories.
The discrepancy here is that, although all the other functions live in
"lib/tmpdirs.c", guestfs_int_create_socketname() is defined in
"lib/launch.c". That makes for a confusing code reading; move the function
to "lib/tmpdirs.c", just below its sibling function
guestfs_int_make_temp_path().
While at it, correct the leading comment on
guestfs_int_create_socketname() -- the socket pathname is created in the
socket directory, not in the temporary directory.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2184967
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20230714132213.96616-6-lersek@redhat.com>
We generate the <interface type="user"> element on libvirt 3.8.0+ already.
For selecting passt rather than SLIRP, we only need to insert the child
element <backend type='passt'>. Make that child element conditional on
libvirt 9.0.0+, plus "passt --help" being executable.
For the latter, place the new helper function guestfs_int_passt_runnable()
in "lib/launch.c" -- we're going to use the same function for the direct
backend as well.
This change exposes a number of (perceived) shortcomings in libvirt; I've
filed <https://bugzilla.redhat.com/show_bug.cgi?id=2222766> about those.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2184967
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20230714132213.96616-3-lersek@redhat.com>
Currently we #define NETWORK_ADDRESS as "169.254.0.0". That's a bug in
libguestfs, but it's been invisible -- thus far it's been canceled out by
*independent* bugs in *both* QEMU and libvirt.
(1) With the direct backend, the current definition of NETWORK_ADDRESS
results in the following QEMU command line option:
-netdev user,id=usernet,net=169.254.0.0/16
According to the QEMU documentation, the "net" property here is supposed
to specify the *exact* IP address that the guest will receive. The guest
however receives 169.254.2.15 (I've checked that with virt-rescue).
In other words, libguestfs doesn't do what the QEMU documentation says,
and QEMU doesn't do what the QEMU documentation says either. The end
result has been good enough -- but only until now.
(2) With the libvirt backend, the current definition of NETWORK_ADDRESS
results in the following domain XML snippet:
<interface type="user">
<model type="virtio"/>
<ip family="ipv4" address="169.254.0.0" prefix="16"/>
</interface>
which libvirt translates, in turn, to
-netdev {"type":"user","net":"169.254.0.0/16","id":"hostnet0"}
According to the domain XML documentation, the @address attribute here is
again supposed to specify the *exact* IP address that the guest will
receive. However, the guest receives 169.254.2.15 (I've checked that with
virt-rescue).
In other words, libguestfs doesn't do what the libvirt documentation says,
and libvirt doesn't do what the libvirt documentation says either. The end
result has been good enough -- but only until now.
Where things break down though is the subsequent passt enablement, in the
rest of this series. For example, when using the libvirt backend together
with passt, libvirt translates the @address attribute to passt's
"--address" option, but passt takes the address *verbatim*, and not as a
subnet base address. That difference is visible in the appliance; for
example, when running virt-rescue on a Fedora 38 image, and issuing "ip
addr". Namely, after enabling passt for the libvirt backend, the
guest-visible IP address changes from 169.254.2.15 to 169.254.0.0, which
is an IP address that makes no sense for an endpoint.
Fix the latent bug by specifying the actual guest IP address we want, in
NETWORK_ADDRESS.
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2184967
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Message-Id: <20230714132213.96616-2-lersek@redhat.com>
The last (only?) caller of guestfs_int_cmd_clear_close_files() disappeared
in commit e4c3968880 ("lib/info: Remove /dev/fd hacking and pass a true
filename to qemu-img info.", 2018-01-23), part of v1.37.36.
Simplify the code by removing guestfs_int_cmd_clear_close_files().
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
Message-Id: <20230711113906.107340-1-lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
When finalizing the handle we call guestfs_close. This function could
be long-running (eg. it may have to shut down the qemu subprocess), so
release the runtime lock.
Since OCaml 4 the old and confusing caml_enter_blocking_section and
caml_leave_blocking_section calls have been replaced with
caml_release_runtime_system and caml_acquire_runtime_system (in that
order). Use the new names.
Various errors like this:
In function ‘test_fuse’,
inlined from ‘main’ at test-fuse.c:133:11:
test-fuse.c:274:5: error: argument 1 null where non-null expected [-Werror=nonnull]
274 | fclose (fp);
| ^~~~~~~~~~~
In file included from test-fuse.c:26:
/usr/include/stdio.h: In function ‘main’:
/usr/include/stdio.h:183:12: note: in a call to function ‘fclose’ declared ‘nonnull’
183 | extern int fclose (FILE *__stream) __nonnull ((1));
| ^~~~~~
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
Since SELinux userspace v3.4 [1], setfiles command supports "-T nthreads"
option, which allows parallel execution. "-T 0" allows using as many
threads as there're available CPU cores. This might speed up the process
of filesystem relabeling in case the appliance is being run with multiple
vCPUs. The latter is true for at least v2v starting from d2b64ecc67
("v2v: Set the number of vCPUs to same as host number of pCPUs.").
For instance, when running virt-v2v-in-place on my 12-core Xeon host
with SSD, with appliance being run with 8 vCPUs (the upper limit specified
in d2b64ecc67), and on the ~150GiB disk VM (physical size on the host),
I get the following results:
./in-place/virt-v2v-in-place -i libvirt fedora37-vm -v -x
Without this patch:
...
commandrvf: setfiles -F -e /sysroot/dev -e /sysroot/proc -e /sysroot/sys -m -C -r /sysroot -v /sysroot/etc/selinux/targeted/contexts/files/file_contexts /sysroot/^M
libguestfs: trace: v2v: selinux_relabel = 0
libguestfs: trace: v2v: rm_f "/.autorelabel"
guestfsd: => selinux_relabel (0x1d3) took 17.94 secs
...
With this patch:
...
commandrvf: setfiles -F -e /sysroot/dev -e /sysroot/proc -e /sysroot/sys -m -C -T 0 -r /sysroot -v /sysroot/etc/selinux/targeted/contexts/files/file_contexts /sysroot/^M
libguestfs: trace: v2v: selinux_relabel = 0
libguestfs: trace: v2v: rm_f "/.autorelabel"
guestfsd: => selinux_relabel (0x1d3) took 5.88 secs
...
So in my scenario it's getting 3 times faster.
[1] https://github.com/SELinuxProject/selinux/releases/tag/3.4
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
'X' in the setiles' stderr doesn't necessarily mean that option 'X'
doesn't exist. For instance, when passing '-T' we get: "setfiles:
option requires an argument -- 'T'".
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Since RHBZ#726528, filesystem.rpm doesn't include /selinux. setfiles
then gives us the warning: "Can't stat exclude path "/sysroot/selinux",
No such file or directory - ignoring."
Though the warning is harmless, let's get rid of it by checking the
existence of /selinux directory.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Andrey Drobyshev (2):
inject_virtio_win: add Virtio_SCSI to block_type
inject_virtio_win: write the proper block controller PCI ID to Win registry
Richard W.M. Jones (2):
mlcustomize: Fix overlong comment
mlcustomize: Add accessors for block driver priority list
Roman Kagan (1):
inject_virtio_win: match only vendor/device/revision
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
This was not being set because of some impenetrable problem with
autoconf. The actual line which set the shell variable was simply
being deleted for some reason. Using an m4 definition works.
Updates: commit f68752462e
