From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chen, Tiejun" Subject: Re: [PATCH] tools/hvmloader: link errno.h from xen internal Date: Mon, 03 Nov 2014 10:03:23 +0800 Message-ID: <5456E26B.9040106@intel.com> References: <1414721915-11186-1-git-send-email-tiejun.chen@intel.com> <545361450200007800043DBC@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <545361450200007800043DBC@mail.emea.novell.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: Jan Beulich Cc: Ian Campbell , Ian Jackson , xen-devel@lists.xen.org, wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com List-Id: xen-devel@lists.xenproject.org On 2014/10/31 17:15, Jan Beulich wrote: >>>> On 31.10.14 at 03:18, wrote: > > (You omitted half of the tools maintainers; Cc-ing them now.) I think I already pick all relevant maintainers to review this patch, $ ./scripts/get_maintainer.pl 0004-tools-hvmloader-link-errno.h-from-xen-internal.patch Stefano Stabellini Wei Liu xen-devel@lists.xen.org So please tell me whom should be included specifically. > >> We will use some error numbers in hvmloader so here just link >> this head file from xen. > > This is not a proper reasoning for using the Xen internal header here. > You should make clear that we want to act on specific hypercall > error codes, and hence require the hypervisors view on the errno.h > values rather than the build environment's (as was sufficient for the > use in xenbus.c). > So rephrase, tools/hvmloader: link errno.h from xen internal We need to act on some specific hypercall error numbers, so require the hypervisor view on the errno.h value rather than just the build environment's number. So here link this headfile from xen. >> --- a/tools/firmware/hvmloader/Makefile >> +++ b/tools/firmware/hvmloader/Makefile >> @@ -84,9 +84,13 @@ ROMS += $(SEABIOS_ROM) >> endif >> >> .PHONY: all >> -all: subdirs-all >> +all: subdirs-all .dir >> $(MAKE) hvmloader >> >> +.dir: >> + @rm -rf errno.h > > Why? We should make sure we are linking to create a non-existing file, otherwise you may see this, ln: failed to create symbolic link '...': File exists > >> --- a/tools/firmware/hvmloader/util.h >> +++ b/tools/firmware/hvmloader/util.h >> @@ -6,6 +6,7 @@ >> #include >> #include >> #include >> +#include "errno.h" > > Does this allow xenbus.c to still build? I think this either should go into I did a test as follows: 1: make clean 2: make xen 3: make tools 4: make clean 5: make tools Is this covering xenbus? > the .c file wanting to use the values (preferable - remember my earlier > comment about introducing unnecessary dependencies?), or the Okay, I can remove this part, then just add this head file into any necessary .c files. This will be addressed in other RMRR patches. Thanks Tiejun > respective #include in xenbus.c should be dropped. > > Jan > >