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

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

[marcandre.lureau]

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 | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/meson.build b/meson.build
index 8dc661363f..565096001d 100644
--- a/meson.build
+++ b/meson.build
@@ -657,6 +657,12 @@ if not get_option('slirp').auto() or have_system
   endif
 endif
 
+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-libslirp.')
+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] build-sys: error when slirp is not found and not disabled

[Daniel P. Berrangé]

On Mon, Oct 03, 2022 at 11:05:34AM +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 | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 8dc661363f..565096001d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -657,6 +657,12 @@ if not get_option('slirp').auto() or have_system
>    endif
>  endif
>  
> +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-libslirp.')
> +endif

I understand the motivation, but this goes against the main principal
of our build time detection, which is to "do the right thing" automatically.
If libslirp is not present on the host, then I would consider the need to
pass --disable-libslirp to be a bug.

I think this pain that people see of loosing slirp support is going to be
a pretty short term problem. IMHO it suffices to print a warning message
right at the very end of configure, after everything else, just for one
or two releases.


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] build-sys: error when slirp is not found and not disabled

[Christian Schoenebeck]

On Montag, 3. Oktober 2022 10:05:14 CEST Daniel P. Berrangé wrote:
> On Mon, Oct 03, 2022 at 11:05:34AM +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 | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/meson.build b/meson.build
> > index 8dc661363f..565096001d 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -657,6 +657,12 @@ if not get_option('slirp').auto() or have_system
> > 
> >    endif
> >  
> >  endif
> > 
> > +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-libslirp.') +endif
> 
> I understand the motivation, but this goes against the main principal
> of our build time detection, which is to "do the right thing" automatically.
> If libslirp is not present on the host, then I would consider the need to
> pass --disable-libslirp to be a bug.
> 
> I think this pain that people see of loosing slirp support is going to be
> a pretty short term problem. IMHO it suffices to print a warning message
> right at the very end of configure, after everything else, just for one
> or two releases.

I guess that many users would find it a bug as well if this core feature just 
stops working. Even if you add a warning; what will probably happen is that 
packages are first built without. You want to risk that backlash, users filing 
reports, etc?

What about using this error for couple releases and then restoring symmetry?

Independent of this particular patch here, there is probably still something 
wrong with slirp detection.

- in the first run it detected correctly that slirp was not installed
- then I installed slirp and it detected correctly that it was installed
- then I uninstalled slirp-dev and slirp and build system still said:

slirp support                : YES 4.4.0

... causing ...

../net/slirp.c:41:10: fatal error: libslirp.h: No such file or directory
   41 | #include <libslirp.h>
      |          ^~~~~~~~~~~~

Best regards,
Christian Schoenebeck

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

[Christian Schoenebeck]

On Montag, 3. Oktober 2022 12:48:35 CEST Christian Schoenebeck wrote:
> On Montag, 3. Oktober 2022 10:05:14 CEST Daniel P. Berrangé wrote:
> > On Mon, Oct 03, 2022 at 11:05:34AM +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 | 6 ++++++
> > >  1 file changed, 6 insertions(+)
> > > 
> > > diff --git a/meson.build b/meson.build
> > > index 8dc661363f..565096001d 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -657,6 +657,12 @@ if not get_option('slirp').auto() or have_system
> > > 
> > >    endif
> > >  
> > >  endif
> > > 
> > > +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-libslirp.') +endif
> > 
> > I understand the motivation, but this goes against the main principal
> > of our build time detection, which is to "do the right thing"
> > automatically. If libslirp is not present on the host, then I would
> > consider the need to pass --disable-libslirp to be a bug.
> > 
> > I think this pain that people see of loosing slirp support is going to be
> > a pretty short term problem. IMHO it suffices to print a warning message
> > right at the very end of configure, after everything else, just for one
> > or two releases.
> 
> I guess that many users would find it a bug as well if this core feature
> just stops working. Even if you add a warning; what will probably happen is
> that packages are first built without. You want to risk that backlash,
> users filing reports, etc?
> 
> What about using this error for couple releases and then restoring symmetry?
> 
> Independent of this particular patch here, there is probably still something
> wrong with slirp detection.
> 
> - in the first run it detected correctly that slirp was not installed
> - then I installed slirp and it detected correctly that it was installed
> - then I uninstalled slirp-dev and slirp and build system still said:
> 
> slirp support                : YES 4.4.0
> 
> ... causing ...
> 
> ../net/slirp.c:41:10: fatal error: libslirp.h: No such file or directory
>    41 | #include <libslirp.h>

Oh, it's because Meson caches this build dependency result. From
build/meson-logs/meson-log.txt:

Dependency slirp found: YES 4.4.0 (cached)

https://github.com/mesonbuild/meson/issues/2643

:/

Best regards,
Christian Schoenebeck

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

[Thomas Huth]

On 03/10/2022 12.48, Christian Schoenebeck wrote:
> On Montag, 3. Oktober 2022 10:05:14 CEST Daniel P. Berrangé wrote:
>> On Mon, Oct 03, 2022 at 11:05:34AM +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 | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/meson.build b/meson.build
>>> index 8dc661363f..565096001d 100644
>>> --- a/meson.build
>>> +++ b/meson.build
>>> @@ -657,6 +657,12 @@ if not get_option('slirp').auto() or have_system
>>>
>>>     endif
>>>   
>>>   endif
>>>
>>> +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-libslirp.') +endif

s/--disable-libslirp/--disable-slirp/

>> I understand the motivation, but this goes against the main principal
>> of our build time detection, which is to "do the right thing" automatically.
>> If libslirp is not present on the host, then I would consider the need to
>> pass --disable-libslirp to be a bug.
>>
>> I think this pain that people see of loosing slirp support is going to be
>> a pretty short term problem. IMHO it suffices to print a warning message
>> right at the very end of configure, after everything else, just for one
>> or two releases.
> 
> I guess that many users would find it a bug as well if this core feature just
> stops working. Even if you add a warning; what will probably happen is that
> packages are first built without. You want to risk that backlash, users filing
> reports, etc?
> 
> What about using this error for couple releases and then restoring symmetry?

I think that's a good compromise. Marc-André, could you maybe add a comment 
in front of your new code, saying something like "This is a temporary 
solution, remove after QEMU 8.1 has been released"?

  Thomas