Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC

[Wei Liu <wei.liu2@citrix.com>]

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