Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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