On 02/10/2022 15:49, Marc-André Lureau wrote:
Hi

On Fri, Sep 30, 2022 at 11:49 PM Christian Schoenebeck
<qemu_...@crudebyte.com> wrote:
On Donnerstag, 29. September 2022 18:32:37 CEST Marc-André Lureau wrote:
From: Marc-André Lureau <marcandre.lur...@redhat.com>

If slirp is not found during compile-time, and not manually disabled,
print a friendly error message, as suggested in the "If your networking
is failing after updating to the latest git version of QEMU..." thread
by various people.

Signed-off-by: Marc-André Lureau <marcandre.lur...@redhat.com>
---
  meson.build |  4 ++++
  net/net.c   | 19 +++++++++++++++++--
  2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index 8dc661363f..4f69d7d0b4 100644
--- a/meson.build
+++ b/meson.build
@@ -657,6 +657,10 @@ if not get_option('slirp').auto() or have_system
    endif
  endif

+if get_option('slirp').disabled()
+  config_host_data.set('CONFIG_SLIRP_DISABLED', true)
+endif
+
  vde = not_found
  if not get_option('vde').auto() or have_system or have_tools
    vde = cc.find_library('vdeplug', has_headers: ['libvdeplug.h'],
diff --git a/net/net.c b/net/net.c
index 2db160e063..e6072a5ddd 100644
--- a/net/net.c
+++ b/net/net.c
@@ -990,14 +990,29 @@ static int net_init_nic(const Netdev *netdev, const
char *name, return idx;
  }

+#if (defined(CONFIG_SLIRP) || !defined(CONFIG_SLIRP_DISABLED))
+static int net_init_user(const Netdev *netdev, const char *name,
+                         NetClientState *peer, Error **errp)
+{
+#ifdef CONFIG_SLIRP
+    return net_init_slirp(netdev, name, peer, errp);
+#else
+    error_setg(errp,
+               "Type 'user' is not a supported netdev backend by this QEMU
build " +               "because the libslirp development files were not
found during build " +               "of QEMU.");
+#endif
+    return -1;
+}
+#endif
I just tried this, but somehow it is not working for me. net_init_user() is
never called and therefore I don't get the error message. That should be
working if the user launched QEMU without any networking arg, right?

That's because vl.c has:
if (default_net) {
...
#ifdef CONFIG_SLIRP
         qemu_opts_parse(net, "user", true, &error_abort);
#endif

Iow, it doesn't try to use slirp by default if it's not found at
compile time. We can eventually change that, but that might break
existing users who don't build with slirp.

Alternatively, it could error out only if slirp was not explicitly
disabled at configure time.

And still, I would find it better if there was also a clear build-time error
if there was no libslirp and slirp feature was not explicitly disabled.
That's not the typical way we deal with dependencies, but I can try to
do that as well.
Maybe change that ifdef section to report the error early instead of introducing the new
helper function, something like

 #ifdef CONFIG_SLIRP
         qemu_opts_parse(net, "user", true, &error_abort);
+        // Explicit error messages, because it is not obvious to users that
+        //     "user" networking is based on code from libslirp.
+#elif !defined(CONFIG_SLIRP_DISABLED))
+        some_error_function(
+            "Type 'user' is not a supported netdev backend by this QEMU build "
+            "because the libslirp development files were not found during build 
"
+            "of QEMU.");
+#else
+        some_error_function(
+            "Type 'user' is not a supported netdev backend by this QEMU build "
+            "because QEMU was explicitly built without libslirp");
 #endif

Also output these messages when the user backend is explicitly requested and not CONFIG_SLIRP.

  static int (* const net_client_init_fun[NET_CLIENT_DRIVER__MAX])(
      const Netdev *netdev,
      const char *name,
      NetClientState *peer, Error **errp) = {
          [NET_CLIENT_DRIVER_NIC]       = net_init_nic,
-#ifdef CONFIG_SLIRP
-        [NET_CLIENT_DRIVER_USER]      = net_init_slirp,
+#if (defined(CONFIG_SLIRP) || !defined(CONFIG_SLIRP_DISABLED))
+        [NET_CLIENT_DRIVER_USER]      = net_init_user,
  #endif
          [NET_CLIENT_DRIVER_TAP]       = net_init_tap,
          [NET_CLIENT_DRIVER_SOCKET]    = net_init_socket,



Enjoy

Jakob
--
Jakob Bohm, CIO, Partner, WiseMo A/S.  http://www.wisemo.com
Transformervej 29, 2860 Soborg, Denmark.  Direct +45 31 13 16 10
This public discussion message is non-binding and may contain errors.
WiseMo - Remote Service Management for PCs, Phones and Embedded


Reply via email to