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 <ler...@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 (&params->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 (&params->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?

Apart from that:

Reviewed-by: Richard W.M. Jones <rjo...@redhat.com>

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-constructor-work-in-a-static-library
>   */
> 
> _______________________________________________
> Libguestfs mailing list
> Libguestfs@redhat.com
> https://listman.redhat.com/mailman/listinfo/libguestfs

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to