5:18 a.m.
On 7/14/23 11:40, Richard W.M. Jones wrote:
On Thu, Jul 13, 2023 at 07:10:47PM +0200, Laszlo Ersek wrote:
> 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(a)redhat.com>
> ---
> lib/guestfs-internal.h | 1 +
> lib/launch-libvirt.c | 11 ++++++++
> lib/launch.c | 28 ++++++++++++++++++++
> 3 files changed, 40 insertions(+)
>
> diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
> index 4be351e3d3cc..a6c4266ad3fe 100644
> --- a/lib/guestfs-internal.h
> +++ b/lib/guestfs-internal.h
> @@ -703,6 +703,7 @@ extern void guestfs_int_unblock_sigterm (void);
> extern int guestfs_int_create_socketname (guestfs_h *g, const char *filename, char
(*sockname)[UNIX_PATH_MAX]);
> extern void guestfs_int_register_backend (const char *name, const struct backend_ops
*);
> extern int guestfs_int_set_backend (guestfs_h *g, const char *method);
> +extern bool guestfs_int_passt_runnable (guestfs_h *g);
>
> /* Close all file descriptors matching the condition. */
> #define close_file_descriptors(cond) do { \
> diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
> index d4bf1a8ff242..994909a35f2d 100644
> --- a/lib/launch-libvirt.c
> +++ b/lib/launch-libvirt.c
> @@ -1414,6 +1414,17 @@ construct_libvirt_xml_devices (guestfs_h *g,
> guestfs_int_version_ge (¶ms->data->libvirt_version, 3, 8, 0))
{
> start_element ("interface") {
> attribute ("type", "user");
> + /* If libvirt is 9.0.0+ and "passt" is available, ask for passt
rather
> + * than SLIRP (RHBZ#2184967). Note that this causes some
> + * appliance-visible changes (although network connectivity is certainly
> + * functional); refer to RHBZ#2222766 about those.
> + */
> + if (guestfs_int_version_ge (¶ms->data->libvirt_version, 9, 0,
0) &&
> + guestfs_int_passt_runnable (g)) {
> + start_element ("backend") {
> + attribute ("type", "passt");
> + } end_element ();
> + }
> start_element ("model") {
> attribute ("type", "virtio");
> } end_element ();
> diff --git a/lib/launch.c b/lib/launch.c
> index 6e08b12006da..94c8f676d8bd 100644
> --- a/lib/launch.c
> +++ b/lib/launch.c
> @@ -36,6 +36,7 @@
> #include <signal.h>
> #include <sys/stat.h>
> #include <sys/types.h>
> +#include <sys/wait.h>
> #include <errno.h>
> #include <assert.h>
> #include <libintl.h>
> @@ -414,6 +415,33 @@ guestfs_int_set_backend (guestfs_h *g, const char *method)
> return 0;
> }
>
> +/**
> + * Return C<true> if we can call S<C<passt --help>>, and it exits
with status
> + * C<0> or C<1>.
> + *
> + * (At least C<passt-0^20230222.g4ddbcb9-2.el9_2.x86_64> terminates with
status
> + * C<1> in response to "--help", which is arguably wrong, and
potentially
> + * subject to change, but it doesn't really matter.)
> + */
> +bool
> +guestfs_int_passt_runnable (guestfs_h *g)
> +{
> + CLEANUP_CMD_CLOSE struct command *cmd = NULL;
> + int r, ex;
> +
> + cmd = guestfs_int_new_command (g);
> + if (cmd == NULL)
> + return false;
> +
> + guestfs_int_cmd_add_string_unquoted (cmd, "passt --help");
Do we need " >/dev/null 2>&1" here to avoid unnecessary messages
being
printed when libguestfs is not in verbose mode?
Yes. Thanks for pointing it out. I didn't quite know what to do with the
stdout / stderr of "passt --help".
So is the following pattern the right one?
guestfs_int_cmd_add_string_unquoted (cmd, "passt --help");
if (!g->verbose)
guestfs_int_cmd_add_string_unquoted (cmd, " >/dev/null 2>&1");
Apart from that:
Reviewed-by: Richard W.M. Jones <rjones(a)redhat.com>
Thanks!
Laszlo
Rich.
> + r = guestfs_int_cmd_run (cmd);
> + if (r == -1 || !WIFEXITED (r))
> + return false;
> +
> + ex = WEXITSTATUS (r);
> + return ex == 0 || ex == 1;
> +}
> +
> /* This hack is only required to make static linking work. See:
> *
https://stackoverflow.com/questions/1202494/why-doesnt-attribute-construc...
> */
>
> _______________________________________________
> Libguestfs mailing list
> Libguestfs(a)redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs