[libnbd PATCH v2 00/10] Second draft of the Rust bindings
by Tage Johansson
This second version of the Rust bindings splits up the `cbkind` field in
the `closure` structure into `cblifetime` and `cbcount` in API.ml. It also
adds a new field to the `call` structure: `modifies_fd`, which tells
whether the call may modify the file descriptor.
Please comment and give feedback.
Best regards,
Tage
Tage Johansson (10):
rust: create basic Rust bindings
generator: Add information about asynchronous handle calls
generator: Add information about the lifetime of closures
rust: Use more specific closure traits
rust: Add a couple of integration tests
rust: Make it possible to run tests with Valgrind
rust: async: Create an async friendly handle type
rust: async: Add a couple of integration tests
generator: Add `modifies_fd` flag to the [call] structure
rust: async: Use the `modifies_fd` flag to exclude calls
.gitignore | 8 +
.ocamlformat | 4 +
Makefile.am | 1 +
configure.ac | 13 +
generator/API.ml | 84 ++
generator/API.mli | 35 +
generator/Makefile.am | 2 +
generator/Rust.ml | 814 ++++++++++++++++++
generator/Rust.mli | 22 +
generator/generator.ml | 3 +
rust/Cargo.toml | 55 ++
rust/Makefile.am | 76 ++
rust/libnbd-sys/Cargo.toml | 30 +
rust/libnbd-sys/build.rs | 60 ++
rust/libnbd-sys/src/lib.rs | 24 +
rust/libnbd-sys/wrapper.h | 18 +
rust/run-tests.sh | 30 +
rust/src/async_handle.rs | 222 +++++
rust/src/error.rs | 118 +++
rust/src/handle.rs | 67 ++
rust/src/lib.rs | 32 +
rust/src/types.rs | 20 +
rust/src/utils.rs | 23 +
rust/tests/nbdkit_pattern/mod.rs | 28 +
rust/tests/test_100_handle.rs | 25 +
rust/tests/test_110_defaults.rs | 33 +
rust/tests/test_120_set_non_defaults.rs | 56 ++
rust/tests/test_130_private_data.rs | 28 +
rust/tests/test_140_explicit_close.rs | 31 +
rust/tests/test_200_connect_command.rs | 33 +
rust/tests/test_210_opt_abort.rs | 39 +
rust/tests/test_220_opt_list.rs | 85 ++
rust/tests/test_230_opt_info.rs | 124 +++
rust/tests/test_240_opt_list_meta.rs | 151 ++++
rust/tests/test_245_opt_list_meta_queries.rs | 98 +++
rust/tests/test_250_opt_set_meta.rs | 124 +++
rust/tests/test_255_opt_set_meta_queries.rs | 111 +++
rust/tests/test_300_get_size.rs | 36 +
rust/tests/test_400_pread.rs | 40 +
rust/tests/test_405_pread_structured.rs | 80 ++
rust/tests/test_410_pwrite.rs | 62 ++
rust/tests/test_460_block_status.rs | 96 +++
rust/tests/test_620_stats.rs | 76 ++
rust/tests/test_async_100_handle.rs | 25 +
rust/tests/test_async_200_connect_command.rs | 34 +
rust/tests/test_async_210_opt_abort.rs | 40 +
rust/tests/test_async_220_opt_list.rs | 86 ++
rust/tests/test_async_230_opt_info.rs | 126 +++
rust/tests/test_async_235memleak.rs | 57 ++
rust/tests/test_async_240_opt_list_meta.rs | 151 ++++
.../test_async_245_opt_list_meta_queries.rs | 96 +++
rust/tests/test_async_250_opt_set_meta.rs | 123 +++
.../test_async_255_opt_set_meta_queries.rs | 111 +++
rust/tests/test_async_400_pread.rs | 41 +
rust/tests/test_async_405_pread_structured.rs | 85 ++
rust/tests/test_async_410_pwrite.rs | 63 ++
rust/tests/test_async_460_block_status.rs | 96 +++
rust/tests/test_async_620_stats.rs | 77 ++
rust/tests/test_log/mod.rs | 86 ++
rustfmt.toml | 19 +
60 files changed, 4433 insertions(+)
create mode 100644 .ocamlformat
create mode 100644 generator/Rust.ml
create mode 100644 generator/Rust.mli
create mode 100644 rust/Cargo.toml
create mode 100644 rust/Makefile.am
create mode 100644 rust/libnbd-sys/Cargo.toml
create mode 100644 rust/libnbd-sys/build.rs
create mode 100644 rust/libnbd-sys/src/lib.rs
create mode 100644 rust/libnbd-sys/wrapper.h
create mode 100755 rust/run-tests.sh
create mode 100644 rust/src/async_handle.rs
create mode 100644 rust/src/error.rs
create mode 100644 rust/src/handle.rs
create mode 100644 rust/src/lib.rs
create mode 100644 rust/src/types.rs
create mode 100644 rust/src/utils.rs
create mode 100644 rust/tests/nbdkit_pattern/mod.rs
create mode 100644 rust/tests/test_100_handle.rs
create mode 100644 rust/tests/test_110_defaults.rs
create mode 100644 rust/tests/test_120_set_non_defaults.rs
create mode 100644 rust/tests/test_130_private_data.rs
create mode 100644 rust/tests/test_140_explicit_close.rs
create mode 100644 rust/tests/test_200_connect_command.rs
create mode 100644 rust/tests/test_210_opt_abort.rs
create mode 100644 rust/tests/test_220_opt_list.rs
create mode 100644 rust/tests/test_230_opt_info.rs
create mode 100644 rust/tests/test_240_opt_list_meta.rs
create mode 100644 rust/tests/test_245_opt_list_meta_queries.rs
create mode 100644 rust/tests/test_250_opt_set_meta.rs
create mode 100644 rust/tests/test_255_opt_set_meta_queries.rs
create mode 100644 rust/tests/test_300_get_size.rs
create mode 100644 rust/tests/test_400_pread.rs
create mode 100644 rust/tests/test_405_pread_structured.rs
create mode 100644 rust/tests/test_410_pwrite.rs
create mode 100644 rust/tests/test_460_block_status.rs
create mode 100644 rust/tests/test_620_stats.rs
create mode 100644 rust/tests/test_async_100_handle.rs
create mode 100644 rust/tests/test_async_200_connect_command.rs
create mode 100644 rust/tests/test_async_210_opt_abort.rs
create mode 100644 rust/tests/test_async_220_opt_list.rs
create mode 100644 rust/tests/test_async_230_opt_info.rs
create mode 100644 rust/tests/test_async_235memleak.rs
create mode 100644 rust/tests/test_async_240_opt_list_meta.rs
create mode 100644 rust/tests/test_async_245_opt_list_meta_queries.rs
create mode 100644 rust/tests/test_async_250_opt_set_meta.rs
create mode 100644 rust/tests/test_async_255_opt_set_meta_queries.rs
create mode 100644 rust/tests/test_async_400_pread.rs
create mode 100644 rust/tests/test_async_405_pread_structured.rs
create mode 100644 rust/tests/test_async_410_pwrite.rs
create mode 100644 rust/tests/test_async_460_block_status.rs
create mode 100644 rust/tests/test_async_620_stats.rs
create mode 100644 rust/tests/test_log/mod.rs
create mode 100644 rustfmt.toml
base-commit: fbde5974fd5c8a3bcb081db0b1074c4a3e723e76
--
2.41.0
1 year, 4 months
[PATCH guestfs-tools] diff: Don't compare st_dev or st_ino fields
by Richard W.M. Jones
See comment for details.
Link: https://listman.redhat.com/archives/libguestfs/2023-July/032061.html
---
diff/diff.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/diff/diff.c b/diff/diff.c
index fb66a2bbd3..b6344b4ec2 100644
--- a/diff/diff.c
+++ b/diff/diff.c
@@ -717,8 +717,20 @@ changed (guestfs_h *g1, struct file *file1,
output_string ("changed:");
#define COMPARE_STAT(n) \
if (file1->stat->n != file2->stat->n) output_string (#n)
- COMPARE_STAT (st_dev);
- COMPARE_STAT (st_ino);
+ /* Comparing st_dev and st_ino is disabled for now, see the longer
+ * discussion here:
+ * https://listman.redhat.com/archives/libguestfs/2023-July/032061.html
+ * Even if we fixed the libguestfs API to do translation of this
+ * field correctly, it seems unlikely that there would ever be a
+ * meaningful difference in the st_dev or st_ino fields. We
+ * already know the fields refer to the same filename. Is it
+ * interesting that the file might have moved to a different disk?
+ * Everything else is comparing the content or direct metadata of
+ * the file, but st_dev and st_ino represent the metadata of the
+ * filesystem which is (arguably) different.
+ */
+// COMPARE_STAT (st_dev);
+// COMPARE_STAT (st_ino);
COMPARE_STAT (st_mode);
COMPARE_STAT (st_nlink);
COMPARE_STAT (st_uid);
--
2.41.0
1 year, 4 months
[PATCH libguestfs] daemon: lvm: Do reverse device name translation on pvs_full device fields
by Richard W.M. Jones
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
---
daemon/lvm.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/daemon/lvm.c b/daemon/lvm.c
index 7e76e17ccc..b8c01f718c 100644
--- a/daemon/lvm.c
+++ b/daemon/lvm.c
@@ -146,7 +146,34 @@ do_vgs (void)
guestfs_int_lvm_pv_list *
do_pvs_full (void)
{
- return parse_command_line_pvs ();
+ guestfs_int_lvm_pv_list *r;
+ size_t i;
+ char *din, *dout;
+
+ r = parse_command_line_pvs ();
+ if (r == NULL)
+ /* parse_command_line_pvs has already called reply_with_error */
+ return NULL;
+
+ /* The pv_name fields contain device names which must be reverse
+ * translated. The problem here is that the generator does not have
+ * a "FMountable" field type in types.mli.
+ */
+ for (i = 0; i < r->guestfs_int_lvm_pv_list_len; ++i) {
+ din = r->guestfs_int_lvm_pv_list_val[i].pv_name;
+ if (din) {
+ dout = reverse_device_name_translation (din);
+ if (!dout) {
+ /* reverse_device_name_translation has already called reply_with_error*/
+ /* XXX memory leak here */
+ return NULL;
+ }
+ r->guestfs_int_lvm_pv_list_val[i].pv_name = dout;
+ free (din);
+ }
+ }
+
+ return r;
}
guestfs_int_lvm_vg_list *
--
2.41.0
1 year, 4 months
[libguestfs PATCH v2 0/7] lib: support networking with passt
by Laszlo Ersek
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2184967
v1: https://listman.redhat.com/archives/libguestfs/2023-July/031984.html
V2 implements small updates; the cumulative v1->v2 diff is just
> diff --git a/lib/launch-direct.c b/lib/launch-direct.c
> index 8d6ad025a4e1..cdfd25a9afed 100644
> --- a/lib/launch-direct.c
> +++ b/lib/launch-direct.c
> @@ -338,9 +338,9 @@ add_drives (guestfs_h *g, struct backend_direct_data *data,
> /**
> * Launch passt such that it daemonizes.
> *
> - * On error, -1 is returned; C<passt_pid> and C<sockpath> are not modified.
> + * On error, C<-1> is returned; C<passt_pid> and C<sockpath> are not modified.
> *
> - * On success, 0 is returned. C<passt_pid> contains the PID of the passt
> + * On success, C<0> is returned. C<passt_pid> contains the PID of the passt
> * background process. C<sockpath> contains the pathname of the unix domain
> * socket where passt will accept a single connection.
> */
> @@ -394,7 +394,12 @@ launch_passt (guestfs_h *g, long *passt_pid, char (*sockpath)[UNIX_PATH_MAX])
> goto close_cmd;
> }
>
> - assert (WIFEXITED (passt_status));
> + if (!WIFEXITED (passt_status)) {
> + error (g, _("internal error: unexpected exit status from passt (%d)"),
> + passt_status);
> + goto close_cmd;
> + }
> +
> passt_exit = WEXITSTATUS (passt_status);
> if (passt_exit != 0) {
> error (g, _("passt exited with status %d"), passt_exit);
> diff --git a/lib/launch.c b/lib/launch.c
> index a0a8e1c45a51..b9b76e509162 100644
> --- a/lib/launch.c
> +++ b/lib/launch.c
> @@ -408,6 +408,9 @@ guestfs_int_passt_runnable (guestfs_h *g)
> return false;
>
> guestfs_int_cmd_add_string_unquoted (cmd, "passt --help");
> + if (!g->verbose)
> + guestfs_int_cmd_add_string_unquoted (cmd, " >/dev/null 2>&1");
> +
> r = guestfs_int_cmd_run (cmd);
> if (r == -1 || !WIFEXITED (r))
> return false;
dispersed over patches #2 and #7.
I lightly tested the updates with virt-rescue (direct & libvirt backends
with passt installed).
Thanks
Laszlo
Laszlo Ersek (7):
lib: fix NETWORK_ADDRESS: make it an actual IP address, not a subnet
base
lib/launch-libvirt: support networking with passt
docs: fix broken link in the guestfs manual
docs: clarify sockdir's separation
lib: move guestfs_int_create_socketname() from "launch.c" to
"tmpdirs.c"
lib: introduce guestfs_int_make_pid_path()
lib/launch-direct: support networking with passt
fish/guestfish.pod | 4 +-
generator/actions_properties.ml | 8 +-
lib/guestfs-internal.h | 32 ++++-
lib/guestfs.pod | 6 +-
lib/launch-direct.c | 152 +++++++++++++++++++-
lib/launch-libvirt.c | 11 ++
lib/launch.c | 57 ++++----
lib/tmpdirs.c | 41 ++++++
8 files changed, 271 insertions(+), 40 deletions(-)
base-commit: 13c7052ff96d5ee99ec1b1252f1a3b4d7aed44d2
1 year, 4 months
Intermittent test failure in guestfs-tools test-virt-filesystems.sh
by Richard W.M. Jones
(See attachment for full log)
https://github.com/rwmjones/guestfs-tools/blob/master/cat/test-virt-files...
This test uses virt-filesystems to list the filesystems in the Fedora
phony disk image. In one run of this test it found an "extra"
/dev/sdb2 filesystem that obviously doesn't exist.
Note that the log contains two runs of virt-filesystems, only the
second one shows a failure.
In the log we see that the appliance has two disks, sda is the
appliance root, sdb is the Fedora image. Device name translation will
map these two disks in reverse so to virt-filesystems the Fedora image
should appear as /dev/sda and the partitions as /dev/sda{1,2}.
The problem seems to be with the call to guestfs_pvs_full which
returns:
<struct guestfs_lvm_pv_list(1) =
[0]{pv_name: /dev/sdb2, <--------------------
pv_uuid: RVgEshd4X91jq8omUQIsB77eyB9miZzL,
pv_fmt: lvm2,
pv_size: 532676608,
dev_size: 536838656,
pv_free: 130023424,
pv_used: 402653184,
pv_attr: a--,
pv_pe_count: 127,
pv_pe_alloc_count: 96,
pv_tags: ,
pe_start: 1048576,
pv_mda_count: 1,
pv_mda_free: 519168, }>
Note that the pv_name returned to virt-filesystems is not reverse
translated, so it returns the appliance name (/dev/sdb2) not the
translated name (/dev/sda2) of the partition containing the Fedora PV.
I will try for a patch which does reverse translation on this field.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
1 year, 4 months
[libnbd PATCH 0/2] Fix docs and testing of completion callback
by Eric Blake
This is my proposal for fixing the documentation to match practice
(namely, that completion.callback is not invoked in the cases where
the aio call itself reports errors); we could instead try to go the
other direction and tweak the generator to guarantee that both
completion.callback and completion.free are reached no matter what,
but that felt more invasive to me.
Eric Blake (2):
api: Tighten rules on completion.callback
tests: Add coverage of nbd_aio_opt_* in wrong state
docs/libnbd.pod | 26 +++--
lib/handle.c | 3 +
lib/opt.c | 2 +
tests/Makefile.am | 5 +
tests/closure-lifetimes.c | 14 +++
tests/errors-not-negotiating-aio.c | 170 +++++++++++++++++++++++++++++
.gitignore | 1 +
7 files changed, 211 insertions(+), 10 deletions(-)
create mode 100644 tests/errors-not-negotiating-aio.c
--
2.41.0
1 year, 4 months
[libnbd PATCH] api: Fix block status assertion under set_strict bypass
by Eric Blake
A compliant server should not send NBD_REPLY_TYPE_BLOCK_STATUS unless
we successfully negotiated a meta context. And our default strictness
settings refuse to let us send NBD_CMD_BLOCK_STATUS unless we
negotiated a meta context. But when you mix non-default settings
(using nbd_set_strict to disable STRICT_COMMANDS) to send a block
status without having negotiated it, coupled with a non-compliant
server that responds with status anyways, we can then hit the
assertion failure where h->meta_valid is not set during the
REPLY.CHUNK_REPLY.RECV_BS_ENTRIES state.
Demonstration of the assertion failure can be done with a quick patch
to nbdkit (here, on top of v1.35.6):
| diff --git i/server/protocol.c w/server/protocol.c
| index cb530e65..6b115d99 100644
| --- i/server/protocol.c
| +++ w/server/protocol.c
| @@ -190,7 +190,7 @@ validate_request (uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
| }
|
| /* Block status allowed? */
| - if (cmd == NBD_CMD_BLOCK_STATUS) {
| + if (cmd == NBD_CMD_BLOCK_STATUS && 0) {
| if (!conn->structured_replies) {
| nbdkit_error ("invalid request: "
| "%s: structured replies was not negotiated",
| @@ -536,7 +536,7 @@ send_structured_reply_block_status (uint64_t cookie,
| size_t i;
| int r;
|
| - assert (conn->meta_context_base_allocation);
| + // assert (conn->meta_context_base_allocation);
| assert (cmd == NBD_CMD_BLOCK_STATUS);
|
| blocks = extents_to_block_descriptors (extents, flags, count, offset,
plus this sequence:
$ patched/nbdkit memory 1M
$ ./run nbdsh --opt-mode -u nbd://localhost
nbd> h.set_request_meta_context(False)
nbd> h.set_export_name('a')
nbd> def c(arg):
... pass
...
nbd> h.opt_set_meta_context_queries(['base:allocation'], c)
1
nbd> h.set_export_name('')
nbd> h.opt_go()
nbd> h.set_strict_mode(0)
nbd> h.block_status(1024*1024, 0, c)
nbdsh: generator/states-reply-chunk.c:425: enter_STATE_REPLY_CHUNK_REPLY_RECV_BS_ENTRIES: Assertion `h->meta_valid' failed.
Aborted (core dumped)
As this is not a typical setup, and is trivially avoided by sticking
to default settings (or more safely, by using TLS connections to
trusted servers that don't reply to a spurious block status call),
this did not warrant a CVE. However, since it is a case where a
server's reply can prompt a libnbd denial of service crash, I will be
sending a security announcement and upcoming patch be to the
libnbd-security man page once backports are in place.
Fixes: 55b09667 ("api: Fix nbd_can_meta_context if NBD_OPT_SET_META_CONTEXT fails", v1.15.3)
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
generator/states-reply-chunk.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/generator/states-reply-chunk.c b/generator/states-reply-chunk.c
index 02c65414..26b8a6b0 100644
--- a/generator/states-reply-chunk.c
+++ b/generator/states-reply-chunk.c
@@ -101,6 +101,7 @@ REPLY.CHUNK_REPLY.START:
case NBD_REPLY_TYPE_BLOCK_STATUS:
if (cmd->type != NBD_CMD_BLOCK_STATUS ||
+ !h->meta_valid || h->meta_contexts.len == 0 ||
length < 12 || ((length-4) & 7) != 0)
goto resync;
assert (CALLBACK_IS_NOT_NULL (cmd->cb.fn.extent));
--
2.41.0
1 year, 4 months
LIBNBD SECURITY: Assertion failure with unexpected block status
by Eric Blake
We have discovered a security flaw with potential minor impact in
libnbd.
Lifecycle
---------
Reported: 2023-07-14 Fixed: 2023-07-15 Published: 2023-07-17
This was not deemed severe enough to warrant a CVE: even though a
malicious server can cause libnbd to crash in a specific scenario, it
requires the client to have first invoked non-default setup, typically
used only during integration testing by developers aware of the risks
of protocol non-compliance.
Credit
------
Reported and patched by Eric Blake <eblake(a)redhat.com>
Description
-----------
libnbd is a Network Block Device (NBD) client library. The NBD
protocol states that a client should not request block status
information from a server without first negotiating that feature;
however, for interoperability testing of server behavior, libnbd
allows a client to make requests that are not compliant with the
protocol. The intent is that even when libnbd is used to trigger a
protocol violation, it will still gracefully handle whatever the
server may return (even if by disconnecting from the server).
However, a flaw in the logic for validating block status responses
meant that a server that does not follow the usual practice of
replying with an NBD_EINVAL error to an unexpected command can crash
libnbd with an assertion failure when libnbd is used to send an
unexpected block status request, rather than the intended behavior of
diagnosing the server's unusual response.
Test if libnbd is vulnerable
----------------------------
As the crash can only occur when coupling non-default settings in
libnbd with non-typical server behavior, there is no easy test for the
vulnerability. The patch includes instructions for reproducing the
crash by modifying nbdkit to behave as a non-typical server.
Workarounds
-----------
By default, libnbd strives to avoid violating the NBD protocol; this
particular crash can only be triggered when specifically using
nbd_set_strict_mode(3) to bypass libnbd's default protections, and use
of this API is not recommended except when doing integration tests of
a server's error handling behaviors. Likewise, the crash depends on a
server responding to a client error differently than recommended by
NBD protocol; it is always a wise idea to use TLS to ensure your
libnbd client is connecting to a server with known properties as a way
to avoid any potential problems where libnbd might mishandle an
unexpected server response.
If you intend to use libnbd to probe for server compliance, it is
recommended to apply the fix or upgrade to a fixed version.
Fixes
-----
The flaw was introduced in libnbd 1.15.3 (commit 55b0966706), when
adding support for manual control over meta-context negotation via
nbd_opt_set_meta_context(3). A fix for the overly-strict assertion is
available for the affected stable branch and the current development
branch.
* development branch (1.17)
https://gitlab.com/nbdkit/libnbd/-/commit/653f9c211da6943ab00a1fa665f0b8f...
or use libnbd >= 1.17.3 from
http://download.libguestfs.org/libnbd/1.17-development/
* stable branch 1.16
https://gitlab.com/nbdkit/libnbd/-/commit/72b4c8622b25589526be19ba82443ee...
or use libnbd >= 1.16.3 from
http://download.libguestfs.org/libnbd/1.16-stable/
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
1 year, 4 months
[libguestfs PATCH 0/7] lib: support networking with passt
by Laszlo Ersek
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2184967
This series makes both backends prefer passt over slirp for appliance
networking, if QEMU or libvirt (respectively) is recent enough, and
passt is installed.
My test setup is:
$ virt-builder fedora-38
Then, each test run looks like this:
Terminal#1:
$ ./run rescue/virt-rescue --network --ro -a fedora-38.img
Terminal#2:
- check if "passt" is running (ps -ef, pgrep, ...), provided it *should*
be running
Terminal#1:
><rescue> ping -c 3 8.8.8.8
><rescue> ip neighbor
><rescue> ip addr
><rescue> ip route
Expected results (in the above order), always on the "eth0" lines:
- all three pings succeed (get replies)
- 52:56:00:00:00:02
- 169.254.2.15/16
- default via 169.254.2.2
Actual results:
(1) At current master (13c7052ff96d):
(1.1) With "LIBGUESTFS_BACKEND=direct":
Pass, this is the baseline.
(1.2) With "LIBGUESTFS_BACKEND=libvirt":
Pass, this is the baseline.
(2) With this series applied:
(2.1) With passt not installed:
(2.1.1) With "LIBGUESTFS_BACKEND=direct":
Pass, this is a regression test concerning the absence of
"passt".
(2.1.2) With "LIBGUESTFS_BACKEND=libvirt":
Pass, this is a regression test concerning the absence of
"passt".
(2.2) With passt installed:
(2.2.1) With "LIBGUESTFS_BACKEND=direct":
Pass, this verifies the new feature.
(2.2.2) With "LIBGUESTFS_BACKEND=libvirt":
Basic connectivity works fine (i.e., the pings).
The "ip neighbor" and "ip route" checks fail. In addition, the
"ip addr" check *partially* fails. All that is due to libvirt
bugs:
(a) Libvirt specifies the *guest MAC* (virtio-net MAC) as the
*host MAC* for passt, with a wrong "--mac-addr" option (from
libvirt commit a56f0168d576, "qemu: hook up passt config to
qemu domains", 2023-01-10). This breaks "ip neighbor".
(b) Libvirt doesn't pass the "--gateway" option to passt. This
breaks "ip route". Namely, passt (following its default
behavior) sets the guest's gateway to 192.168.0.1, which is
the gateway for my *host*.
(c) "ip addr" also reports "169.254.2.15/1". The IP address is
fine, but the netmask is wrong; it's not /16. This is most
likely a consequence of (b) -- normally the gateway is
supposed to be on the same Ethernet segment (subnet) as the
endpoint! 192 decimal is 11000000 binary, while 169 decimal
is 10101001 binary, and their longest common initial bit
sequence is just 1 bit -- hence the /1, most likely.
I've filed <https://bugzilla.redhat.com/show_bug.cgi?id=2222766>
about these.
The upshot is that "appliance networking with passt" works with either
backend, but with the libvirt backend, there are some appliance-visible
differences from the SLIRP environment. Whether that's a problem in
practice, I can't tell (probably not a problem) -- dependent on future
decisions about RHBZ#2222766, we might want to update the libvirt
backend code introduced here.
Laszlo
Laszlo Ersek (7):
lib: fix NETWORK_ADDRESS: make it an actual IP address, not a subnet
base
lib/launch-libvirt: support networking with passt
docs: fix broken link in the guestfs manual
docs: clarify sockdir's separation
lib: move guestfs_int_create_socketname() from "launch.c" to
"tmpdirs.c"
lib: introduce guestfs_int_make_pid_path()
lib/launch-direct: support networking with passt
fish/guestfish.pod | 4 +-
generator/actions_properties.ml | 8 +-
lib/guestfs-internal.h | 32 ++++-
lib/guestfs.pod | 6 +-
lib/launch-direct.c | 147 +++++++++++++++++++-
lib/launch-libvirt.c | 11 ++
lib/launch.c | 54 +++----
lib/tmpdirs.c | 41 ++++++
8 files changed, 263 insertions(+), 40 deletions(-)
base-commit: 13c7052ff96d5ee99ec1b1252f1a3b4d7aed44d2
1 year, 4 months
