[Libguestfs] [libguestfs PATCH 2/7] lib/launch-libvirt: support networking with passt

[Laszlo Ersek]

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 at 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?

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