[PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[M A Young]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 394 bytes --]

In 
http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=8962a8f951ea83e8d10ee23aeb20266e4795b06e 
CONFIG_QEMU was replaced by QEMU_TRADITIONAL_LOC in several places but not 
in stubdom/Makefile, and as a result building stubdom is likely to fail 
when xen-setup-stubdom isn't found. This patch replaces CONFIG_QEMU with 
QEMU_TRADITIONAL_LOC in stubdom/Makefile as well.

 	Michael Young

[-- Attachment #2: Type: TEXT/PLAIN, Size: 889 bytes --]

In commit 8962a8f951ea83e8d10ee23aeb20266e4795b06e CONFIG_QEMU was replaced
by QEMU_TRADITIONAL_LOC. However stubdom/Makefile still uses CONFIG_QEMU so
building stubdom is likely to fail.
This patch replaces CONFIG_QEMU with QEMU_TRADITIONAL_LOC in stubdom/Makefile
as well.

Signed-off-by: Michael Young <m.a.young@durham.ac.uk>

--- xen-4.5.0/stubdom/Makefile.orig	2014-10-22 21:07:26.000000000 +0100
+++ xen-4.5.0/stubdom/Makefile	2014-10-22 22:59:40.732112287 +0100
@@ -263,7 +263,7 @@
 .PHONY: $(CROSS_ROOT)
 $(CROSS_ROOT): cross-newlib cross-zlib cross-libpci
 
-QEMU_ROOT := $(shell if [ -d "$(CONFIG_QEMU)" ]; then echo "$(CONFIG_QEMU)"; else echo .; fi)
+QEMU_ROOT := $(shell if [ -d "$(QEMU_TRADITIONAL_LOC)" ]; then echo "$(QEMU_TRADITIONAL_LOC)"; else echo .; fi)
 
 ifneq ($(filter ioemu,$(STUBDOM_TARGETS)),)
 IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[Wei Liu]

On Thu, Oct 23, 2014 at 06:37:43PM +0100, M A Young wrote:
> In http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=8962a8f951ea83e8d10ee23aeb20266e4795b06e
> CONFIG_QEMU was replaced by QEMU_TRADITIONAL_LOC in several places but not
> in stubdom/Makefile, and as a result building stubdom is likely to fail when
> xen-setup-stubdom isn't found. This patch replaces CONFIG_QEMU with
> QEMU_TRADITIONAL_LOC in stubdom/Makefile as well.
> 

While I understand the rationale behind this change, I'm a bit confused
by the description. What does it mean by "is likely to fail"? Does it
mean it succeeds sometimes and fails sometimes?

What's your build setup? I'm wondering why this is not caught in
OSSTest.

Wei.

> 	Michael Young

> In commit 8962a8f951ea83e8d10ee23aeb20266e4795b06e CONFIG_QEMU was replaced
> by QEMU_TRADITIONAL_LOC. However stubdom/Makefile still uses CONFIG_QEMU so
> building stubdom is likely to fail.
> This patch replaces CONFIG_QEMU with QEMU_TRADITIONAL_LOC in stubdom/Makefile
> as well.
> 
> Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
> 
> --- xen-4.5.0/stubdom/Makefile.orig	2014-10-22 21:07:26.000000000 +0100
> +++ xen-4.5.0/stubdom/Makefile	2014-10-22 22:59:40.732112287 +0100
> @@ -263,7 +263,7 @@
>  .PHONY: $(CROSS_ROOT)
>  $(CROSS_ROOT): cross-newlib cross-zlib cross-libpci
>  
> -QEMU_ROOT := $(shell if [ -d "$(CONFIG_QEMU)" ]; then echo "$(CONFIG_QEMU)"; else echo .; fi)
> +QEMU_ROOT := $(shell if [ -d "$(QEMU_TRADITIONAL_LOC)" ]; then echo "$(QEMU_TRADITIONAL_LOC)"; else echo .; fi)
>  
>  ifneq ($(filter ioemu,$(STUBDOM_TARGETS)),)
>  IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[Ian Campbell]

On Fri, 2014-10-24 at 10:44 +0100, Wei Liu wrote:
> On Thu, Oct 23, 2014 at 06:37:43PM +0100, M A Young wrote:
> > In http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=8962a8f951ea83e8d10ee23aeb20266e4795b06e
> > CONFIG_QEMU was replaced by QEMU_TRADITIONAL_LOC in several places but not
> > in stubdom/Makefile, and as a result building stubdom is likely to fail when
> > xen-setup-stubdom isn't found. This patch replaces CONFIG_QEMU with
> > QEMU_TRADITIONAL_LOC in stubdom/Makefile as well.
> > 
> 
> While I understand the rationale behind this change, I'm a bit confused
> by the description. What does it mean by "is likely to fail"? Does it
> mean it succeeds sometimes and fails sometimes?
> 
> What's your build setup? I'm wondering why this is not caught in
> OSSTest.

Is it that osstest builds the regular tools first (and hence clones the
tree) before hitting the stubdom build whereas if you do the stubdom
build in isolation this doesn't happen?

> 
> Wei.
> 
> > 	Michael Young
> 
> > In commit 8962a8f951ea83e8d10ee23aeb20266e4795b06e CONFIG_QEMU was replaced
> > by QEMU_TRADITIONAL_LOC. However stubdom/Makefile still uses CONFIG_QEMU so
> > building stubdom is likely to fail.
> > This patch replaces CONFIG_QEMU with QEMU_TRADITIONAL_LOC in stubdom/Makefile
> > as well.
> > 
> > Signed-off-by: Michael Young <m.a.young@durham.ac.uk>
> > 
> > --- xen-4.5.0/stubdom/Makefile.orig	2014-10-22 21:07:26.000000000 +0100
> > +++ xen-4.5.0/stubdom/Makefile	2014-10-22 22:59:40.732112287 +0100
> > @@ -263,7 +263,7 @@
> >  .PHONY: $(CROSS_ROOT)
> >  $(CROSS_ROOT): cross-newlib cross-zlib cross-libpci
> >  
> > -QEMU_ROOT := $(shell if [ -d "$(CONFIG_QEMU)" ]; then echo "$(CONFIG_QEMU)"; else echo .; fi)
> > +QEMU_ROOT := $(shell if [ -d "$(QEMU_TRADITIONAL_LOC)" ]; then echo "$(QEMU_TRADITIONAL_LOC)"; else echo .; fi)
> >  
> >  ifneq ($(filter ioemu,$(STUBDOM_TARGETS)),)
> >  IOEMU_LINKFARM_TARGET := ioemu/linkfarm.stamp
> 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel
> 

Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[M A Young]

On Fri, 24 Oct 2014, Wei Liu wrote:

> On Thu, Oct 23, 2014 at 06:37:43PM +0100, M A Young wrote:
>> In http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=8962a8f951ea83e8d10ee23aeb20266e4795b06e
>> CONFIG_QEMU was replaced by QEMU_TRADITIONAL_LOC in several places but not
>> in stubdom/Makefile, and as a result building stubdom is likely to fail when
>> xen-setup-stubdom isn't found. This patch replaces CONFIG_QEMU with
>> QEMU_TRADITIONAL_LOC in stubdom/Makefile as well.
>>
>
> While I understand the rationale behind this change, I'm a bit confused
> by the description. What does it mean by "is likely to fail"? Does it
> mean it succeeds sometimes and fails sometimes?
>
> What's your build setup? I'm wondering why this is not caught in
> OSSTest.

If OSSTest is setting CONFIG_QEMU explicitly then the build of stubdom 
will work, as will everything else as QEMU_TRADITIONAL_LOC is set to the 
value of CONFIG_QEMU (in Config.mk) if it exists. Nothing in the 
xen code sets CONFIG_QEMU so if it isn't set explicitly stubdom/Makefile 
sets QEMU_ROOT to . in stubdom/Makefile which isn't the right path for 
xen-setup-stubdom (unless something is copying it over).

 	Michael Young

Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[Wei Liu]

On Fri, Oct 24, 2014 at 11:09:09AM +0100, M A Young wrote:
> On Fri, 24 Oct 2014, Wei Liu wrote:
> 
> >On Thu, Oct 23, 2014 at 06:37:43PM +0100, M A Young wrote:
> >>In http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=8962a8f951ea83e8d10ee23aeb20266e4795b06e
> >>CONFIG_QEMU was replaced by QEMU_TRADITIONAL_LOC in several places but not
> >>in stubdom/Makefile, and as a result building stubdom is likely to fail when
> >>xen-setup-stubdom isn't found. This patch replaces CONFIG_QEMU with
> >>QEMU_TRADITIONAL_LOC in stubdom/Makefile as well.
> >>
> >
> >While I understand the rationale behind this change, I'm a bit confused
> >by the description. What does it mean by "is likely to fail"? Does it
> >mean it succeeds sometimes and fails sometimes?
> >
> >What's your build setup? I'm wondering why this is not caught in
> >OSSTest.
> 
> If OSSTest is setting CONFIG_QEMU explicitly then the build of stubdom will
> work, as will everything else as QEMU_TRADITIONAL_LOC is set to the value of
> CONFIG_QEMU (in Config.mk) if it exists. Nothing in the xen code sets
> CONFIG_QEMU so if it isn't set explicitly stubdom/Makefile sets QEMU_ROOT to
> . in stubdom/Makefile which isn't the right path for xen-setup-stubdom
> (unless something is copying it over).
> 

OSSTest doesn't set CONFIG_QEMU.

And looking at stubdom/Makefile, even if QEMU_ROOT is set to ".", the
end result is still correct.

347           ( $(buildmakevars2shellvars); \                                       
348             cd ioemu ; \                                                        
349             LWIPDIR=$(CURDIR)/lwip-$(XEN_TARGET_ARCH) \                         
350             TARGET_CPPFLAGS="$(TARGET_CPPFLAGS)" \                              
351             TARGET_CFLAGS="$(TARGET_CFLAGS)" \                                  
352             TARGET_LDFLAGS="$(TARGET_LDFLAGS)" \                                
353             $(QEMU_ROOT)/xen-setup-stubdom )      

Also xen-setup-stubdom is tracked in that repository, not copied from
somewhere else. Probably worth checking what deleted it?

That said, I think this patch is correct and should be accepted as bug
fix, because it's doing what 8962a8f951e did.

Only one minor comment on the commit message, can you use short commit
hash with commit title instead of a URL to web git interface? Like

"In 8962a8f ("make: Normalize config options for external trees")
CONFIG_QEMU was replace by ..."

Wei.

> 	Michael Young

Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[Konrad Rzeszutek Wilk]

On Fri, Oct 24, 2014 at 11:36:41AM +0100, Wei Liu wrote:
> On Fri, Oct 24, 2014 at 11:09:09AM +0100, M A Young wrote:
> > On Fri, 24 Oct 2014, Wei Liu wrote:
> > 
> > >On Thu, Oct 23, 2014 at 06:37:43PM +0100, M A Young wrote:
> > >>In http://xenbits.xenproject.org/gitweb/?p=xen.git;a=commit;h=8962a8f951ea83e8d10ee23aeb20266e4795b06e
> > >>CONFIG_QEMU was replaced by QEMU_TRADITIONAL_LOC in several places but not
> > >>in stubdom/Makefile, and as a result building stubdom is likely to fail when
> > >>xen-setup-stubdom isn't found. This patch replaces CONFIG_QEMU with
> > >>QEMU_TRADITIONAL_LOC in stubdom/Makefile as well.
> > >>
> > >
> > >While I understand the rationale behind this change, I'm a bit confused
> > >by the description. What does it mean by "is likely to fail"? Does it
> > >mean it succeeds sometimes and fails sometimes?
> > >
> > >What's your build setup? I'm wondering why this is not caught in
> > >OSSTest.
> > 
> > If OSSTest is setting CONFIG_QEMU explicitly then the build of stubdom will
> > work, as will everything else as QEMU_TRADITIONAL_LOC is set to the value of
> > CONFIG_QEMU (in Config.mk) if it exists. Nothing in the xen code sets
> > CONFIG_QEMU so if it isn't set explicitly stubdom/Makefile sets QEMU_ROOT to
> > . in stubdom/Makefile which isn't the right path for xen-setup-stubdom
> > (unless something is copying it over).
> > 
> 
> OSSTest doesn't set CONFIG_QEMU.
> 
> And looking at stubdom/Makefile, even if QEMU_ROOT is set to ".", the
> end result is still correct.
> 
> 347           ( $(buildmakevars2shellvars); \                                       
> 348             cd ioemu ; \                                                        
> 349             LWIPDIR=$(CURDIR)/lwip-$(XEN_TARGET_ARCH) \                         
> 350             TARGET_CPPFLAGS="$(TARGET_CPPFLAGS)" \                              
> 351             TARGET_CFLAGS="$(TARGET_CFLAGS)" \                                  
> 352             TARGET_LDFLAGS="$(TARGET_LDFLAGS)" \                                
> 353             $(QEMU_ROOT)/xen-setup-stubdom )      
> 
> Also xen-setup-stubdom is tracked in that repository, not copied from
> somewhere else. Probably worth checking what deleted it?
> 
> That said, I think this patch is correct and should be accepted as bug
> fix, because it's doing what 8962a8f951e did.

Release-Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

And Wei confirmed that his "is corret" is an Acked-by: Wei Liu <wei.liu2@citrix.com>

Thank you!
> 
> Only one minor comment on the commit message, can you use short commit
> hash with commit title instead of a URL to web git interface? Like
> 
> "In 8962a8f ("make: Normalize config options for external trees")
> CONFIG_QEMU was replace by ..."
> 
> Wei.
> 
> > 	Michael Young
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[Wei Liu]

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