* [PATCH] net: print a more actionable error when slirp is not found @ 2022-09-29 16:32 marcandre.lureau 2022-09-30 19:47 ` Christian Schoenebeck 0 siblings, 1 reply; 4+ messages in thread From: marcandre.lureau @ 2022-09-29 16:32 UTC (permalink / raw) To: qemu-devel; +Cc: jb-gnumlists, thuth, jasowang, Marc-André Lureau From: Marc-André Lureau <marcandre.lureau@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.lureau@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 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, -- 2.37.3 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: print a more actionable error when slirp is not found 2022-09-29 16:32 [PATCH] net: print a more actionable error when slirp is not found marcandre.lureau @ 2022-09-30 19:47 ` Christian Schoenebeck 2022-10-02 13:49 ` Marc-André Lureau 0 siblings, 1 reply; 4+ messages in thread From: Christian Schoenebeck @ 2022-09-30 19:47 UTC (permalink / raw) To: qemu-devel; +Cc: jb-gnumlists, thuth, jasowang, Marc-André Lureau On Donnerstag, 29. September 2022 18:32:37 CEST Marc-André Lureau wrote: > From: Marc-André Lureau <marcandre.lureau@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.lureau@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? 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. > > 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, ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: print a more actionable error when slirp is not found 2022-09-30 19:47 ` Christian Schoenebeck @ 2022-10-02 13:49 ` Marc-André Lureau 2022-10-13 10:14 ` Jakob Bohm 0 siblings, 1 reply; 4+ messages in thread From: Marc-André Lureau @ 2022-10-02 13:49 UTC (permalink / raw) To: Christian Schoenebeck; +Cc: qemu-devel, jb-gnumlists, thuth, jasowang Hi On Fri, Sep 30, 2022 at 11:49 PM Christian Schoenebeck <qemu_oss@crudebyte.com> wrote: > > On Donnerstag, 29. September 2022 18:32:37 CEST Marc-André Lureau wrote: > > From: Marc-André Lureau <marcandre.lureau@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.lureau@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. > > > > > 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, > > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: print a more actionable error when slirp is not found 2022-10-02 13:49 ` Marc-André Lureau @ 2022-10-13 10:14 ` Jakob Bohm 0 siblings, 0 replies; 4+ messages in thread From: Jakob Bohm @ 2022-10-13 10:14 UTC (permalink / raw) To: Marc-André Lureau, Christian Schoenebeck; +Cc: qemu-devel, thuth, jasowang On 02/10/2022 15:49, Marc-André Lureau wrote: > Hi > > On Fri, Sep 30, 2022 at 11:49 PM Christian Schoenebeck > <qemu_oss@crudebyte.com> wrote: >> On Donnerstag, 29. September 2022 18:32:37 CEST Marc-André Lureau wrote: >>> From: Marc-André Lureau <marcandre.lureau@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.lureau@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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-10-13 10:56 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-09-29 16:32 [PATCH] net: print a more actionable error when slirp is not found marcandre.lureau 2022-09-30 19:47 ` Christian Schoenebeck 2022-10-02 13:49 ` Marc-André Lureau 2022-10-13 10:14 ` Jakob Bohm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).