From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50453) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c6Lbz-0006ap-LP for qemu-devel@nongnu.org; Mon, 14 Nov 2016 13:04:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c6Lbu-0008AB-Oq for qemu-devel@nongnu.org; Mon, 14 Nov 2016 13:04:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:59586) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c6Lbu-0008A0-JR for qemu-devel@nongnu.org; Mon, 14 Nov 2016 13:04:02 -0500 Date: Tue, 15 Nov 2016 02:03:59 +0800 From: Fam Zheng Message-ID: <20161114180359.GA24217@lemon> References: <1478566785-4002-1-git-send-email-ashish.mittal@veritas.com> <1478566785-4002-2-git-send-email-ashish.mittal@veritas.com> <20161114150701.GD26198@stefanha-x1.localdomain> <20161114154906.GC2373@lemon> <20161114165034.GA1352@stefanha-x1.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161114165034.GA1352@stefanha-x1.localdomain> Subject: Re: [Qemu-devel] [PATCH v6 1/2] block/vxhs.c: Add support for a new block device type called "vxhs" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Ashish Mittal , qemu-devel@nongnu.org, pbonzini@redhat.com, kwolf@redhat.com, armbru@redhat.com, berrange@redhat.com, jcody@redhat.com, ashish.mittal@veritas.com, Rakesh.Ranjan@veritas.com, Buddhi.Madhav@veritas.com, Ketan.Nilangekar@veritas.com, Abhijit.Dey@veritas.com, Venkatesha.Mg@veritas.com On Mon, 11/14 16:50, Stefan Hajnoczi wrote: > On Mon, Nov 14, 2016 at 11:49:06PM +0800, Fam Zheng wrote: > > On Mon, 11/14 15:07, Stefan Hajnoczi wrote: > > > On Mon, Nov 07, 2016 at 04:59:44PM -0800, Ashish Mittal wrote: > > > > diff --git a/block/vxhs.c b/block/vxhs.c > > > > new file mode 100644 > > > > index 0000000..8913e8f > > > > --- /dev/null > > > > +++ b/block/vxhs.c > > > > @@ -0,0 +1,689 @@ > > > > +/* > > > > + * QEMU Block driver for Veritas HyperScale (VxHS) > > > > + * > > > > + * This work is licensed under the terms of the GNU GPL, version 2 or later. > > > > + * See the COPYING file in the top-level directory. > > > > + * > > > > + */ > > > > + > > > > +#include "qemu/osdep.h" > > > > +#include "block/block_int.h" > > > > +#include > > > > > > 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. Fam