All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Andreas Färber" <afaerber@suse.de>
To: Olaf Hering <olaf@aepfle.de>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	"xen-devel@lists.xensource.com Devel"
	<xen-devel@lists.xensource.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386
Date: Thu, 05 Apr 2012 09:28:24 +0200	[thread overview]
Message-ID: <4F7D4998.8000000@suse.de> (raw)
In-Reply-To: <CAFEAcA-h2vQdYNNBTb_2hXEHZsK9023V9dVJLL9AvybuejBb_Q@mail.gmail.com>

Am 04.04.2012 18:09, schrieb Peter Maydell:
> On 4 April 2012 16:40, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Having looked at configure I'm pretty sure what we want here is
>>  QEMU_CFLAGS="-march=i486 $QEMU_CFLAGS"
>>
>> because we're only doing this for the benefit of a particular bit
>> of code in hw/vhost.c and so QEMU_CFLAGS is sufficient. Also this
>> brings it into line with other places where we add a -march flag,
>> which use QEMU_CFLAGS, not CFLAGS.
> 
> ...and having dug around in the git history we find that QEMU_CFLAGS
> were introduced in commit a558ee1, whose commit message defines the
> difference like this:
>     QEMU_CFLAGS: flags without which we can't compile
>     CFLAGS: "-g -O2"
> 
> "-march=i486" is clearly "flags without which we can't compile",
> so we should be setting it in QEMU_CFLAGS, not CFLAGS.
> 
> Olaf, if you want to submit a fixed patch I think we should apply
> it to upstream qemu.
> 
> PS: remarks like
> "This patch is against the qemu-xen tree, but it should apply also to
> qemu.git since it has the same issue. Please apply to both trees."
> 
> should go below the '---' in a patch email so they don't appear
> in the git commit message when the patch is applied.

And you also forgot to update the subject to something like:
"configure: Fix CFLAGS handling for i386" - this is for the QEMU
repository after all, so no need to put that into the commit message.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

WARNING: multiple messages have this Message-ID (diff)
From: "Andreas Färber" <afaerber@suse.de>
To: Olaf Hering <olaf@aepfle.de>
Cc: Peter Maydell <peter.maydell@linaro.org>,
	"xen-devel@lists.xensource.com Devel"
	<xen-devel@lists.xensource.com>,
	Ian Jackson <Ian.Jackson@eu.citrix.com>,
	qemu-devel@nongnu.org
Subject: Re: [Xen-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386
Date: Thu, 05 Apr 2012 09:28:24 +0200	[thread overview]
Message-ID: <4F7D4998.8000000@suse.de> (raw)
In-Reply-To: <CAFEAcA-h2vQdYNNBTb_2hXEHZsK9023V9dVJLL9AvybuejBb_Q@mail.gmail.com>

Am 04.04.2012 18:09, schrieb Peter Maydell:
> On 4 April 2012 16:40, Peter Maydell <peter.maydell@linaro.org> wrote:
>> Having looked at configure I'm pretty sure what we want here is
>>  QEMU_CFLAGS="-march=i486 $QEMU_CFLAGS"
>>
>> because we're only doing this for the benefit of a particular bit
>> of code in hw/vhost.c and so QEMU_CFLAGS is sufficient. Also this
>> brings it into line with other places where we add a -march flag,
>> which use QEMU_CFLAGS, not CFLAGS.
> 
> ...and having dug around in the git history we find that QEMU_CFLAGS
> were introduced in commit a558ee1, whose commit message defines the
> difference like this:
>     QEMU_CFLAGS: flags without which we can't compile
>     CFLAGS: "-g -O2"
> 
> "-march=i486" is clearly "flags without which we can't compile",
> so we should be setting it in QEMU_CFLAGS, not CFLAGS.
> 
> Olaf, if you want to submit a fixed patch I think we should apply
> it to upstream qemu.
> 
> PS: remarks like
> "This patch is against the qemu-xen tree, but it should apply also to
> qemu.git since it has the same issue. Please apply to both trees."
> 
> should go below the '---' in a patch email so they don't appear
> in the git commit message when the patch is applied.

And you also forgot to update the subject to something like:
"configure: Fix CFLAGS handling for i386" - this is for the QEMU
repository after all, so no need to put that into the commit message.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

  reply	other threads:[~2012-04-05  7:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-03 17:32 [Qemu-devel] [PATCH v2] qemu/configure: fix CFLAGS handling for i386 Olaf Hering
2012-04-03 17:32 ` Olaf Hering
2012-04-04 15:16 ` [Qemu-devel] [Xen-devel] " Ian Jackson
2012-04-04 15:16   ` Ian Jackson
2012-04-04 15:40   ` [Qemu-devel] [Xen-devel] " Peter Maydell
2012-04-04 15:40     ` [Qemu-devel] " Peter Maydell
2012-04-04 16:09     ` [Qemu-devel] [Xen-devel] " Peter Maydell
2012-04-04 16:09       ` [Qemu-devel] " Peter Maydell
2012-04-05  7:28       ` Andreas Färber [this message]
2012-04-05  7:28         ` [Xen-devel] " Andreas Färber

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=4F7D4998.8000000@suse.de \
    --to=afaerber@suse.de \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=olaf@aepfle.de \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=xen-devel@lists.xensource.com \
    /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.