From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34512) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCzeu-0004MH-Fx for qemu-devel@nongnu.org; Mon, 10 Feb 2014 17:49:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WCzep-00041E-Rw for qemu-devel@nongnu.org; Mon, 10 Feb 2014 17:49:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:10446) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WCzep-000419-JR for qemu-devel@nongnu.org; Mon, 10 Feb 2014 17:48:55 -0500 Date: Mon, 10 Feb 2014 17:48:41 -0500 From: Luiz Capitulino Message-ID: <20140210174841.51b3cf53@redhat.com> In-Reply-To: <52F94B3B.4080706@redhat.com> References: <1390890126-17377-1-git-send-email-qiaonuohan@cn.fujitsu.com> <1390890126-17377-4-git-send-email-qiaonuohan@cn.fujitsu.com> <20140210143526.0130040c@redhat.com> <52F94B3B.4080706@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v8 03/13] dump: add API to write header of flatten format List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Laszlo Ersek Cc: peter.maydell@linaro.org, stefanha@gmail.com, qemu-devel@nongnu.org, qiaonuohan , kumagai-atsushi@mxc.nes.nec.co.jp, anderson@redhat.com, afaerber@suse.de On Mon, 10 Feb 2014 22:57:15 +0100 Laszlo Ersek wrote: > On 02/10/14 20:35, Luiz Capitulino wrote: > > On Tue, 28 Jan 2014 14:21:56 +0800 > > qiaonuohan wrote: > >=20 > >> flatten format will be used when writing kdump-compressed format. The = format is > >> also used by makedumpfile, you can refer to the following URL to get m= ore > >> detailed information about flatten format of kdump-compressed format: > >> http://sourceforge.net/projects/makedumpfile/ > >> > >> The two functions here are used to write start flat header and end fla= t header > >> to vmcore, and they will be called later when flatten format is used. > >> > >> struct MakedumpfileHeader stored at the head of vmcore is used to indi= cate the > >> vmcore is in flatten format. > >> > >> struct MakedumpfileHeader { > >> char signature[16]; /* =3D "makedumpfile" */ > >> int64_t type; /* =3D 1 */ > >> int64_t version; /* =3D 1 */ > >> }; > >> > >> And struct MakedumpfileDataHeader, with offset and buf_size set to -1,= is used > >> to indicate the end of vmcore in flatten format. > >> > >> struct MakedumpfileDataHeader { > >> int64_t offset; /* =3D -1 */ > >> int64_t buf_size; /* =3D -1 */ > >> }; > >> > >> Signed-off-by: Qiao Nuohan > >> Reviewed-by: Laszlo Ersek > >=20 > > This patch breaks git bisect: > >=20 > > /home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:689:12: error: = =E2=80=98write_start_flat_header=E2=80=99 defined but not used [-Werror=3Du= nused-function] > > /home/lcapitulino/work/src/upstream/qmp-unstable/dump.c:715:12: error: = =E2=80=98write_end_flat_header=E2=80=99 defined but not used [-Werror=3Dunu= sed-function] > > cc1: all warnings being treated as errors > > make[1]: *** [dump.o] Error 1 > > make[1]: *** Waiting for unfinished jobs.... > > make: *** [subdir-x86_64-softmmu] Error 2 >=20 > This is one of the most useless warnings (when turned into an error). > It's common to add the building blocks first during a series, and then > to put them to use all at once, in one of the later patches. I have > faced this before (see commit 27d59ccd), and it sucks very hard. > Catching unused functions is helpful when you don't know about them. The > point of a patch series is that in patch N you know what you're going to > do in patch N+2. IOW, you know the future, while gcc doesn't. >=20 > Plus, bisection is *already* unusable with -Werror enabled. Suppose you > want to bisect a really long range, including commits that had been > written when gcc was only at, say, 4.4. At that time, gcc-4.4's -Werror > flag let some things pass that will now most certainly break your > bisection, when you compile the tree at those earlier commits with > gcc-4.8. I'm not speaking theoretically, I have factually witnessed this. >=20 > In effect, the -Werror flag *binds* a specific qemu commit to the gcc > version that is shipped by the main distros at the time of the commit. > In order to preserve all information, the commit would have to save the > gcc version to build it with, and git bisect should restore that > information (ie. require that you have that compiler version installed). >=20 > In short, -Werror and bisection don't mix. They already don't, and we > shouldn't expect them to. I understand what you're saying, and I don't want people to do needless and endless respins, but letting bisect break at will doesn't seem a good option either. What other options do we have? What's the general QEMU directive in cases l= ike this? 1. Do what you did in commit 27d59ccd? 2. Apply it and let bisect break? 3. Drop -Werror? >=20 > In practice, in order to silence this warning, Qiao would have to move > these function definitions into the patch that uses them (bad from a > design and review standpoint), or add useless calls inside this patch > (which I did in commit 27d59ccd, and which is ridiculous). >=20 > Qiao could also play with the diagnostics pragma (ie. suppress the > warning just in this patch, just for these functions), but that pragma > only works with gcc-4.6+, if memory serves. >=20 > Thanks > Laszlo >=20