[PATCH 0/4] firmware/shim: fix Xen tree setup

[PATCH 0/4] firmware/shim: fix Xen tree setup

[Jan Beulich]

This is a split of the RFC with the mkdir fix added, but with the symlink
handling left off for now (I'll need some more time to properly deal with
that without using -xtype, ideally treating absolute and relative ones
differently).

1: correctly handle errors during Xen tree setup
2: better filtering of dependency files during Xen tree setup
3: better filtering of intermediate files during Xen tree setup
4: avoid mkdir error during Xen tree setup

Signed-off-by: Jan Beulich <jbeulich@suse.com>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/4] firmware/shim: correctly handle errors during Xen tree setup

[Jan Beulich]

"set -e" on a separate Makefile line is meaningless. Glue together all
the lines that this is supposed to cover.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/firmware/xen-dir/Makefile
+++ b/tools/firmware/xen-dir/Makefile
@@ -16,18 +16,18 @@ DEP_FILES=$(foreach i, $(LINK_FILES), $(
 
 linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES) FORCE
 	mkdir -p $(D)
-	set -e
 	rm -f linkfarm.stamp.tmp
+	set -e; \
 	$(foreach d, $(LINK_DIRS), \
 		 (mkdir -p $(D)/$(d); \
 		  cd $(D)/$(d); \
 		  find $(XEN_ROOT)/$(d)/ -type d |\
-			sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir -p);)
+			sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir -p);) \
 	$(foreach d, $(LINK_DIRS), \
 		(cd $(XEN_ROOT); \
 		 find $(d) ! -type l -type f \
 		 $(addprefix ! -path , '*.[oda1]' '*.d[12]')) \
-		 >> linkfarm.stamp.tmp ; )
+		 >> linkfarm.stamp.tmp ; ) \
 	$(foreach f, $(LINK_FILES), \
 		echo $(f) >> linkfarm.stamp.tmp ;)
 	cmp -s linkfarm.stamp.tmp linkfarm.stamp && \




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/4] firmware/shim: better filtering of dependency files during Xen tree setup

[Jan Beulich]

I have no idea what *.d1 is supposed to refer to - we only have .*.d
and .*.d2 files (note also the leading dot).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/firmware/xen-dir/Makefile
+++ b/tools/firmware/xen-dir/Makefile
@@ -26,7 +26,7 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES)
 	$(foreach d, $(LINK_DIRS), \
 		(cd $(XEN_ROOT); \
 		 find $(d) ! -type l -type f \
-		 $(addprefix ! -path , '*.[oda1]' '*.d[12]')) \
+		 $(addprefix ! -path , '*.[oa1]' '.*.d' '.*.d2')) \
 		 >> linkfarm.stamp.tmp ; ) \
 	$(foreach f, $(LINK_FILES), \
 		echo $(f) >> linkfarm.stamp.tmp ;)




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/4] firmware/shim: better filtering of intermediate files during Xen tree setup

[Jan Beulich]

I have no idea what *.1 is meant to cover. Instead also exclude
preprocessed and non-source assembly files.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- unstable.orig/tools/firmware/xen-dir/Makefile	2018-02-07 15:30:24.038600788 +0100
+++ unstable/tools/firmware/xen-dir/Makefile	2018-02-07 15:35:18.197717910 +0100
@@ -26,7 +26,7 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES)
 	$(foreach d, $(LINK_DIRS), \
 		(cd $(XEN_ROOT); \
 		 find $(d) ! -type l -type f \
-		 $(addprefix ! -path , '*.[oa1]' '.*.d' '.*.d2')) \
+		 $(addprefix ! -path , '*.[isoa]' '.*.d' '.*.d2')) \
 		 >> linkfarm.stamp.tmp ; ) \
 	$(foreach f, $(LINK_FILES), \
 		echo $(f) >> linkfarm.stamp.tmp ;)




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 4/4] firmware/shim: avoid mkdir error during Xen tree setup

[Jan Beulich]

"mkdir -p" reports a missing operand, as config/ has no subdirs. Oddly
enough this doesn't cause the whole command (and hence the build to
fail), despite the "set -e" now covering the entire set of commands -
perhaps a quirk of the relatively old bash I've seen this with (a few
simple experiments suggest that commands inside () producing a non-
success status would exit the inner shell, but not the outer one).

Add a dummy . argument to the invocation.

Suggested-by: Wei Liu <wei.liu2@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- unstable.orig/tools/firmware/xen-dir/Makefile	2018-02-07 15:35:18.197717910 +0100
+++ unstable/tools/firmware/xen-dir/Makefile	2018-02-07 15:35:51.027213823 +0100
@@ -22,7 +22,7 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES)
 		 (mkdir -p $(D)/$(d); \
 		  cd $(D)/$(d); \
 		  find $(XEN_ROOT)/$(d)/ -type d |\
-			sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir -p);) \
+			sed 's,^$(XEN_ROOT)/$(d)/,,g' | xargs mkdir -p .);) \
 	$(foreach d, $(LINK_DIRS), \
 		(cd $(XEN_ROOT); \
 		 find $(d) ! -type l -type f \




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/4] firmware/shim: better filtering of dependency files during Xen tree setup

[Roger Pau Monné]

On Wed, Feb 07, 2018 at 08:07:40AM -0700, Jan Beulich wrote:
> I have no idea what *.d1 is supposed to refer to - we only have .*.d
> and .*.d2 files (note also the leading dot).
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/tools/firmware/xen-dir/Makefile
> +++ b/tools/firmware/xen-dir/Makefile
> @@ -26,7 +26,7 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES)
>  	$(foreach d, $(LINK_DIRS), \
>  		(cd $(XEN_ROOT); \
>  		 find $(d) ! -type l -type f \
> -		 $(addprefix ! -path , '*.[oda1]' '*.d[12]')) \
> +		 $(addprefix ! -path , '*.[oa1]' '.*.d' '.*.d2')) \

Don't you want -name here instead of -path?

AFAICT using '.*.d' is not going to work with path, because that's the
full path, not the name of the file. This used to work before because
the patterns started with '*'.

Also I cannot find any .a or .1 files, but maybe that's just because
of my build system (I don't build EFI).

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/4] firmware/shim: correctly handle errors during Xen tree setup

[Roger Pau Monné]

On Wed, Feb 07, 2018 at 08:07:01AM -0700, Jan Beulich wrote:
> "set -e" on a separate Makefile line is meaningless. Glue together all
> the lines that this is supposed to cover.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/4] firmware/shim: better filtering of intermediate files during Xen tree setup

[Roger Pau Monné]

On Wed, Feb 07, 2018 at 08:08:15AM -0700, Jan Beulich wrote:
> I have no idea what *.1 is meant to cover. Instead also exclude
> preprocessed and non-source assembly files.

Oh, I guess that answers my question in 2/4 then. I don't seem to have
neither .i or .a files, but certainly .s should be excluded.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/4] firmware/shim: avoid mkdir error during Xen tree setup

[Roger Pau Monné]

On Wed, Feb 07, 2018 at 08:08:47AM -0700, Jan Beulich wrote:
> "mkdir -p" reports a missing operand, as config/ has no subdirs. Oddly
> enough this doesn't cause the whole command (and hence the build to
> fail), despite the "set -e" now covering the entire set of commands -
> perhaps a quirk of the relatively old bash I've seen this with (a few
> simple experiments suggest that commands inside () producing a non-
> success status would exit the inner shell, but not the outer one).
> 
> Add a dummy . argument to the invocation.
> 
> Suggested-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/4] firmware/shim: better filtering of dependency files during Xen tree setup

[Jan Beulich]

>>> On 07.02.18 at 17:05, <roger.pau@citrix.com> wrote:
> On Wed, Feb 07, 2018 at 08:07:40AM -0700, Jan Beulich wrote:
>> I have no idea what *.d1 is supposed to refer to - we only have .*.d
>> and .*.d2 files (note also the leading dot).
>> 
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> 
>> --- a/tools/firmware/xen-dir/Makefile
>> +++ b/tools/firmware/xen-dir/Makefile
>> @@ -26,7 +26,7 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES)
>>  	$(foreach d, $(LINK_DIRS), \
>>  		(cd $(XEN_ROOT); \
>>  		 find $(d) ! -type l -type f \
>> -		 $(addprefix ! -path , '*.[oda1]' '*.d[12]')) \
>> +		 $(addprefix ! -path , '*.[oa1]' '.*.d' '.*.d2')) \
> 
> Don't you want -name here instead of -path?
> 
> AFAICT using '.*.d' is not going to work with path, because that's the
> full path, not the name of the file. This used to work before because
> the patterns started with '*'.

Oh, good point. Once again I have no idea why -path was used in
the first place.

> Also I cannot find any .a or .1 files, but maybe that's just because
> of my build system (I don't build EFI).

The .1 part goes away in patch 3. I suppose .a is just to cover
archives even if we don't use any right now.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/4] firmware/shim: better filtering of intermediate files during Xen tree setup

[Jan Beulich]

>>> On 07.02.18 at 17:12, <roger.pau@citrix.com> wrote:
> On Wed, Feb 07, 2018 at 08:08:15AM -0700, Jan Beulich wrote:
>> I have no idea what *.1 is meant to cover. Instead also exclude
>> preprocessed and non-source assembly files.
> 
> Oh, I guess that answers my question in 2/4 then. I don't seem to have
> neither .i or .a files, but certainly .s should be excluded.

.s and .i can both be generated for the case someone has a need to
look at them (I need to from time to time). As we have rules for both,
we should also exclude what those rules can generate.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/4] firmware/shim: correctly handle errors during Xen tree setup

[Wei Liu]

On Wed, Feb 07, 2018 at 08:07:01AM -0700, Jan Beulich wrote:
> "set -e" on a separate Makefile line is meaningless. Glue together all
> the lines that this is supposed to cover.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/4] firmware/shim: better filtering of dependency files during Xen tree setup

[Wei Liu]

On Wed, Feb 07, 2018 at 09:27:49AM -0700, Jan Beulich wrote:
> >>> On 07.02.18 at 17:05, <roger.pau@citrix.com> wrote:
> > On Wed, Feb 07, 2018 at 08:07:40AM -0700, Jan Beulich wrote:
> >> I have no idea what *.d1 is supposed to refer to - we only have .*.d
> >> and .*.d2 files (note also the leading dot).
> >> 
> >> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> >> 
> >> --- a/tools/firmware/xen-dir/Makefile
> >> +++ b/tools/firmware/xen-dir/Makefile
> >> @@ -26,7 +26,7 @@ linkfarm.stamp: $(DEP_DIRS) $(DEP_FILES)
> >>  	$(foreach d, $(LINK_DIRS), \
> >>  		(cd $(XEN_ROOT); \
> >>  		 find $(d) ! -type l -type f \
> >> -		 $(addprefix ! -path , '*.[oda1]' '*.d[12]')) \
> >> +		 $(addprefix ! -path , '*.[oa1]' '.*.d' '.*.d2')) \
> > 
> > Don't you want -name here instead of -path?
> > 
> > AFAICT using '.*.d' is not going to work with path, because that's the
> > full path, not the name of the file. This used to work before because
> > the patterns started with '*'.
> 
> Oh, good point. Once again I have no idea why -path was used in
> the first place.
> 

'-name' is indeed better here.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/4] firmware/shim: better filtering of intermediate files during Xen tree setup

[Wei Liu]

On Wed, Feb 07, 2018 at 08:08:15AM -0700, Jan Beulich wrote:
> I have no idea what *.1 is meant to cover. Instead also exclude
> preprocessed and non-source assembly files.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/4] firmware/shim: avoid mkdir error during Xen tree setup

[Wei Liu]

On Wed, Feb 07, 2018 at 08:08:47AM -0700, Jan Beulich wrote:
> "mkdir -p" reports a missing operand, as config/ has no subdirs. Oddly
> enough this doesn't cause the whole command (and hence the build to
> fail), despite the "set -e" now covering the entire set of commands -
> perhaps a quirk of the relatively old bash I've seen this with (a few
> simple experiments suggest that commands inside () producing a non-
> success status would exit the inner shell, but not the outer one).
> 
> Add a dummy . argument to the invocation.
> 
> Suggested-by: Wei Liu <wei.liu2@citrix.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel