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

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|