Re: [Qemu-devel] [PATCH v3] tests/migration: Add source to PC boot block

[Eric Blake <eblake@redhat.com>]

On 02/12/2018 12:34 PM, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> 
> The boot block used in the migration test is currently only
> shipped as a hex (with the source in the git commit message),

Would be nice to point to commit ea0c6d62 (I assume that's the commit 
message you're referring to).

> change this to actually include the source.

Yeah, GPL really wants us to ship the preferred editing form of sources ;)

> 
> A script is added to rebuild the header but the expectation is that
> the generated hex is shipped as well as the .s, so that
> there's no requirement to have just the right assembler etc.
> 
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---

> +++ b/tests/migration-test.c
> @@ -80,57 +80,13 @@ static const char *tmpfs;
>   /* A simple PC boot sector that modifies memory (1-100MB) quickly
>    * outputing a 'B' every so often if it's still running.

Pre-existing, but while here,
s/outputing/outputting/

> diff --git a/tests/migration/rebuild-x86-bootblock.sh b/tests/migration/rebuild-x86-bootblock.sh
> new file mode 100755
> index 0000000000..ee9b53ceb4
> --- /dev/null
> +++ b/tests/migration/rebuild-x86-bootblock.sh
> @@ -0,0 +1,35 @@
> +#!/bin/sh
> +# Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
> +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> +# See the COPYING file in the top-level directory.
> +#
> +# Author: dgilbert@redhat.com
> +
> +ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
> +HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
> +
> +if [ ! -e "$ASMFILE" ]
> +then
> +  echo "Couldn't find $ASMFILE" >&2
> +  exit 1
> +fi
> +
> +ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XXXXX)
Portable use of mktemp requires at least 6 X, not 5.

> +cd $ASM_WORK_DIR &&

Unsafe if $PWD contains spaces; needs to be quoted.

> +as --32 -march=i486 "$ASMFILE" -o x86.o &&
> +objcopy -O binary x86.o x86.boot &&
> +dd if=x86.boot of=x86.bootsect \
> +  bs=256 count=2 skip=124 &&
> +xxd -i x86.bootsect |
> +sed -e 's/.*int.*//' > x86.hex &&
> +cat - x86.hex <<HERE > "$HEADER"
> +/* This file is automatically generated from
> + * tests/migration/x86-a-b-bootblock.s, edit that and then run
> + * tests/migration/rebuild-x86-bootblock.sh to update,
> + * and then remember to send both in your patch submission.
> + */
> +HERE
> +
> +rm x86.hex x86.bootsect x86.boot x86.o
> +cd .. && rmdir $ASM_WORK_DIR

Another place that needs quoting.

> +++ b/tests/migration/x86-a-b-bootblock.s
> @@ -0,0 +1,92 @@
> +# x86 bootblock used in migration test
> +#  repeatedly increments the first byte of each page in a 100MB
> +#  range.
> +#  Outputs an initial 'A' on serial followed by repeated 'B's
> +#
> +# run   tests/migration/rebuild-x86-bootblock.sh
> +#   to regenerate the hex, and remember to include both the .h and .s
> +#   in any patches.
> +#
> +# Copyright (c) 2016 Red Hat, Inc. and/or its affiliates

Do you want to add 2018, since you've now modified things since the 
original commit?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org