Re: [Qemu-devel] [PATCH v6 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

From: Eric Blake <eblake@redhat.com>
To: Fam Zheng <famz@redhat.com>, Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, Venkatesha.Mg@veritas.com,
	ashish.mittal@veritas.com, jcody@redhat.com,
	qemu-devel@nongnu.org, Rakesh.Ranjan@veritas.com,
	armbru@redhat.com, Ketan.Nilangekar@veritas.com,
	Abhijit.Dey@veritas.com, pbonzini@redhat.com,
	Buddhi.Madhav@veritas.com, Ashish Mittal <ashmit602@gmail.com>
Subject: Re: [Qemu-devel] [PATCH v6 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"
Date: Mon, 14 Nov 2016 13:06:16 -0600	[thread overview]
Message-ID: <ff56438b-d75f-eb5a-c398-9ce1b20c2f40@redhat.com> (raw)
In-Reply-To: <20161114180359.GA24217@lemon>

[-- Attachment #1: Type: text/plain, Size: 1894 bytes --]

On 11/14/2016 12:03 PM, Fam Zheng wrote:

>>>> Please move system headers (<>) above user headers ("").  This way you
>>>> can be sure the system header isn't affected by any macros defined by
>>>> user headers.
>>>
>>> Yes, but still after "qemu/osdep.h", which prepares necessary bits for any other
>>> headers.
>>
>> I disagree.  qnio_api.h is a third-party library that doesn't need QEMU
>> headers to fix up the environment for it.
>>
>> By including osdep.h first you mask bugs in qnio_api.h.  Perhaps
>> qnio_api.h forgot to include a header and we won't notice because
>> osdep.h happened to bring in those headers first...
>>
>> Can you explain the rationale for your statement?
> 
> I point this out just because I rememebr this effort happened not long ago,
> which is to make osdep.h always included first (there is also a
> ./scripts/clean-includes to reorder the include):
> 
> https://lists.nongnu.org/archive/html/qemu-devel/2015-12/msg01110.html
> 
> I think it is mostly for uncommon compilers that should have little to do with
> libqnio in particular, but this is a common practice of current QEMU.

If the file is copied in verbatim from a third-party source, then it
should not be including osdep.h, and should be added to the list of
exempt files in scripts/clean-includes - at which point the file SHOULD
be clean because it should already be usable as-is in its third-party
original location.

If we modify the file as part of including it in qemu, then qemu rules
apply and having osdep.h first is good practice.

So I guess you have to determine if libqnio is something that should
compile completely independent from qemu, or whether it is so closely
tied to the rest of qemu that it should follow qemu conventions.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]