On Fri, Jul 14, 2023 at 12:18:21PM +0200, Laszlo Ersek wrote: > 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 <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 (¶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");
Yes that's better actually. Rich. > > > > > Apart from that: > > > > Reviewed-by: Richard W.M. Jones <rjo...@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@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-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 _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
