From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: [PATCH] stubdom/Makefile should use QEMU_TRADITIONAL_LOC Date: Fri, 24 Oct 2014 11:14:29 -0400 Message-ID: <20141024151429.GB9938@laptop.dumpdata.com> References: <20141024094457.GB15988@zion.uk.xensource.com> <20141024103641.GD15988@zion.uk.xensource.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <20141024103641.GD15988@zion.uk.xensource.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Wei Liu Cc: xen-devel@lists.xen.org, Stefano Stabellini , Ian Jackson , Ian Campbell , M A Young List-Id: xen-devel@lists.xenproject.org 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 And Wei confirmed that his "is corret" is an Acked-by: Wei Liu 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