Re: [RFC PATCH V2 1/8] xen/mem_event: Cleanup of mem_event structures

From: "Jan Beulich" <JBeulich@suse.com>
To: Tamas K Lengyel <tamas.lengyel@zentific.com>
Cc: Tim Deegan <tim@xen.org>, Kevin Tian <kevin.tian@intel.com>,
	wei.liu2@citrix.com, Ian Campbell <ian.campbell@citrix.com>,
	Razvan Cojocaru <rcojocaru@bitdefender.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Eddie Dong <eddie.dong@intel.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Andres Lagar-Cavilla <andres@lagarcavilla.org>,
	Jun Nakajima <jun.nakajima@intel.com>,
	rshriram@cs.ubc.ca, Keir Fraser <keir@xen.org>,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	yanghy@cn.fujitsu.com
Subject: Re: [RFC PATCH V2 1/8] xen/mem_event: Cleanup of mem_event structures
Date: Thu, 29 Jan 2015 12:02:45 +0000	[thread overview]
Message-ID: <54CA2F75020000780005ACDB@mail.emea.novell.com> (raw)
In-Reply-To: <CAErYnsigmNOL7a7RCMnicnUmdYd8oij_ap1k1M4H0COUbHrSuA@mail.gmail.com>

>>> On 29.01.15 at 12:54, <tamas.lengyel@zentific.com> wrote:
> On Thu, Jan 22, 2015 at 4:34 PM, Tamas K Lengyel
> <tamas.lengyel@zentific.com> wrote:
>> On Thu, Jan 22, 2015 at 4:00 PM, Jan Beulich <JBeulich@suse.com> wrote:
>>>>>> On 18.01.15 at 16:17, <tamas.lengyel@zentific.com> wrote:
>>>> --- a/xen/include/Makefile
>>>> +++ b/xen/include/Makefile
>>>> @@ -90,7 +90,7 @@ ifeq ($(XEN_TARGET_ARCH),$(XEN_COMPILE_ARCH))
>>>>
>>>>  all: headers.chk
>>>>
>>>> -headers.chk: $(filter-out public/arch-% public/%ctl.h public/xsm/%
>>>> public/%hvm/save.h, $(wildcard public/*.h public/*/*.h) $(public-y)) Makefile
>>>> +headers.chk: $(filter-out public/arch-% public/%ctl.h public/mem_event.h
>>>> public/xsm/% public/%hvm/save.h, $(wildcard public/*.h public/*/*.h)
>>>> $(public-y)) Makefile
>>>
>>> I think you should finally break this already too long line. But of course
>>> first of all you'll want to explain why the addition is necessary/correct.
>>> The mere fact that this now becomes a tools-only interface isn't
>>> enough imo - some of the other headers excluded here would better
>>> undergo the checking too, just that first they would need cleaning up.
>>
>> I have to revisit what is actually going on here, I believe I had to
>> add this to get Xen to build. On a second look not sure why that was.
> 
> So I double checked and the addition here is correct, without
> excluding the header, the compilation fails at #if !defined(__XEN__)
> && !defined(__XEN_TOOLS__) in mem_event.h.

Hardly at that line. I'm afraid I won't be convinced without you
explaining what fails why, and why that can't be dealt with.

> So I'll break the line but
> cleaning up/checking the other headers IMHO is out of scope for this
> series.

Of course, no-one asked you to do this (and even less so in this series).

Jan