| Message ID | 20170216185651.30110-2-wei.liu2@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 16/02/17 18:56, Wei Liu wrote: > Several `ln -sf` can race with each other and cause error like: > > 14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory > > Provide dedicated targets for soft-linking directories. > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > --- > tools/fuzz/x86_instruction_emulator/Makefile | 8 ++++++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile > index fede7e9afd..0cd49753cf 100644 > --- a/tools/fuzz/x86_instruction_emulator/Makefile > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > @@ -8,12 +8,16 @@ else > x86-instruction-emulator-fuzzer-all: > endif > > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > +x86_emulate: > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate You should be able to do this: x86_emulate/%: x86_emulate > + > +asm: > [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm > > +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm And this: asm/%: asm Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > + > x86_emulate.c x86_emulate.h: %: > [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* >
>>> On 16.02.17 at 19:56, <wei.liu2@citrix.com> wrote: > --- a/tools/fuzz/x86_instruction_emulator/Makefile > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > @@ -8,12 +8,16 @@ else > x86-instruction-emulator-fuzzer-all: > endif > > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > +x86_emulate: > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . In addition to what Andrew has asked for, I would then also appreciate if you used $@ here and ... > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate > + > +asm: > [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm ... here along the lines of ... > +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm > + > x86_emulate.c x86_emulate.h: %: > [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* the use of $* here. Looking at this last one makes me notice, btw, that the second argument to ln is missing? Is it portable to assume that it'll use . in that case? Jan
On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote: > On 16/02/17 18:56, Wei Liu wrote: > > Several `ln -sf` can race with each other and cause error like: > > > > 14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory > > > > Provide dedicated targets for soft-linking directories. > > > > Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Signed-off-by: Wei Liu <wei.liu2@citrix.com> > > --- > > tools/fuzz/x86_instruction_emulator/Makefile | 8 ++++++-- > > 1 file changed, 6 insertions(+), 2 deletions(-) > > > > diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile > > index fede7e9afd..0cd49753cf 100644 > > --- a/tools/fuzz/x86_instruction_emulator/Makefile > > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > > @@ -8,12 +8,16 @@ else > > x86-instruction-emulator-fuzzer-all: > > endif > > > > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > > +x86_emulate: > > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > > > > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: > > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate > > You should be able to do this: > > x86_emulate/%: x86_emulate > Using % won't work. Jan made a similar comment during the first iteration of the fuzzer series. It turned out that we needed to list explicitly every file because of the other rules explicitly listed those files. > > + > > +asm: > > [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm > > > > +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm > > And this: > > asm/%: asm > This can be made work by eliminating the explicit listing of individual files because we're linking asm-x86 wholesale. Wei. > Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > > + > > x86_emulate.c x86_emulate.h: %: > > [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* > > >
On Fri, Feb 17, 2017 at 02:24:15AM -0700, Jan Beulich wrote: > >>> On 16.02.17 at 19:56, <wei.liu2@citrix.com> wrote: > > --- a/tools/fuzz/x86_instruction_emulator/Makefile > > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > > @@ -8,12 +8,16 @@ else > > x86-instruction-emulator-fuzzer-all: > > endif > > > > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > > +x86_emulate: > > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > > In addition to what Andrew has asked for, I would then also appreciate > if you used $@ here and ... > Using $@ is OK. > > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: > > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate > > + > > +asm: > > [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm > > ... here along the lines of ... > > > +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm > > + > > x86_emulate.c x86_emulate.h: %: > > [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$* > > the use of $* here. Looking at this last one makes me notice, btw, > that the second argument to ln is missing? Is it portable to assume > that it'll use . in that case? > Yes. From manpage: ln [OPTION]... TARGET (2nd form) ... in the 2nd form, create a link to TARGET in the current directory. Wei. > Jan >
>>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote: > On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote: >> On 16/02/17 18:56, Wei Liu wrote: >> > --- a/tools/fuzz/x86_instruction_emulator/Makefile >> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile >> > @@ -8,12 +8,16 @@ else >> > x86-instruction-emulator-fuzzer-all: >> > endif >> > >> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: >> > +x86_emulate: >> > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . >> > >> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: >> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate >> >> You should be able to do this: >> >> x86_emulate/%: x86_emulate >> > > Using % won't work. Jan made a similar comment during the first > iteration of the fuzzer series. It turned out that we needed to list > explicitly every file because of the other rules explicitly listed > those files. There was something in that area, but explicitly mentioning files elsewhere should not prevent this dependency to work as long as the files don't exist without the rule's commands having run. Jan
On Fri, Feb 17, 2017 at 03:33:26AM -0700, Jan Beulich wrote: > >>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote: > > On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote: > >> On 16/02/17 18:56, Wei Liu wrote: > >> > --- a/tools/fuzz/x86_instruction_emulator/Makefile > >> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > >> > @@ -8,12 +8,16 @@ else > >> > x86-instruction-emulator-fuzzer-all: > >> > endif > >> > > >> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > >> > +x86_emulate: > >> > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > >> > > >> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: > >> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate > >> > >> You should be able to do this: > >> > >> x86_emulate/%: x86_emulate > >> > > > > Using % won't work. Jan made a similar comment during the first > > iteration of the fuzzer series. It turned out that we needed to list > > explicitly every file because of the other rules explicitly listed > > those files. > > There was something in that area, but explicitly mentioning files > elsewhere should not prevent this dependency to work as long as > the files don't exist without the rule's commands having run. > I didn't dig out the old email so the description of the cause might be inaccurate. Do you want me to do anything other than explicitly listing every file as we did before? I'm not sure I get your request. Wei. > Jan >
>>> On 17.02.17 at 12:36, <wei.liu2@citrix.com> wrote: > On Fri, Feb 17, 2017 at 03:33:26AM -0700, Jan Beulich wrote: >> >>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote: >> > On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote: >> >> On 16/02/17 18:56, Wei Liu wrote: >> >> > --- a/tools/fuzz/x86_instruction_emulator/Makefile >> >> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile >> >> > @@ -8,12 +8,16 @@ else >> >> > x86-instruction-emulator-fuzzer-all: >> >> > endif >> >> > >> >> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: >> >> > +x86_emulate: >> >> > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . >> >> > >> >> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: >> >> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate >> >> >> >> You should be able to do this: >> >> >> >> x86_emulate/%: x86_emulate >> >> >> > >> > Using % won't work. Jan made a similar comment during the first >> > iteration of the fuzzer series. It turned out that we needed to list >> > explicitly every file because of the other rules explicitly listed >> > those files. >> >> There was something in that area, but explicitly mentioning files >> elsewhere should not prevent this dependency to work as long as >> the files don't exist without the rule's commands having run. > > I didn't dig out the old email so the description of the cause might be > inaccurate. > > Do you want me to do anything other than explicitly listing every file > as we did before? I'm not sure I get your request. Well, I'd prefer the shorter variant Andrew did suggest, so if that's not possible to be used here, I think it would help if the reason was recorded (in the commit message) now, to avoid later having the discussion yet another time. Jan
On Fri, Feb 17, 2017 at 04:44:45AM -0700, Jan Beulich wrote: > >>> On 17.02.17 at 12:36, <wei.liu2@citrix.com> wrote: > > On Fri, Feb 17, 2017 at 03:33:26AM -0700, Jan Beulich wrote: > >> >>> On 17.02.17 at 11:12, <wei.liu2@citrix.com> wrote: > >> > On Thu, Feb 16, 2017 at 07:10:41PM +0000, Andrew Cooper wrote: > >> >> On 16/02/17 18:56, Wei Liu wrote: > >> >> > --- a/tools/fuzz/x86_instruction_emulator/Makefile > >> >> > +++ b/tools/fuzz/x86_instruction_emulator/Makefile > >> >> > @@ -8,12 +8,16 @@ else > >> >> > x86-instruction-emulator-fuzzer-all: > >> >> > endif > >> >> > > >> >> > -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: > >> >> > +x86_emulate: > >> >> > [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . > >> >> > > >> >> > -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: > >> >> > +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate > >> >> > >> >> You should be able to do this: > >> >> > >> >> x86_emulate/%: x86_emulate > >> >> > >> > > >> > Using % won't work. Jan made a similar comment during the first > >> > iteration of the fuzzer series. It turned out that we needed to list > >> > explicitly every file because of the other rules explicitly listed > >> > those files. > >> > >> There was something in that area, but explicitly mentioning files > >> elsewhere should not prevent this dependency to work as long as > >> the files don't exist without the rule's commands having run. > > > > I didn't dig out the old email so the description of the cause might be > > inaccurate. > > > > Do you want me to do anything other than explicitly listing every file > > as we did before? I'm not sure I get your request. > > Well, I'd prefer the shorter variant Andrew did suggest, so if that's > not possible to be used here, I think it would help if the reason was > recorded (in the commit message) now, to avoid later having the > discussion yet another time. I think I figure out how to make it work. New version of patches coming soon. Wei.
diff --git a/tools/fuzz/x86_instruction_emulator/Makefile b/tools/fuzz/x86_instruction_emulator/Makefile index fede7e9afd..0cd49753cf 100644 --- a/tools/fuzz/x86_instruction_emulator/Makefile +++ b/tools/fuzz/x86_instruction_emulator/Makefile @@ -8,12 +8,16 @@ else x86-instruction-emulator-fuzzer-all: endif -x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: +x86_emulate: [ -L x86_emulate ] || ln -sf $(XEN_ROOT)/xen/arch/x86/x86_emulate . -asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: +x86_emulate/x86_emulate.c x86_emulate/x86_emulate.h: x86_emulate + +asm: [ -L asm ] || ln -sf $(XEN_ROOT)/xen/include/asm-x86 asm +asm/x86-vendors.h asm/x86-defns.h asm/msr-index.h: asm + x86_emulate.c x86_emulate.h: %: [ -L $* ] || ln -sf $(XEN_ROOT)/tools/tests/x86_emulator/$*
Several `ln -sf` can race with each other and cause error like: 14:43:56 00:07:06 O: ln: cannot remove 'asm': No such file or directory Provide dedicated targets for soft-linking directories. Reported-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Wei Liu <wei.liu2@citrix.com> --- tools/fuzz/x86_instruction_emulator/Makefile | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)