From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:32944) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLWUR-0003GW-AY for qemu-devel@nongnu.org; Wed, 11 Feb 2015 07:34:00 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLWUM-0003l7-9U for qemu-devel@nongnu.org; Wed, 11 Feb 2015 07:33:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38374) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLWUM-0003kw-12 for qemu-devel@nongnu.org; Wed, 11 Feb 2015 07:33:54 -0500 Date: Wed, 11 Feb 2015 13:33:43 +0100 From: "Michael S. Tsirkin" Message-ID: <20150211123343.GC4623@redhat.com> References: <1423511512-29208-1-git-send-email-mst@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Qemu-devel] [PATCH RFC] scripts/update-linux-headers.sh: pull virtio hdrs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Cornelia Huck , "Chen, Tiejun" , QEMU Developers , Stefan Hajnoczi , Alexander Graf On Wed, Feb 11, 2015 at 02:12:35AM +0000, Peter Maydell wrote: > On 9 February 2015 at 19:56, Michael S. Tsirkin wrote: > > +rm -rf "$output/standard-headers/linux" > > +mkdir -p "$output/standard-headers/linux" > > +for f in $tmpdir/include/linux/virtio*h; do > > + header=$(expr "$f" : '.*/\(.*\)'); > > + sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \ > > + -e 's/linux\/types/inttypes/' \ > > + -e 's/__bitwise__//' \ > > + "$tmpdir/include/linux/$header" > \ > > + "$output/standard-headers/linux/$header"; > > +done > > This doesn't seem to be doing anything to fix up > the '__attribute__((packed))' annotations. I don't know - what needs to be fixed up? > Presumably you're intending to put standard-headers/ > on the include path? It would probably be better to > not make the headers be in "linux/" in that case, > since it would mean confusion/clashes for what > "linux/virtio_net.h" etc mean -- are they the QEMU > sanitized versions or the host OS's? (Having them > in linux/ also makes code review harder since it breaks > the current rule of thumb that is "no include of > linux/anything in code that's not Linux-host-specific".) Agreed, I'll change this. > You need to strip out all the #include > from these headers, otherwise this won't build on non > Linux hosts. > Then you need to add in whatever the > equivalent is to get the defines/types those includes > were providing (for instance our virtio-net.h does a > simple #define of ETH_ALEN). You probably want to make > the update script fail if there's an include it's not > expecting to deal with, otherwise you're likely to > end up with a set of headers that seem OK on Linux > but fail when tested on other OSes -- better to fail > early and for the person trying to do the header update > than to end up with a change that won't pass my build > tests and gets bounced. OK, seems easy. Will do. > All that makes it seem to me like it's more trouble > than it's worth compared to doing a one-time manual > import. > > -- PMM Only true if we are perfect and don't make mistakes. If we do make mistakes, I want to fix them in one place and propagate the fix automatically. And since we are working on virtio 1.0 which is a huge change, introducing mistakes seems more likely. -- MST