[PATCH v2] build-sys: error when slirp is not found and not disabled

marcandre.lureau@redhat.com posted 1 patch 1 year, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20221006083322.2612639-1-marcandre.lureau@redhat.com
meson.build | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH v2] build-sys: error when slirp is not found and not disabled
Posted by marcandre.lureau@redhat.com 1 year, 6 months ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

This is an alternative configure-time solution to "[PATCH] net:
print a more actionable error when slirp is not found".

See also "If your networking is failing after updating to the latest git
version of QEMU..." ML thread.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 meson.build | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/meson.build b/meson.build
index 4321b8f8da..b05080b051 100644
--- a/meson.build
+++ b/meson.build
@@ -690,6 +690,13 @@ if not get_option('slirp').auto() or have_system
   endif
 endif
 
+# Remove this error after QEMU 8.1 has been released.
+if not get_option('slirp').disabled() and not slirp.found()
+  error('libslirp is not explicitely disabled and was not found. ' +
+        'Since qemu 7.2, libslirp is no longer included as a submodule ' +
+        'fallback, you must install it on your system or --disable-slirp.')
+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'],
-- 
2.37.3


Re: [PATCH v2] build-sys: error when slirp is not found and not disabled
Posted by Christian Schoenebeck 1 year, 6 months ago
On Donnerstag, 6. Oktober 2022 10:33:22 CEST marcandre.lureau@redhat.com 
wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This is an alternative configure-time solution to "[PATCH] net:
> print a more actionable error when slirp is not found".
> 
> See also "If your networking is failing after updating to the latest git
> version of QEMU..." ML thread.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  meson.build | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 4321b8f8da..b05080b051 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -690,6 +690,13 @@ if not get_option('slirp').auto() or have_system
>    endif
>  endif
> 
> +# Remove this error after QEMU 8.1 has been released.
> +if not get_option('slirp').disabled() and not slirp.found()
> +  error('libslirp is not explicitely disabled and was not found. ' +

"explicitly", except of that:

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

> +        'Since qemu 7.2, libslirp is no longer included as a submodule ' +
> +        'fallback, you must install it on your system or --disable-slirp.')
> +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'],
Re: [PATCH v2] build-sys: error when slirp is not found and not disabled
Posted by Daniel P. Berrangé 1 year, 6 months ago
On Thu, Oct 06, 2022 at 12:33:22PM +0400, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This is an alternative configure-time solution to "[PATCH] net:
> print a more actionable error when slirp is not found".
> 
> See also "If your networking is failing after updating to the latest git
> version of QEMU..." ML thread.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  meson.build | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 4321b8f8da..b05080b051 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -690,6 +690,13 @@ if not get_option('slirp').auto() or have_system
>    endif
>  endif
>  
> +# Remove this error after QEMU 8.1 has been released.
> +if not get_option('slirp').disabled() and not slirp.found()
> +  error('libslirp is not explicitely disabled and was not found. ' +
> +        'Since qemu 7.2, libslirp is no longer included as a submodule ' +
> +        'fallback, you must install it on your system or --disable-slirp.')
> +endif
> +

I'm still not convinced we should be making this a fatal error, as
opposed to treating it as a warning we display at the end of meson
execution, which is what we do in other cases where we want to
alert users to something important about their build environment.


We have this for example:

  message()
  warning('SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!')
  message()
  message('CPU host architecture ' + cpu + ' support is not currently maintained.')
  message('The QEMU project intends to remove support for this host CPU in')
  message('a future release if nobody volunteers to maintain it and to')
  message('provide a build host for our continuous integration setup.')
  message('configure has succeeded and you can continue to build, but')
  message('if you care about QEMU on this platform you should contact')
  message('us upstream at qemu-devel@nongnu.org.')


This is just as important to show users as the slirp case IMHO, so
it isn't clear why this approach is insufficient for slirp too.


One irritation though, is that there's no way to get this text to
display *after* meson prints the summary() data, so it is likely
scrolled off the screen.

I think 'summary()'  ought to have a way to register warning messages
that are guaranteed to be the last thing printed, with boldness.

In absence of that, we can partially mitigate this by using a custom
summary section though.

Consider this:

@@ -3936,3 +3937,20 @@ if not supported_oses.contains(targetos)
   message('if you care about QEMU on this platform you should contact')
   message('us upstream at qemu-devel@nongnu.org.')
 endif
+
+warning_info = {}
+
+# Remove this warning after QEMU 8.1 has been released.
+if not get_option('slirp').disabled() and not slirp.found()
+    warning_info += {'SLIRP': 'libslirp not present, "user" network backend will not be available'}
+    message()
+    warning('libslirp not present, "user" network backend will not be available')
+    message()
+    message('Since qemu 7.2, libslirp is no longer included as a submodule')
+    message('fallback, you must install it on your system if you require')
+    message('-netdev user / -net user to be a supported network backend')
+    message()
+endif
+
+summary(warning_info, bool_yn: true,
+        section: 'WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS')

Would mean that meson/configures ends  with:




Message:
../meson.build:3946: WARNING: libslirp not present, "user" network backend will not be available
Message:
Message: Since qemu 7.2, libslirp is no longer included as a submodule
Message: fallback, you must install it on your system if you require
Message: -netdev user / -net user to be a supported network backend
Message:
Build targets in project: 576

qemu 7.1.50

  Directories
    Install prefix               : /usr/local
    BIOS directory               : share/qemu
    firmware path                : share/qemu-firmware

...snip a page or two more summary...

    zstd support                 : YES 1.5.2
    NUMA host support            : YES
    capstone                     : YES 4.0.2
    libpmem support              : YES 1.11.1
    libdaxctl support            : YES 74
    libudev                      : YES 250
    FUSE lseek                   : YES
    selinux                      : YES 3.3

  WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS
    SLIRP                        : libslirp not present, "user" network backend will not be available

  Subprojects
    libvduse                     : YES
    libvhost-user                : YES

  User defined options
    Native files                 : config-meson.cross
    prefix                       : /usr/local
    werror                       : true
    vfio_user_server             : disabled

Found ninja-1.10.2 at /usr/bin/ninja
Running postconf script '/usr/bin/python3 /home/berrange/src/virt/qemu/scripts/symlink-install-tree.py'



Is that really not sufficiently alerting to users that they might be
loosing the 'user' network feature ?

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v2] build-sys: error when slirp is not found and not disabled
Posted by Christian Schoenebeck 1 year, 6 months ago
On Donnerstag, 6. Oktober 2022 11:39:02 CEST Daniel P. Berrangé wrote:
> On Thu, Oct 06, 2022 at 12:33:22PM +0400, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > 
> > This is an alternative configure-time solution to "[PATCH] net:
> > print a more actionable error when slirp is not found".
> > 
> > See also "If your networking is failing after updating to the latest git
> > version of QEMU..." ML thread.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > 
> >  meson.build | 7 +++++++
> >  1 file changed, 7 insertions(+)
> > 
> > diff --git a/meson.build b/meson.build
> > index 4321b8f8da..b05080b051 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -690,6 +690,13 @@ if not get_option('slirp').auto() or have_system
> > 
> >    endif
> >  
> >  endif
> > 
> > +# Remove this error after QEMU 8.1 has been released.
> > +if not get_option('slirp').disabled() and not slirp.found()
> > +  error('libslirp is not explicitely disabled and was not found. ' +
> > +        'Since qemu 7.2, libslirp is no longer included as a submodule '
> > +
> > +        'fallback, you must install it on your system or
> > --disable-slirp.') +endif
> > +
> 
> I'm still not convinced we should be making this a fatal error, as
> opposed to treating it as a warning we display at the end of meson
> execution, which is what we do in other cases where we want to
> alert users to something important about their build environment.
> 
> We have this for example:
>   message()
>   warning('SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!')
>   message()
>   message('CPU host architecture ' + cpu + ' support is not currently
>   maintained.') message('The QEMU project intends to remove support for
>   this host CPU in') message('a future release if nobody volunteers to
>   maintain it and to') message('provide a build host for our continuous
>   integration setup.') message('configure has succeeded and you can
>   continue to build, but') message('if you care about QEMU on this platform
>   you should contact') message('us upstream at qemu-devel@nongnu.org.')
> 
> This is just as important to show users as the slirp case IMHO, so
> it isn't clear why this approach is insufficient for slirp too.

There is a substantial difference between those two cases: the user
immediately realizes that the missing CPU arch is not available, whereas in
this missing SLIRP case it happened to myself that I was just testing a VM
(without any networking args) and scratched my head why networking on VM
stopped working without any error message at all, so I started to check my
networking, routing, VM network config, host config, found nothing, and
eventually git-bisected the issue just to find out that its because slirp has
been removed as a submodule. And I am a developer, so imagine what a regular
user might think.

And yes, with that warning it would have saved my trouble at least, but 
considering that binaries are often auto built, chances are high that they are
at first rolled out without SLIRP and hence without user networking, and hence
still causing those wondering moments to users.

> One irritation though, is that there's no way to get this text to
> display *after* meson prints the summary() data, so it is likely
> scrolled off the screen.
> 
> I think 'summary()'  ought to have a way to register warning messages
> that are guaranteed to be the last thing printed, with boldness.
> 
> In absence of that, we can partially mitigate this by using a custom
> summary section though.
> 
> Consider this:
> 
> @@ -3936,3 +3937,20 @@ if not supported_oses.contains(targetos)
> 
>    message('if you care about QEMU on this platform you should contact')
>    message('us upstream at qemu-devel@nongnu.org.')
>  
>  endif
> 
> +
> +warning_info = {}
> +
> +# Remove this warning after QEMU 8.1 has been released.
> +if not get_option('slirp').disabled() and not slirp.found()
> +    warning_info += {'SLIRP': 'libslirp not present, "user" network backend
> will not be available'} +    message()
> +    warning('libslirp not present, "user" network backend will not be
> available') +    message()
> +    message('Since qemu 7.2, libslirp is no longer included as a
> submodule')
> +    message('fallback, you must install it on your system if you require')
> +    message('-netdev user / -net user to be a supported network backend')
> +    message()
> +endif
> +
> +summary(warning_info, bool_yn: true,
> +        section: 'WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥
> WARNINGS')
> 
> Would mean that meson/configures ends  with:
> 
> 
> 
> 
> Message:
> ../meson.build:3946: WARNING: libslirp not present, "user" network backend
> will not be available Message:
> Message: Since qemu 7.2, libslirp is no longer included as a submodule
> Message: fallback, you must install it on your system if you require
> Message: -netdev user / -net user to be a supported network backend
> Message:
> Build targets in project: 576
> 
> qemu 7.1.50
> 
>   Directories
>   
>     Install prefix               : /usr/local
>     BIOS directory               : share/qemu
>     firmware path                : share/qemu-firmware
> 
> ...snip a page or two more summary...
> 
>     zstd support                 : YES 1.5.2
>     NUMA host support            : YES
>     capstone                     : YES 4.0.2
>     libpmem support              : YES 1.11.1
>     libdaxctl support            : YES 74
>     libudev                      : YES 250
>     FUSE lseek                   : YES
>     selinux                      : YES 3.3
>   
>   WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS
>   
>     SLIRP                        : libslirp not present, "user" network
>     backend will not be available>   
>   Subprojects
>   
>     libvduse                     : YES
>     libvhost-user                : YES
>   
>   User defined options
>   
>     Native files                 : config-meson.cross
>     prefix                       : /usr/local
>     werror                       : true
>     vfio_user_server             : disabled
> 
> Found ninja-1.10.2 at /usr/bin/ninja
> Running postconf script '/usr/bin/python3
> /home/berrange/src/virt/qemu/scripts/symlink-install-tree.py'
> 
> 
> 
> Is that really not sufficiently alerting to users that they might be
> loosing the 'user' network feature ?

As I asked before: do you rather want to risk a backlash, binaries being auto
built, and users filing bug reports because their network no longer works and
they don't know why?

What is the real-world problem you see with making this temporarily an error
and then restoring symmetry later on?

Best regards,
Christian Schoenebeck
Re: [PATCH v2] build-sys: error when slirp is not found and not disabled
Posted by Daniel P. Berrangé 1 year, 6 months ago
On Thu, Oct 06, 2022 at 12:12:07PM +0200, Christian Schoenebeck wrote:
> On Donnerstag, 6. Oktober 2022 11:39:02 CEST Daniel P. Berrangé wrote:
> > On Thu, Oct 06, 2022 at 12:33:22PM +0400, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > 
> > > This is an alternative configure-time solution to "[PATCH] net:
> > > print a more actionable error when slirp is not found".
> > > 
> > > See also "If your networking is failing after updating to the latest git
> > > version of QEMU..." ML thread.
> > > 
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > 
> > >  meson.build | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index 4321b8f8da..b05080b051 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -690,6 +690,13 @@ if not get_option('slirp').auto() or have_system
> > > 
> > >    endif
> > >  
> > >  endif
> > > 
> > > +# Remove this error after QEMU 8.1 has been released.
> > > +if not get_option('slirp').disabled() and not slirp.found()
> > > +  error('libslirp is not explicitely disabled and was not found. ' +
> > > +        'Since qemu 7.2, libslirp is no longer included as a submodule '
> > > +
> > > +        'fallback, you must install it on your system or
> > > --disable-slirp.') +endif
> > > +
> > 
> > I'm still not convinced we should be making this a fatal error, as
> > opposed to treating it as a warning we display at the end of meson
> > execution, which is what we do in other cases where we want to
> > alert users to something important about their build environment.
> > 
> > We have this for example:
> >   message()
> >   warning('SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!')
> >   message()
> >   message('CPU host architecture ' + cpu + ' support is not currently
> >   maintained.') message('The QEMU project intends to remove support for
> >   this host CPU in') message('a future release if nobody volunteers to
> >   maintain it and to') message('provide a build host for our continuous
> >   integration setup.') message('configure has succeeded and you can
> >   continue to build, but') message('if you care about QEMU on this platform
> >   you should contact') message('us upstream at qemu-devel@nongnu.org.')
> > 
> > This is just as important to show users as the slirp case IMHO, so
> > it isn't clear why this approach is insufficient for slirp too.
> 
> There is a substantial difference between those two cases: the user
> immediately realizes that the missing CPU arch is not available, whereas in
> this missing SLIRP case it happened to myself that I was just testing a VM
> (without any networking args) and scratched my head why networking on VM
> stopped working without any error message at all, so I started to check my
> networking, routing, VM network config, host config, found nothing, and
> eventually git-bisected the issue just to find out that its because slirp has
> been removed as a submodule. And I am a developer, so imagine what a regular
> user might think.
> 
> And yes, with that warning it would have saved my trouble at least, but 
> considering that binaries are often auto built, chances are high that they are
> at first rolled out without SLIRP and hence without user networking, and hence
> still causing those wondering moments to users.
> 
> > One irritation though, is that there's no way to get this text to
> > display *after* meson prints the summary() data, so it is likely
> > scrolled off the screen.
> > 
> > I think 'summary()'  ought to have a way to register warning messages
> > that are guaranteed to be the last thing printed, with boldness.
> > 
> > In absence of that, we can partially mitigate this by using a custom
> > summary section though.
> > 
> > Consider this:
> > 
> > @@ -3936,3 +3937,20 @@ if not supported_oses.contains(targetos)
> > 
> >    message('if you care about QEMU on this platform you should contact')
> >    message('us upstream at qemu-devel@nongnu.org.')
> >  
> >  endif
> > 
> > +
> > +warning_info = {}
> > +
> > +# Remove this warning after QEMU 8.1 has been released.
> > +if not get_option('slirp').disabled() and not slirp.found()
> > +    warning_info += {'SLIRP': 'libslirp not present, "user" network backend
> > will not be available'} +    message()
> > +    warning('libslirp not present, "user" network backend will not be
> > available') +    message()
> > +    message('Since qemu 7.2, libslirp is no longer included as a
> > submodule')
> > +    message('fallback, you must install it on your system if you require')
> > +    message('-netdev user / -net user to be a supported network backend')
> > +    message()
> > +endif
> > +
> > +summary(warning_info, bool_yn: true,
> > +        section: 'WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥
> > WARNINGS')
> > 
> > Would mean that meson/configures ends  with:
> > 
> > 
> > 
> > 
> > Message:
> > ../meson.build:3946: WARNING: libslirp not present, "user" network backend
> > will not be available Message:
> > Message: Since qemu 7.2, libslirp is no longer included as a submodule
> > Message: fallback, you must install it on your system if you require
> > Message: -netdev user / -net user to be a supported network backend
> > Message:
> > Build targets in project: 576
> > 
> > qemu 7.1.50
> > 
> >   Directories
> >   
> >     Install prefix               : /usr/local
> >     BIOS directory               : share/qemu
> >     firmware path                : share/qemu-firmware
> > 
> > ...snip a page or two more summary...
> > 
> >     zstd support                 : YES 1.5.2
> >     NUMA host support            : YES
> >     capstone                     : YES 4.0.2
> >     libpmem support              : YES 1.11.1
> >     libdaxctl support            : YES 74
> >     libudev                      : YES 250
> >     FUSE lseek                   : YES
> >     selinux                      : YES 3.3
> >   
> >   WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS
> >   
> >     SLIRP                        : libslirp not present, "user" network
> >     backend will not be available>   
> >   Subprojects
> >   
> >     libvduse                     : YES
> >     libvhost-user                : YES
> >   
> >   User defined options
> >   
> >     Native files                 : config-meson.cross
> >     prefix                       : /usr/local
> >     werror                       : true
> >     vfio_user_server             : disabled
> > 
> > Found ninja-1.10.2 at /usr/bin/ninja
> > Running postconf script '/usr/bin/python3
> > /home/berrange/src/virt/qemu/scripts/symlink-install-tree.py'
> > 
> > 
> > 
> > Is that really not sufficiently alerting to users that they might be
> > loosing the 'user' network feature ?
> 
> As I asked before: do you rather want to risk a backlash, binaries being auto
> built, and users filing bug reports because their network no longer works and
> they don't know why?

The current status is that there's no visible warning - just an
easily missing line in configure outpout  "slirp: no". I accept
that is not sufficiently visible as it.

What i'm questioning is the length we have to go to in order to
make it more visible to users.

Implicitly you are suggesting that no one bothers to look at the output
of configure/meson, because if they did, then IMHO the suggested extra
visible warning messages would handle this scenario sufficiently well.


> What is the real-world problem you see with making this temporarily an error
> and then restoring symmetry later on?

This is a divergance from our normal precedent for handling optional
build dependencies, and it is not clear that is is justified. We have
a deprecation process for dropping features, but in terms of build deps
we have always considered that we are free to change those at any time
with zero notice. We expect people to read the release notes and/or
configure output, in order to decide if they're lacking features.

Consider if we bump the minimum verson of any other library, perhaps
liburing. If a user runs configure with new QEMU, liburing support
will get automatically disabled if they host only has the older version,
meaning their QEMU build looses the io-uring features.  This is not
inherently different to the slirp case, as they have to look at the
configure output to see the missing feature, and/or read the release
notes. I don't want us to be in a situation where we have to give
extra warnings every time we make a change to expected build deps
that could result in users loosing features that their builds
previously had.

Making slirp a hard error is setting a new precedent for how to
deal with external library deps that I don't think is justified.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|


Re: [PATCH v2] build-sys: error when slirp is not found and not disabled
Posted by Christian Schoenebeck 1 year, 6 months ago
On Donnerstag, 6. Oktober 2022 12:26:50 CEST Daniel P. Berrangé wrote:
> On Thu, Oct 06, 2022 at 12:12:07PM +0200, Christian Schoenebeck wrote:
> > On Donnerstag, 6. Oktober 2022 11:39:02 CEST Daniel P. Berrangé wrote:
> > > On Thu, Oct 06, 2022 at 12:33:22PM +0400, marcandre.lureau@redhat.com 
wrote:
> > > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > 
> > > > This is an alternative configure-time solution to "[PATCH] net:
> > > > print a more actionable error when slirp is not found".
> > > > 
> > > > See also "If your networking is failing after updating to the latest
> > > > git
> > > > version of QEMU..." ML thread.
> > > > 
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > > 
> > > >  meson.build | 7 +++++++
> > > >  1 file changed, 7 insertions(+)
> > > > 
> > > > diff --git a/meson.build b/meson.build
> > > > index 4321b8f8da..b05080b051 100644
> > > > --- a/meson.build
> > > > +++ b/meson.build
> > > > @@ -690,6 +690,13 @@ if not get_option('slirp').auto() or have_system
> > > > 
> > > >    endif
> > > >  
> > > >  endif
> > > > 
> > > > +# Remove this error after QEMU 8.1 has been released.
> > > > +if not get_option('slirp').disabled() and not slirp.found()
> > > > +  error('libslirp is not explicitely disabled and was not found. ' +
> > > > +        'Since qemu 7.2, libslirp is no longer included as a
> > > > submodule '
> > > > +
> > > > +        'fallback, you must install it on your system or
> > > > --disable-slirp.') +endif
> > > > +
> > > 
> > > I'm still not convinced we should be making this a fatal error, as
> > > opposed to treating it as a warning we display at the end of meson
> > > execution, which is what we do in other cases where we want to
> > > alert users to something important about their build environment.
> > > 
> > > We have this for example:
> > >   message()
> > >   warning('SUPPORT FOR THIS HOST CPU WILL GO AWAY IN FUTURE RELEASES!')
> > >   message()
> > >   message('CPU host architecture ' + cpu + ' support is not currently
> > >   maintained.') message('The QEMU project intends to remove support for
> > >   this host CPU in') message('a future release if nobody volunteers to
> > >   maintain it and to') message('provide a build host for our continuous
> > >   integration setup.') message('configure has succeeded and you can
> > >   continue to build, but') message('if you care about QEMU on this
> > >   platform
> > >   you should contact') message('us upstream at qemu-devel@nongnu.org.')
> > > 
> > > This is just as important to show users as the slirp case IMHO, so
> > > it isn't clear why this approach is insufficient for slirp too.
> > 
> > There is a substantial difference between those two cases: the user
> > immediately realizes that the missing CPU arch is not available, whereas
> > in
> > this missing SLIRP case it happened to myself that I was just testing a VM
> > (without any networking args) and scratched my head why networking on VM
> > stopped working without any error message at all, so I started to check my
> > networking, routing, VM network config, host config, found nothing, and
> > eventually git-bisected the issue just to find out that its because slirp
> > has been removed as a submodule. And I am a developer, so imagine what a
> > regular user might think.
> > 
> > And yes, with that warning it would have saved my trouble at least, but
> > considering that binaries are often auto built, chances are high that they
> > are at first rolled out without SLIRP and hence without user networking,
> > and hence still causing those wondering moments to users.
> > 
> > > One irritation though, is that there's no way to get this text to
> > > display *after* meson prints the summary() data, so it is likely
> > > scrolled off the screen.
> > > 
> > > I think 'summary()'  ought to have a way to register warning messages
> > > that are guaranteed to be the last thing printed, with boldness.
> > > 
> > > In absence of that, we can partially mitigate this by using a custom
> > > summary section though.
> > > 
> > > Consider this:
> > > 
> > > @@ -3936,3 +3937,20 @@ if not supported_oses.contains(targetos)
> > > 
> > >    message('if you care about QEMU on this platform you should contact')
> > >    message('us upstream at qemu-devel@nongnu.org.')
> > >  
> > >  endif
> > > 
> > > +
> > > +warning_info = {}
> > > +
> > > +# Remove this warning after QEMU 8.1 has been released.
> > > +if not get_option('slirp').disabled() and not slirp.found()
> > > +    warning_info += {'SLIRP': 'libslirp not present, "user" network
> > > backend will not be available'} +    message()
> > > +    warning('libslirp not present, "user" network backend will not be
> > > available') +    message()
> > > +    message('Since qemu 7.2, libslirp is no longer included as a
> > > submodule')
> > > +    message('fallback, you must install it on your system if you
> > > require')
> > > +    message('-netdev user / -net user to be a supported network
> > > backend')
> > > +    message()
> > > +endif
> > > +
> > > +summary(warning_info, bool_yn: true,
> > > +        section: 'WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥
> > > WARNINGS')
> > > 
> > > Would mean that meson/configures ends  with:
> > > 
> > > 
> > > 
> > > 
> > > Message:
> > > ../meson.build:3946: WARNING: libslirp not present, "user" network
> > > backend
> > > will not be available Message:
> > > Message: Since qemu 7.2, libslirp is no longer included as a submodule
> > > Message: fallback, you must install it on your system if you require
> > > Message: -netdev user / -net user to be a supported network backend
> > > Message:
> > > Build targets in project: 576
> > > 
> > > qemu 7.1.50
> > > 
> > >   Directories
> > >   
> > >     Install prefix               : /usr/local
> > >     BIOS directory               : share/qemu
> > >     firmware path                : share/qemu-firmware
> > > 
> > > ...snip a page or two more summary...
> > > 
> > >     zstd support                 : YES 1.5.2
> > >     NUMA host support            : YES
> > >     capstone                     : YES 4.0.2
> > >     libpmem support              : YES 1.11.1
> > >     libdaxctl support            : YES 74
> > >     libudev                      : YES 250
> > >     FUSE lseek                   : YES
> > >     selinux                      : YES 3.3
> > >   
> > >   WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS 💥 WARNINGS
> > >   
> > >     SLIRP                        : libslirp not present, "user" network
> > >     backend will not be available>
> > >   
> > >   Subprojects
> > >   
> > >     libvduse                     : YES
> > >     libvhost-user                : YES
> > >   
> > >   User defined options
> > >   
> > >     Native files                 : config-meson.cross
> > >     prefix                       : /usr/local
> > >     werror                       : true
> > >     vfio_user_server             : disabled
> > > 
> > > Found ninja-1.10.2 at /usr/bin/ninja
> > > Running postconf script '/usr/bin/python3
> > > /home/berrange/src/virt/qemu/scripts/symlink-install-tree.py'
> > > 
> > > 
> > > 
> > > Is that really not sufficiently alerting to users that they might be
> > > loosing the 'user' network feature ?
> > 
> > As I asked before: do you rather want to risk a backlash, binaries being
> > auto built, and users filing bug reports because their network no longer
> > works and they don't know why?
> 
> The current status is that there's no visible warning - just an
> easily missing line in configure outpout  "slirp: no". I accept
> that is not sufficiently visible as it.
> 
> What i'm questioning is the length we have to go to in order to
> make it more visible to users.
> 
> Implicitly you are suggesting that no one bothers to look at the output
> of configure/meson, because if they did, then IMHO the suggested extra
> visible warning messages would handle this scenario sufficiently well.

If binaries are auto built, and they are, then yes, it might happen that 
nobody looks at your suggested warning. And as such affected users never get 
in touch with build messages, they would simply stumble in the dark.

> > What is the real-world problem you see with making this temporarily an
> > error and then restoring symmetry later on?
> 
> This is a divergance from our normal precedent for handling optional
> build dependencies, and it is not clear that is is justified. We have
> a deprecation process for dropping features, but in terms of build deps
> we have always considered that we are free to change those at any time
> with zero notice. We expect people to read the release notes and/or
> configure output, in order to decide if they're lacking features.

Yes, but that's not about deprecation. If an arch is dropped, it happpend
intentionally. In this case it's about preventing that default networking is 
disabled unintentionally by accident.

> Consider if we bump the minimum verson of any other library, perhaps
> liburing. If a user runs configure with new QEMU, liburing support
> will get automatically disabled if they host only has the older version,
> meaning their QEMU build looses the io-uring features.  This is not
> inherently different to the slirp case, as they have to look at the
> configure output to see the missing feature, and/or read the release
> notes. I don't want us to be in a situation where we have to give
> extra warnings every time we make a change to expected build deps
> that could result in users loosing features that their builds
> previously had.

OK, I currently don't know what the user impact of missing liburing would be, 
but the reason why I think that a temporary error is justified in this slirp 
case is, that it's (ATM at least) from user perspective quite horrible to find 
the root cause why this happens. I mean when you launch the VM without any 
networking args, and don't have slirp, then you currently don't get any 
runtime error at all.

If there was a clear and user-friendly runtime error already like

"ERROR: QEMU was built without user networking (https://somewhere/#help123)"

Then a build warning would probably be sufficient.

> Making slirp a hard error is setting a new precedent for how to
> deal with external library deps that I don't think is justified.

#usersfirst ;-)

Anyway, I am not entitled as QEMU bug wrangler. So if you rather want to risk 
processing bug reports for that, your call.

Best regards,
Christian Schoenebeck
Re: [PATCH v2] build-sys: error when slirp is not found and not disabled
Posted by Thomas Huth 1 year, 6 months ago
On 06/10/2022 10.33, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This is an alternative configure-time solution to "[PATCH] net:
> print a more actionable error when slirp is not found".
> 
> See also "If your networking is failing after updating to the latest git
> version of QEMU..." ML thread.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   meson.build | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 4321b8f8da..b05080b051 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -690,6 +690,13 @@ if not get_option('slirp').auto() or have_system
>     endif
>   endif
>   
> +# Remove this error after QEMU 8.1 has been released.
> +if not get_option('slirp').disabled() and not slirp.found()
> +  error('libslirp is not explicitely disabled and was not found. ' +
> +        'Since qemu 7.2, libslirp is no longer included as a submodule ' +

Maybe s/qemu/QEMU/

> +        'fallback, you must install it on your system or --disable-slirp.')
> +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'],

Reviewed-by: Thomas Huth <thuth@redhat.com>