* [PATCH 0/4] Fixes for the --without-default-features configure switch
@ 2021-07-13 9:31 Thomas Huth
2021-07-13 9:31 ` [PATCH 1/4] configure: Fix --without-default-features propagation to meson Thomas Huth
` (5 more replies)
0 siblings, 6 replies; 12+ messages in thread
From: Thomas Huth @ 2021-07-13 9:31 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Cole Robinson
Many features do not get properly disabled when the user runs the
configure script with --without-default-features. Let's fix that now.
Thomas Huth (4):
configure: Fix --without-default-features propagation to meson
configure: Allow vnc to get disabled with --without-default-features
configure: Fix the default setting of the "xen" feature
configure: Let --without-default-features disable vhost-kernel and
vhost-vdpa
configure | 8 +++++---
meson.build | 2 +-
2 files changed, 6 insertions(+), 4 deletions(-)
--
2.27.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 1/4] configure: Fix --without-default-features propagation to meson
2021-07-13 9:31 [PATCH 0/4] Fixes for the --without-default-features configure switch Thomas Huth
@ 2021-07-13 9:31 ` Thomas Huth
2021-07-13 16:42 ` Philippe Mathieu-Daudé
2021-07-13 9:31 ` [PATCH 2/4] configure: Allow vnc to get disabled with --without-default-features Thomas Huth
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Thomas Huth @ 2021-07-13 9:31 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Cole Robinson
A typo prevents that many features get disabled when the user
runs "configure" with the --without-default-features switch.
Reported-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 85db248ac1..229ea52516 100755
--- a/configure
+++ b/configure
@@ -5205,7 +5205,7 @@ if test "$skip_meson" = no; then
-Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
-Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
-Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
- $(if test "$default_features" = no; then echo "-Dauto_features=disabled"; fi) \
+ $(if test "$default_feature" = no; then echo "-Dauto_features=disabled"; fi) \
-Dtcg_interpreter=$tcg_interpreter \
$cross_arg \
"$PWD" "$source_path"
--
2.27.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 2/4] configure: Allow vnc to get disabled with --without-default-features
2021-07-13 9:31 [PATCH 0/4] Fixes for the --without-default-features configure switch Thomas Huth
2021-07-13 9:31 ` [PATCH 1/4] configure: Fix --without-default-features propagation to meson Thomas Huth
@ 2021-07-13 9:31 ` Thomas Huth
2021-07-13 9:31 ` [PATCH 3/4] configure: Fix the default setting of the "xen" feature Thomas Huth
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2021-07-13 9:31 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Cole Robinson
There's no reason why we should keep VNC enabled when the user
specified --without-default-features.
Reported-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 2 +-
meson.build | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/configure b/configure
index 229ea52516..1974c46f6e 100755
--- a/configure
+++ b/configure
@@ -304,7 +304,7 @@ virtiofsd="auto"
virtfs="auto"
libudev="auto"
mpath="auto"
-vnc="enabled"
+vnc="auto"
sparse="auto"
vde="$default_feature"
vnc_sasl="auto"
diff --git a/meson.build b/meson.build
index 6cd2dc582a..5dedaf73e3 100644
--- a/meson.build
+++ b/meson.build
@@ -900,7 +900,7 @@ vnc = not_found
png = not_found
jpeg = not_found
sasl = not_found
-if get_option('vnc').enabled()
+if not get_option('vnc').disabled()
vnc = declare_dependency() # dummy dependency
png = dependency('libpng', required: get_option('vnc_png'),
method: 'pkg-config', kwargs: static_kwargs)
--
2.27.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 3/4] configure: Fix the default setting of the "xen" feature
2021-07-13 9:31 [PATCH 0/4] Fixes for the --without-default-features configure switch Thomas Huth
2021-07-13 9:31 ` [PATCH 1/4] configure: Fix --without-default-features propagation to meson Thomas Huth
2021-07-13 9:31 ` [PATCH 2/4] configure: Allow vnc to get disabled with --without-default-features Thomas Huth
@ 2021-07-13 9:31 ` Thomas Huth
2021-07-13 9:31 ` [PATCH 4/4] configure: Let --without-default-features disable vhost-kernel and vhost-vdpa Thomas Huth
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2021-07-13 9:31 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Cole Robinson
The "xen" variable should either contain "enabled", "disabled" or
nothing (for auto detection). But when the user currently runs the
configure script with --without-default-features, it gets set to
"no" instead. This does not work as expected, the feature will still
be enabled if the Xen headers are present. Thus set the variable
to "disabled" instead if default_feature switch has been set.
Reported-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/configure b/configure
index 1974c46f6e..6c7336b763 100755
--- a/configure
+++ b/configure
@@ -311,7 +311,7 @@ vnc_sasl="auto"
vnc_jpeg="auto"
vnc_png="auto"
xkbcommon="auto"
-xen="$default_feature"
+xen=${default_feature:+disabled}
xen_ctrl_version="$default_feature"
xen_pci_passthrough="auto"
linux_aio="$default_feature"
--
2.27.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH 4/4] configure: Let --without-default-features disable vhost-kernel and vhost-vdpa
2021-07-13 9:31 [PATCH 0/4] Fixes for the --without-default-features configure switch Thomas Huth
` (2 preceding siblings ...)
2021-07-13 9:31 ` [PATCH 3/4] configure: Fix the default setting of the "xen" feature Thomas Huth
@ 2021-07-13 9:31 ` Thomas Huth
2021-07-13 14:54 ` [PATCH 0/4] Fixes for the --without-default-features configure switch Cole Robinson
2021-07-14 15:06 ` Paolo Bonzini
5 siblings, 0 replies; 12+ messages in thread
From: Thomas Huth @ 2021-07-13 9:31 UTC (permalink / raw)
To: qemu-devel, Paolo Bonzini; +Cc: Cole Robinson
The vhost_kernel and vhost_vdpa variables should be pre-initialized with
the $default_feature setting so that these features get disabled when
the user runs the configure scripts with --without-default-features.
Reported-by: Cole Robinson <crobinso@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
configure | 2 ++
1 file changed, 2 insertions(+)
diff --git a/configure b/configure
index 6c7336b763..89cc9b24eb 100755
--- a/configure
+++ b/configure
@@ -321,6 +321,7 @@ attr="auto"
xfs="$default_feature"
tcg="enabled"
membarrier="$default_feature"
+vhost_kernel="$default_feature"
vhost_net="$default_feature"
vhost_crypto="$default_feature"
vhost_scsi="$default_feature"
@@ -328,6 +329,7 @@ vhost_vsock="$default_feature"
vhost_user="no"
vhost_user_blk_server="auto"
vhost_user_fs="$default_feature"
+vhost_vdpa="$default_feature"
bpf="auto"
kvm="auto"
hax="auto"
--
2.27.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Fixes for the --without-default-features configure switch
2021-07-13 9:31 [PATCH 0/4] Fixes for the --without-default-features configure switch Thomas Huth
` (3 preceding siblings ...)
2021-07-13 9:31 ` [PATCH 4/4] configure: Let --without-default-features disable vhost-kernel and vhost-vdpa Thomas Huth
@ 2021-07-13 14:54 ` Cole Robinson
2021-07-14 15:06 ` Paolo Bonzini
2021-07-14 15:06 ` Paolo Bonzini
5 siblings, 1 reply; 12+ messages in thread
From: Cole Robinson @ 2021-07-13 14:54 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Paolo Bonzini
On 7/13/21 5:31 AM, Thomas Huth wrote:
> Many features do not get properly disabled when the user runs the
> configure script with --without-default-features. Let's fix that now.
>
> Thomas Huth (4):
> configure: Fix --without-default-features propagation to meson
> configure: Allow vnc to get disabled with --without-default-features
> configure: Fix the default setting of the "xen" feature
> configure: Let --without-default-features disable vhost-kernel and
> vhost-vdpa
>
Patches look fine and fix some issues but others persist
(--disable-system isn't triggered). IMO this needs an audit, but more
importantly 'configure' should be rearranged a bit to make this less
likely to regress:
* move all the --enable/--disable variable init into one section with
nothing else mixed in
* convert the values to all use
$default_yes/no/auto/enabled/disabled/... variable syntax so visually
it's consistent, and if a default is ever changed like $default_no ->
$default_yes then we behave correctly (as opposed to 'no' -> 'yes').
Thanks,
Cole
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] configure: Fix --without-default-features propagation to meson
2021-07-13 9:31 ` [PATCH 1/4] configure: Fix --without-default-features propagation to meson Thomas Huth
@ 2021-07-13 16:42 ` Philippe Mathieu-Daudé
2021-07-14 5:43 ` Markus Armbruster
2021-07-14 7:35 ` Thomas Huth
0 siblings, 2 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-13 16:42 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Paolo Bonzini; +Cc: Alex Bennée, Cole Robinson
+Alex
On 7/13/21 11:31 AM, Thomas Huth wrote:
> A typo prevents that many features get disabled when the user
> runs "configure" with the --without-default-features switch.
>
> Reported-by: Cole Robinson <crobinso@redhat.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> configure | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 85db248ac1..229ea52516 100755
> --- a/configure
> +++ b/configure
> @@ -5205,7 +5205,7 @@ if test "$skip_meson" = no; then
> -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
> -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
> -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
> - $(if test "$default_features" = no; then echo "-Dauto_features=disabled"; fi) \
> + $(if test "$default_feature" = no; then echo "-Dauto_features=disabled"; fi) \
The option should be name plural (default_features)...
What is 'auto_features' used for?
> -Dtcg_interpreter=$tcg_interpreter \
> $cross_arg \
> "$PWD" "$source_path"
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] configure: Fix --without-default-features propagation to meson
2021-07-13 16:42 ` Philippe Mathieu-Daudé
@ 2021-07-14 5:43 ` Markus Armbruster
2021-07-14 7:35 ` Thomas Huth
1 sibling, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2021-07-14 5:43 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Paolo Bonzini, Thomas Huth, Alex Bennée, qemu-devel, Cole Robinson
Philippe Mathieu-Daudé <philmd@redhat.com> writes:
> +Alex
>
> On 7/13/21 11:31 AM, Thomas Huth wrote:
>> A typo prevents that many features get disabled when the user
>> runs "configure" with the --without-default-features switch.
>>
>> Reported-by: Cole Robinson <crobinso@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> configure | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 85db248ac1..229ea52516 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5205,7 +5205,7 @@ if test "$skip_meson" = no; then
>> -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
>> -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
>> -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
>> - $(if test "$default_features" = no; then echo "-Dauto_features=disabled"; fi) \
>> + $(if test "$default_feature" = no; then echo "-Dauto_features=disabled"; fi) \
>
> The option should be name plural (default_features)...
Actually, no. The variable holds the initial value for the feature
variables that are to be controlled by --with-default-features.
Example:
vde="$default_feature"
Perhaps $feature_default would be a better name.
> What is 'auto_features' used for?
>
>> -Dtcg_interpreter=$tcg_interpreter \
>> $cross_arg \
>> "$PWD" "$source_path"
>>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] configure: Fix --without-default-features propagation to meson
2021-07-13 16:42 ` Philippe Mathieu-Daudé
2021-07-14 5:43 ` Markus Armbruster
@ 2021-07-14 7:35 ` Thomas Huth
2021-07-14 9:14 ` Philippe Mathieu-Daudé
1 sibling, 1 reply; 12+ messages in thread
From: Thomas Huth @ 2021-07-14 7:35 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini
Cc: Alex Bennée, Cole Robinson
On 13/07/2021 18.42, Philippe Mathieu-Daudé wrote:
> +Alex
>
> On 7/13/21 11:31 AM, Thomas Huth wrote:
>> A typo prevents that many features get disabled when the user
>> runs "configure" with the --without-default-features switch.
>>
>> Reported-by: Cole Robinson <crobinso@redhat.com>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>> configure | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 85db248ac1..229ea52516 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5205,7 +5205,7 @@ if test "$skip_meson" = no; then
>> -Ddocs=$docs -Dsphinx_build=$sphinx_build -Dinstall_blobs=$blobs \
>> -Dvhost_user_blk_server=$vhost_user_blk_server -Dmultiprocess=$multiprocess \
>> -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
>> - $(if test "$default_features" = no; then echo "-Dauto_features=disabled"; fi) \
>> + $(if test "$default_feature" = no; then echo "-Dauto_features=disabled"; fi) \
>
> The option should be name plural (default_features)...
I agree with Markus - the way it is used in the configure script, it's
rather meant as singular.
> What is 'auto_features' used for?
https://mesonbuild.com/Build-options.html#features
HTH,
Thomas
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 1/4] configure: Fix --without-default-features propagation to meson
2021-07-14 7:35 ` Thomas Huth
@ 2021-07-14 9:14 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2021-07-14 9:14 UTC (permalink / raw)
To: Thomas Huth, qemu-devel, Paolo Bonzini; +Cc: Alex Bennée, Cole Robinson
On 7/14/21 9:35 AM, Thomas Huth wrote:
> On 13/07/2021 18.42, Philippe Mathieu-Daudé wrote:
>> +Alex
>>
>> On 7/13/21 11:31 AM, Thomas Huth wrote:
>>> A typo prevents that many features get disabled when the user
>>> runs "configure" with the --without-default-features switch.
>>>
>>> Reported-by: Cole Robinson <crobinso@redhat.com>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>> configure | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index 85db248ac1..229ea52516 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -5205,7 +5205,7 @@ if test "$skip_meson" = no; then
>>> -Ddocs=$docs -Dsphinx_build=$sphinx_build
>>> -Dinstall_blobs=$blobs \
>>> -Dvhost_user_blk_server=$vhost_user_blk_server
>>> -Dmultiprocess=$multiprocess \
>>> -Dfuse=$fuse -Dfuse_lseek=$fuse_lseek
>>> -Dguest_agent_msi=$guest_agent_msi -Dbpf=$bpf\
>>> - $(if test "$default_features" = no; then echo
>>> "-Dauto_features=disabled"; fi) \
>>> + $(if test "$default_feature" = no; then echo
>>> "-Dauto_features=disabled"; fi) \
>>
>> The option should be name plural (default_features)...
>
> I agree with Markus - the way it is used in the configure script, it's
> rather meant as singular.
OK.
>> What is 'auto_features' used for?
>
> https://mesonbuild.com/Build-options.html#features
I see, thank you.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Fixes for the --without-default-features configure switch
2021-07-13 14:54 ` [PATCH 0/4] Fixes for the --without-default-features configure switch Cole Robinson
@ 2021-07-14 15:06 ` Paolo Bonzini
0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2021-07-14 15:06 UTC (permalink / raw)
To: Cole Robinson, Thomas Huth, qemu-devel
On 13/07/21 16:54, Cole Robinson wrote:
> Patches look fine and fix some issues but others persist
> (--disable-system isn't triggered).
I wouldn't say --disable-system counts as a feature, since it's really a
shortcut for choosing a subset of the targets. Likewise for linux_user
and bsd_user.
> IMO this needs an audit, but more
> importantly 'configure' should be rearranged a bit to make this less
> likely to regress:
>
> * move all the --enable/--disable variable init into one section with
> nothing else mixed in
>
> * convert the values to all use
> $default_yes/no/auto/enabled/disabled/... variable syntax so visually
> it's consistent, and if a default is ever changed like $default_no ->
> $default_yes then we behave correctly (as opposed to 'no' -> 'yes').
This is a nice idea. We should only have default_yes/no/auto, plus
"auto" for Meson options.
Also there's the idea of parsing --enable/--disable options for Meson
options automatically from the introspection data. This has the
advantage that you get the default automatically from meson_options.txt
and -Dauto_features, without any code in configure.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 0/4] Fixes for the --without-default-features configure switch
2021-07-13 9:31 [PATCH 0/4] Fixes for the --without-default-features configure switch Thomas Huth
` (4 preceding siblings ...)
2021-07-13 14:54 ` [PATCH 0/4] Fixes for the --without-default-features configure switch Cole Robinson
@ 2021-07-14 15:06 ` Paolo Bonzini
5 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2021-07-14 15:06 UTC (permalink / raw)
To: Thomas Huth, qemu-devel; +Cc: Cole Robinson
On 13/07/21 11:31, Thomas Huth wrote:
> Many features do not get properly disabled when the user runs the
> configure script with --without-default-features. Let's fix that now.
>
> Thomas Huth (4):
> configure: Fix --without-default-features propagation to meson
> configure: Allow vnc to get disabled with --without-default-features
> configure: Fix the default setting of the "xen" feature
> configure: Let --without-default-features disable vhost-kernel and
> vhost-vdpa
>
> configure | 8 +++++---
> meson.build | 2 +-
> 2 files changed, 6 insertions(+), 4 deletions(-)
>
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-07-14 15:23 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 9:31 [PATCH 0/4] Fixes for the --without-default-features configure switch Thomas Huth
2021-07-13 9:31 ` [PATCH 1/4] configure: Fix --without-default-features propagation to meson Thomas Huth
2021-07-13 16:42 ` Philippe Mathieu-Daudé
2021-07-14 5:43 ` Markus Armbruster
2021-07-14 7:35 ` Thomas Huth
2021-07-14 9:14 ` Philippe Mathieu-Daudé
2021-07-13 9:31 ` [PATCH 2/4] configure: Allow vnc to get disabled with --without-default-features Thomas Huth
2021-07-13 9:31 ` [PATCH 3/4] configure: Fix the default setting of the "xen" feature Thomas Huth
2021-07-13 9:31 ` [PATCH 4/4] configure: Let --without-default-features disable vhost-kernel and vhost-vdpa Thomas Huth
2021-07-13 14:54 ` [PATCH 0/4] Fixes for the --without-default-features configure switch Cole Robinson
2021-07-14 15:06 ` Paolo Bonzini
2021-07-14 15:06 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.