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
>
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.