All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anthony PERARD <anthony.perard@citrix.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: xen-devel@lists.xenproject.org,
	Paul Durrant <paul.durrant@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	qemu-devel@nongnu.org
Subject: Re: [Xen-devel] [Qemu-devel] [PATCH v2 1/4] xen: Fix build with public headers
Date: Thu, 20 Jun 2019 15:44:46 +0100	[thread overview]
Message-ID: <20190620144446.GD13449@perard.uk.xensource.com> (raw)
In-Reply-To: <20190618121408.GM28525@redhat.com>

On Tue, Jun 18, 2019 at 01:14:08PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jun 18, 2019 at 12:23:38PM +0100, Anthony PERARD wrote:
> > Following 37677d7db3 "Clean up a few header guard symbols", QEMU start
> > to fail to build:
> > 
> > In file included from ~/xen/tools/../tools/include/xen/io/blkif.h:31:0,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen_blkif.h:5,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen-block.c:22:
> > ~/xen/tools/../tools/include/xen/io/ring.h:68:0: error: "__CONST_RING_SIZE" redefined [-Werror]
> >  #define __CONST_RING_SIZE(_s, _sz) \
> > 
> > In file included from ~/xen/tools/qemu-xen-dir/hw/block/xen_blkif.h:4:0,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen-block.c:22:
> > ~/xen/tools/qemu-xen-dir/include/hw/xen/io/ring.h:66:0: note: this is the location of the previous definition
> >  #define __CONST_RING_SIZE(_s, _sz) \
> > 
> > The issue is that some public xen headers have been imported (by
> > f65eadb639 "xen: import ring.h from xen") but not all. With the change
> > in the guards symbole, the ring.h header start to be imported twice.
> 
> Ah, so the include/hw/xen/io/ring.h file in tree is a copy of
> /usr/include/xen/io/ring.h from xen-devel.  Previously both
> these used "#ifndef __XEN_PUBLIC_IO_RING_H__". After
> the header guard cleanup in 37677d7db3, our local copy used a
> different header guard from the installed copy & thus we're
> not protected from dual inclusion.
> 
> IMHO the right solutions here are either
> 
>  - Don't copy public Xen headers into our tree
>  - Keep our Xen header copies identical to the originals
> 
> Importing public headers and then changing them locally is the worst
> thing to do. With that in mind I think we should revert the part of
> commit 37677d7db3 that touched the imported Xen headers.

Yes, it's propably a better thing to do. So, I'm going to update the
series and do:
- revert part of 37677d7db3
- import the public headers that depends on ring.h. Or in other words,
  the one that describe an interface with a guest.
  I'll do some modification on the headers but only to remove the stuff
  that QEMU doesn't need (like how to make an hypercall).

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

WARNING: multiple messages have this Message-ID (diff)
From: Anthony PERARD <anthony.perard@citrix.com>
To: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: xen-devel@lists.xenproject.org,
	Paul Durrant <paul.durrant@citrix.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 1/4] xen: Fix build with public headers
Date: Thu, 20 Jun 2019 15:44:46 +0100	[thread overview]
Message-ID: <20190620144446.GD13449@perard.uk.xensource.com> (raw)
In-Reply-To: <20190618121408.GM28525@redhat.com>

On Tue, Jun 18, 2019 at 01:14:08PM +0100, Daniel P. Berrangé wrote:
> On Tue, Jun 18, 2019 at 12:23:38PM +0100, Anthony PERARD wrote:
> > Following 37677d7db3 "Clean up a few header guard symbols", QEMU start
> > to fail to build:
> > 
> > In file included from ~/xen/tools/../tools/include/xen/io/blkif.h:31:0,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen_blkif.h:5,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen-block.c:22:
> > ~/xen/tools/../tools/include/xen/io/ring.h:68:0: error: "__CONST_RING_SIZE" redefined [-Werror]
> >  #define __CONST_RING_SIZE(_s, _sz) \
> > 
> > In file included from ~/xen/tools/qemu-xen-dir/hw/block/xen_blkif.h:4:0,
> >                  from ~/xen/tools/qemu-xen-dir/hw/block/xen-block.c:22:
> > ~/xen/tools/qemu-xen-dir/include/hw/xen/io/ring.h:66:0: note: this is the location of the previous definition
> >  #define __CONST_RING_SIZE(_s, _sz) \
> > 
> > The issue is that some public xen headers have been imported (by
> > f65eadb639 "xen: import ring.h from xen") but not all. With the change
> > in the guards symbole, the ring.h header start to be imported twice.
> 
> Ah, so the include/hw/xen/io/ring.h file in tree is a copy of
> /usr/include/xen/io/ring.h from xen-devel.  Previously both
> these used "#ifndef __XEN_PUBLIC_IO_RING_H__". After
> the header guard cleanup in 37677d7db3, our local copy used a
> different header guard from the installed copy & thus we're
> not protected from dual inclusion.
> 
> IMHO the right solutions here are either
> 
>  - Don't copy public Xen headers into our tree
>  - Keep our Xen header copies identical to the originals
> 
> Importing public headers and then changing them locally is the worst
> thing to do. With that in mind I think we should revert the part of
> commit 37677d7db3 that touched the imported Xen headers.

Yes, it's propably a better thing to do. So, I'm going to update the
series and do:
- revert part of 37677d7db3
- import the public headers that depends on ring.h. Or in other words,
  the one that describe an interface with a guest.
  I'll do some modification on the headers but only to remove the stuff
  that QEMU doesn't need (like how to make an hypercall).

Thanks,

-- 
Anthony PERARD


  reply	other threads:[~2019-06-20 14:45 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-18 11:23 [Xen-devel] [PATCH v2 0/4] Fix build of Xen support + cleanup Anthony PERARD
2019-06-18 11:23 ` [Qemu-devel] " Anthony PERARD
2019-06-18 11:23 ` [Xen-devel] [PATCH v2 1/4] xen: Fix build with public headers Anthony PERARD
2019-06-18 11:23   ` [Qemu-devel] " Anthony PERARD
2019-06-18 12:14   ` [Xen-devel] " Daniel P. Berrangé
2019-06-18 12:14     ` Daniel P. Berrangé
2019-06-20 14:44     ` Anthony PERARD [this message]
2019-06-20 14:44       ` Anthony PERARD
2019-06-18 11:23 ` [Xen-devel] [PATCH v2 2/4] xen: Import other xen/io/*.h Anthony PERARD
2019-06-18 11:23   ` [Qemu-devel] " Anthony PERARD
2019-06-18 11:23 ` [Xen-devel] [PATCH v2 3/4] xen: Drop includes of xen/hvm/params.h Anthony PERARD
2019-06-18 11:23   ` [Qemu-devel] " Anthony PERARD
2019-06-18 11:51   ` [Xen-devel] " Paul Durrant
2019-06-18 11:51     ` [Qemu-devel] " Paul Durrant
2019-06-18 11:23 ` [Xen-devel] [PATCH v2 4/4] xen: Avoid VLA Anthony PERARD
2019-06-18 11:23   ` [Qemu-devel] " Anthony PERARD
2019-06-18 11:35   ` [Xen-devel] " Philippe Mathieu-Daudé
2019-06-18 11:35     ` Philippe Mathieu-Daudé
2019-06-18 11:52   ` [Xen-devel] " Paul Durrant
2019-06-18 11:52     ` [Qemu-devel] " Paul Durrant
2019-06-18 12:00 ` [Xen-devel] [PATCH v2 0/4] Fix build of Xen support + cleanup no-reply
2019-06-18 12:00   ` [Qemu-devel] " no-reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190620144446.GD13449@perard.uk.xensource.com \
    --to=anthony.perard@citrix.com \
    --cc=berrange@redhat.com \
    --cc=paul.durrant@citrix.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.