* [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig"
@ 2021-09-08 11:17 Anthony PERARD
2021-09-16 15:34 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Anthony PERARD @ 2021-09-08 11:17 UTC (permalink / raw)
To: xen-devel
Cc: Anthony PERARD, Andrew Cooper, George Dunlap, Ian Jackson,
Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu
This will help prevent the CI loop from having build failures when
`checkpolicy` isn't available when doing "randconfig" jobs.
To prevent "randconfig" from selecting XSM_FLASK_POLICY when
`checkpolicy` isn't available, we will actually override the config
output with the use of KCONFIG_ALLCONFIG.
Doing this way still allow a user/developer to set XSM_FLASK_POLICY
even when "checkpolicy" isn't available. It also prevent the build
system from reset the config when "checkpolicy" isn't available
anymore. And XSM_FLASK_POLICY is still selected automatically when
`checkpolicy` is available.
But this also work well for "randconfig", as it will not select
XSM_FLASK_POLICY when "checkpolicy" is missing.
This patch allows to easily add more override which depends on the
environment.
Also, move the check out of Config.mk and into xen/ build system.
Nothing in tools/ is using that information as it's done by
./configure.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
---
v3:
- use KCONFIG_ALLCONFIG
- don't override XSM_FLASK_POLICY value unless we do randconfig.
- no more changes to the current behavior of kconfig, only to
randconfig.
v2 was "[XEN PATCH v2] xen: allow XSM_FLASK_POLICY only if checkpolicy binary is available"
---
Config.mk | 6 ------
xen/Makefile | 18 ++++++++++++++++--
xen/common/Kconfig | 2 +-
3 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/Config.mk b/Config.mk
index d0712724f8e4..144411133f38 100644
--- a/Config.mk
+++ b/Config.mk
@@ -137,12 +137,6 @@ export XEN_HAS_BUILD_ID=y
build_id_linker := --build-id=sha1
endif
-ifndef XEN_HAS_CHECKPOLICY
- CHECKPOLICY ?= checkpolicy
- XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && echo y || echo n)
- export XEN_HAS_CHECKPOLICY
-endif
-
define buildmakevars2shellvars
export PREFIX="$(prefix)"; \
export XEN_SCRIPT_DIR="$(XEN_SCRIPT_DIR)"; \
diff --git a/xen/Makefile b/xen/Makefile
index f47423dacd9a..89804aefe385 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -17,6 +17,8 @@ export XEN_BUILD_HOST ?= $(shell hostname)
PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 2>/dev/null) python)
export PYTHON ?= $(PYTHON_INTERPRETER)
+export CHECKPOLICY ?= checkpolicy
+
export BASEDIR := $(CURDIR)
export XEN_ROOT := $(BASEDIR)/..
@@ -178,6 +180,8 @@ CFLAGS += $(CLANG_FLAGS)
export CLANG_FLAGS
endif
+export HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q xen)
+
export root-make-done := y
endif # root-make-done
@@ -189,14 +193,24 @@ ifeq ($(config-build),y)
# *config targets only - make sure prerequisites are updated, and descend
# in tools/kconfig to make the *config target
+# Create a file for KCONFIG_ALLCONFIG which depends on the environment.
+# This will be use by kconfig targets allyesconfig/allmodconfig/allnoconfig/randconfig
+filechk_kconfig_allconfig = \
+ $(if $(findstring n,$(HAS_CHECKPOLICY)),echo 'CONFIG_XSM_FLASK_POLICY=n';) \
+ $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG), :)
+
+
+.allconfig.tmp: FORCE
+ set -e; { $(call filechk_kconfig_allconfig); } > $@
+
config: FORCE
$(MAKE) $(kconfig) $@
# Config.mk tries to include .config file, don't try to remake it
%/.config: ;
-%config: FORCE
- $(MAKE) $(kconfig) $@
+%config: .allconfig.tmp FORCE
+ $(MAKE) $(kconfig) KCONFIG_ALLCONFIG=$< $@
else # !config-build
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 0ddd18e11af3..73d8afb7bcbd 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -235,7 +235,7 @@ config XSM_FLASK_AVC_STATS
config XSM_FLASK_POLICY
bool "Compile Xen with a built-in FLASK security policy"
- default y if "$(XEN_HAS_CHECKPOLICY)" = "y"
+ default y if "$(HAS_CHECKPOLICY)"
depends on XSM_FLASK
---help---
This includes a default XSM policy in the hypervisor so that the
--
Anthony PERARD
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig"
2021-09-08 11:17 [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig" Anthony PERARD
@ 2021-09-16 15:34 ` Jan Beulich
2021-09-27 9:46 ` Anthony PERARD
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2021-09-16 15:34 UTC (permalink / raw)
To: Anthony PERARD
Cc: Andrew Cooper, George Dunlap, Ian Jackson, Julien Grall,
Stefano Stabellini, Wei Liu, xen-devel
On 08.09.2021 13:17, Anthony PERARD wrote:
> --- a/Config.mk
> +++ b/Config.mk
> @@ -137,12 +137,6 @@ export XEN_HAS_BUILD_ID=y
> build_id_linker := --build-id=sha1
> endif
>
> -ifndef XEN_HAS_CHECKPOLICY
> - CHECKPOLICY ?= checkpolicy
> - XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && echo y || echo n)
> - export XEN_HAS_CHECKPOLICY
> -endif
Is there a particular reason to go from XEN_HAS_CHECKPOLICY to ...
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -17,6 +17,8 @@ export XEN_BUILD_HOST ?= $(shell hostname)
> PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 2>/dev/null) python)
> export PYTHON ?= $(PYTHON_INTERPRETER)
>
> +export CHECKPOLICY ?= checkpolicy
> +
> export BASEDIR := $(CURDIR)
> export XEN_ROOT := $(BASEDIR)/..
>
> @@ -178,6 +180,8 @@ CFLAGS += $(CLANG_FLAGS)
> export CLANG_FLAGS
> endif
>
> +export HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q xen)
... HAS_CHECKPOLICY? As soon as things get put in the environment,
I'm always suspecting possible name collisions ...
> @@ -189,14 +193,24 @@ ifeq ($(config-build),y)
> # *config targets only - make sure prerequisites are updated, and descend
> # in tools/kconfig to make the *config target
>
> +# Create a file for KCONFIG_ALLCONFIG which depends on the environment.
> +# This will be use by kconfig targets allyesconfig/allmodconfig/allnoconfig/randconfig
> +filechk_kconfig_allconfig = \
> + $(if $(findstring n,$(HAS_CHECKPOLICY)),echo 'CONFIG_XSM_FLASK_POLICY=n';) \
> + $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG), :)
Nit: It would be nice if you were consistent with the blanks after
commas in $(if ...). Personally I'm also considering $(if ...)s the
more difficult to follow the longer they are. Hence for the 2nd one
I wonder whether
$(if $(KCONFIG_ALLCONFIG),cat,:) $(KCONFIG_ALLCONFIG)
wouldn't be easier to read.
> +
> +
Nit: Please avoid double blank lines.
> +.allconfig.tmp: FORCE
> + set -e; { $(call filechk_kconfig_allconfig); } > $@
Is there a particular reason for the .tmp suffix?
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig"
2021-09-16 15:34 ` Jan Beulich
@ 2021-09-27 9:46 ` Anthony PERARD
2021-09-27 10:03 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Anthony PERARD @ 2021-09-27 9:46 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, George Dunlap, Ian Jackson, Julien Grall,
Stefano Stabellini, Wei Liu, xen-devel
On Thu, Sep 16, 2021 at 05:34:00PM +0200, Jan Beulich wrote:
> On 08.09.2021 13:17, Anthony PERARD wrote:
> > --- a/Config.mk
> > +++ b/Config.mk
> > @@ -137,12 +137,6 @@ export XEN_HAS_BUILD_ID=y
> > build_id_linker := --build-id=sha1
> > endif
> >
> > -ifndef XEN_HAS_CHECKPOLICY
> > - CHECKPOLICY ?= checkpolicy
> > - XEN_HAS_CHECKPOLICY := $(shell $(CHECKPOLICY) -h 2>&1 | grep -q xen && echo y || echo n)
> > - export XEN_HAS_CHECKPOLICY
> > -endif
>
> Is there a particular reason to go from XEN_HAS_CHECKPOLICY to ...
>
> > --- a/xen/Makefile
> > +++ b/xen/Makefile
> > @@ -17,6 +17,8 @@ export XEN_BUILD_HOST ?= $(shell hostname)
> > PYTHON_INTERPRETER := $(word 1,$(shell which python3 python python2 2>/dev/null) python)
> > export PYTHON ?= $(PYTHON_INTERPRETER)
> >
> > +export CHECKPOLICY ?= checkpolicy
> > +
> > export BASEDIR := $(CURDIR)
> > export XEN_ROOT := $(BASEDIR)/..
> >
> > @@ -178,6 +180,8 @@ CFLAGS += $(CLANG_FLAGS)
> > export CLANG_FLAGS
> > endif
> >
> > +export HAS_CHECKPOLICY := $(call success,$(CHECKPOLICY) -h 2>&1 | grep -q xen)
>
> ... HAS_CHECKPOLICY? As soon as things get put in the environment,
Not really anymore, it's just left over from having put this in Kconfig
in previous version of the patch.
> I'm always suspecting possible name collisions ...
Yes, it's probably better to keep the XEN_ prefix.
> > @@ -189,14 +193,24 @@ ifeq ($(config-build),y)
> > # *config targets only - make sure prerequisites are updated, and descend
> > # in tools/kconfig to make the *config target
> >
> > +# Create a file for KCONFIG_ALLCONFIG which depends on the environment.
> > +# This will be use by kconfig targets allyesconfig/allmodconfig/allnoconfig/randconfig
> > +filechk_kconfig_allconfig = \
> > + $(if $(findstring n,$(HAS_CHECKPOLICY)),echo 'CONFIG_XSM_FLASK_POLICY=n';) \
> > + $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG), :)
>
> Nit: It would be nice if you were consistent with the blanks after
> commas in $(if ...). Personally I'm also considering $(if ...)s the
> more difficult to follow the longer they are. Hence for the 2nd one
> I wonder whether
>
> $(if $(KCONFIG_ALLCONFIG),cat,:) $(KCONFIG_ALLCONFIG)
>
> wouldn't be easier to read.
How about:
$(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
:
.. instead, as that would be more consistent with the previous line,
that is there would be only one branch to the $(if ) and no else, and
thus probably easier to read.
> > +.allconfig.tmp: FORCE
> > + set -e; { $(call filechk_kconfig_allconfig); } > $@
>
> Is there a particular reason for the .tmp suffix?
Yes, .*.tmp are already ignored via .gitignore.
Thanks,
--
Anthony PERARD
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig"
2021-09-27 9:46 ` Anthony PERARD
@ 2021-09-27 10:03 ` Jan Beulich
0 siblings, 0 replies; 4+ messages in thread
From: Jan Beulich @ 2021-09-27 10:03 UTC (permalink / raw)
To: Anthony PERARD
Cc: Andrew Cooper, George Dunlap, Ian Jackson, Julien Grall,
Stefano Stabellini, Wei Liu, xen-devel
On 27.09.2021 11:46, Anthony PERARD wrote:
> On Thu, Sep 16, 2021 at 05:34:00PM +0200, Jan Beulich wrote:
>> On 08.09.2021 13:17, Anthony PERARD wrote:
>>> @@ -189,14 +193,24 @@ ifeq ($(config-build),y)
>>> # *config targets only - make sure prerequisites are updated, and descend
>>> # in tools/kconfig to make the *config target
>>>
>>> +# Create a file for KCONFIG_ALLCONFIG which depends on the environment.
>>> +# This will be use by kconfig targets allyesconfig/allmodconfig/allnoconfig/randconfig
>>> +filechk_kconfig_allconfig = \
>>> + $(if $(findstring n,$(HAS_CHECKPOLICY)),echo 'CONFIG_XSM_FLASK_POLICY=n';) \
>>> + $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG), :)
>>
>> Nit: It would be nice if you were consistent with the blanks after
>> commas in $(if ...). Personally I'm also considering $(if ...)s the
>> more difficult to follow the longer they are. Hence for the 2nd one
>> I wonder whether
>>
>> $(if $(KCONFIG_ALLCONFIG),cat,:) $(KCONFIG_ALLCONFIG)
>>
>> wouldn't be easier to read.
>
> How about:
>
> $(if $(KCONFIG_ALLCONFIG), cat $(KCONFIG_ALLCONFIG);) \
> :
>
> .. instead, as that would be more consistent with the previous line,
> that is there would be only one branch to the $(if ) and no else, and
> thus probably easier to read.
Oh, sure, even better if that works.
>>> +.allconfig.tmp: FORCE
>>> + set -e; { $(call filechk_kconfig_allconfig); } > $@
>>
>> Is there a particular reason for the .tmp suffix?
>
> Yes, .*.tmp are already ignored via .gitignore.
I see. Could you add two words to the description saying so? Or
maybe even just a post-commit-message remark would do.
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-27 10:04 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 11:17 [XEN PATCH v3] xen: rework `checkpolicy` detection when using "randconfig" Anthony PERARD
2021-09-16 15:34 ` Jan Beulich
2021-09-27 9:46 ` Anthony PERARD
2021-09-27 10:03 ` Jan Beulich
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).