On Thu, 10 Sep 2015 10:42:31 +0300 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> On Wed, Sep 09, 2015 at 06:34:01PM +0200, Paolo Bonzini wrote: > > The Hyper-V definitions are an industry standard and can be used > > from code that is not KVM-specific. > > > > The changes to scripts/update-linux-headers.sh are required because there > > is both an asm-x86/hyperv.h and a linux/hyperv.h file. linux/hyperv.h > > introduces dependencies on additional Linux uapi headers, so we only > > want the former. > > > > The solution is to make cp_virtio (now renamed to cp_portable) copy > > one file only, instead of using the "find" command, and call it multiple > > times. The new function is really just a reindentation of the old one. > > > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > > I'd rather see a script update, then result of running it > in a separate patch. > > > --- > > .../standard-headers}/asm-x86/hyperv.h | 10 +- > > linux-headers/asm-x86/hyperv.h | 253 > > +----------------------------- > > scripts/update-linux-headers.sh | 79 +++++----- > > target-i386/kvm.c | 2 +- > > 4 files changed, 52 insertions(+), 295 deletions(-) > > copy {linux-headers => include/standard-headers}/asm-x86/hyperv.h (98%) > > diff --git a/scripts/update-linux-headers.sh > > b/scripts/update-linux-headers.sh > > index 7f7b592..2f25e84 100755 > > --- a/scripts/update-linux-headers.sh > > +++ b/scripts/update-linux-headers.sh > > @@ -28,39 +28,32 @@ if [ -z "$output" ]; then > > output="$PWD" > > fi > > > > -cp_virtio() { > > - from=$1 > > +cp_portable() { > > + f=$1 > > to=$2 > > - virtio=$(find "$from" -name '*virtio*h' -o -name "input.h" -o -name > > "pci_regs.h") > > - if [ "$virtio" ]; then > > - rm -rf "$to" > > - mkdir -p "$to" > > - for f in $virtio; do > > - if > > - grep '#include' "$f" | grep -v -e 'linux/virtio' \ > > - -e 'linux/types' \ > > - -e 'stdint' \ > > - -e 'linux/if_ether' \ > > - -e 'sys/' \ > > - > /dev/null > > - then > > - echo "Unexpected #include in input file $f". > > - exit 2 > > - fi > > - > > - header=$(basename "$f"); > > - sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \ > > - -e 's/__s\([0-9][0-9]*\)/int\1_t/g' \ > > - -e 's/__le\([0-9][0-9]*\)/uint\1_t/g' \ > > - -e 's/__be\([0-9][0-9]*\)/uint\1_t/g' \ > > - -e 's/<linux\/\([^>]*\)>/"standard-headers\/linux\/\1"/' \ > > - -e 's/__bitwise__//' \ > > - -e 's/__attribute__((packed))/QEMU_PACKED/' \ > > - -e 's/__inline__/inline/' \ > > - -e '/sys\/ioctl.h/d' \ > > - "$f" > "$to/$header"; > > - done > > + if > > + grep '#include' "$f" | grep -v -e 'linux/virtio' \ > > + -e 'linux/types' \ > > + -e 'stdint' \ > > + -e 'linux/if_ether' \ > > + -e 'sys/' \ > > + > /dev/null > > + then > > + echo "Unexpected #include in input file $f". > > + exit 2 > > fi > > + > > + header=$(basename "$f"); > > + sed -e 's/__u\([0-9][0-9]*\)/uint\1_t/g' \ > > + -e 's/__s\([0-9][0-9]*\)/int\1_t/g' \ > > + -e 's/__le\([0-9][0-9]*\)/uint\1_t/g' \ > > + -e 's/__be\([0-9][0-9]*\)/uint\1_t/g' \ > > + -e 's/<linux\/\([^>]*\)>/"standard-headers\/linux\/\1"/' \ > > + -e 's/__bitwise__//' \ > > + -e 's/__attribute__((packed))/QEMU_PACKED/' \ > > + -e 's/__inline__/inline/' \ > > + -e '/sys\/ioctl.h/d' \ > > + "$f" > "$to/$header"; > > } > > > > # This will pick up non-directories too (eg "Kconfig") but we will > > @@ -68,6 +61,7 @@ cp_virtio() { > > ARCHLIST=$(cd "$linux/arch" && echo *) > > > > for arch in $ARCHLIST; do > > + > > # Discard anything which isn't a KVM-supporting architecture > > if ! [ -e "$linux/arch/$arch/include/asm/kvm.h" ] && > > ! [ -e "$linux/arch/$arch/include/uapi/asm/kvm.h" ] ; then > > This empty line looks ugly imho. > > > @@ -86,14 +80,19 @@ for arch in $ARCHLIST; do > > for header in kvm.h kvm_para.h; do > > cp "$tmpdir/include/asm/$header" "$output/linux-headers/asm-$arch" > > done > > - if [ $arch = x86 ]; then > > - cp "$tmpdir/include/asm/hyperv.h" "$output/linux-headers/asm-x86" > > - fi > > if [ $arch = powerpc ]; then > > cp "$tmpdir/include/asm/epapr_hcalls.h" > > "$output/linux-headers/asm-powerpc/" > > fi > > > > - cp_virtio "$tmpdir/include/asm" > > "$output/include/standard-headers/asm-$arch" > > + rm -rf "$output/include/standard-headers/asm-$arch" > > + mkdir -p "$output/include/standard-headers/asm-$arch" > > + if [ $arch = s390 ]; then > > + cp_portable "$tmpdir/include/asm/kvm_virtio.h" > > "$output/include/standard-headers/asm-s390/" > > + cp_portable "$tmpdir/include/asm/virtio-ccw.h" > > "$output/include/standard-headers/asm-s390/" > > I think it's possible that s390 will split its virtio files up in > the future, I frankly don't see how we'd want to split those up any further. > or that more architectures will add their own. Probably unlikely for virtio files: s390 is the only one with two unique transports. For other portable headers: possible; but I'd think they're more likely to appear in architecture-independent code. > See below for a suggestion. > > > > + fi > > + if [ $arch = x86 ]; then > > + cp_portable "$tmpdir/include/asm/hyperv.h" > > "$output/include/standard-headers/asm-x86/" > > + fi > > done > > > > rm -rf "$output/linux-headers/linux" > > @@ -113,6 +112,9 @@ else > > cp "$linux/COPYING" "$output/linux-headers" > > fi > > > > +cat <<EOF >$output/linux-headers/asm-x86/hyperv.h > > +#include "standard-headers/asm-x86/hyperv.h" > > +EOF > > I don't think this is needed. We only did this for > virtio_config to avoid the code churn. Hyperv has > a single user, so no issue. > > > cat <<EOF >$output/linux-headers/linux/virtio_config.h > > #include "standard-headers/linux/virtio_config.h" > > EOF > > @@ -120,7 +122,12 @@ cat <<EOF >$output/linux-headers/linux/virtio_ring.h > > #include "standard-headers/linux/virtio_ring.h" > > EOF > > > > -cp_virtio "$tmpdir/include/linux/" "$output/include/standard-headers/linux" > > +rm -rf "$output/include/standard-headers/linux" > > +mkdir -p "$output/include/standard-headers/linux" > > +for i in "$tmpdir"/include/linux/*virtio*.h > > "$tmpdir/include/linux/input.h" \ > > + "$tmpdir/include/linux/pci_regs.h"; do > > + cp_portable "$i" "$output/include/standard-headers/linux" > > +done > > How about we move the above loop into cp_virtio? > Then we can reuse it for asm like we did. > hyperv can use cp_portable if it wants to. I prefer specifying the files to copy outside of the copying function: it's much more obvious what's going on. Otherwise, you get "if there's a file that happens to match this pattern, copy it" - and it's unclear to the casual reader which of those actually exist, as linux/ and asm/ have different sets of files. > > > > > cat <<EOF >$output/include/standard-headers/linux/types.h > > #include <stdint.h>