Re: [Qemu-devel] [PATCH V8 1/4] tests/migration: Convert x86 boot block compilation script into Makefile

[Wei Huang]

On 09/03/2018 06:08 AM, Andrew Jones wrote:
> On Sat, Sep 01, 2018 at 01:11:12AM -0400, Wei Huang wrote:
>> The x86 boot block header currently is generated with a shell script.
>> To better support other CPUs (e.g. aarch64), we convert the script
>> into Makefile. This allows us to 1) support cross-compilation easily,
>> and 2) avoid creating a script file for every architecture.
>>
>> Note that, in the new design, the cross compiler prefix can be specified by
>> setting the CROSS_PREFIX in "make" command. Also to allow gcc pre-processor
>> to include the C-style file correctly, it also renames the
>> x86-a-b-bootblock.s file extension from .s to .S.
>>
>> Signed-off-by: Wei Huang <address@hidden>
>> ---
>>  tests/migration/Makefile                           | 31 ++++++++++++++++++++
>>  tests/migration/rebuild-x86-bootblock.sh           | 33 
>> ----------------------
>>  .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   |  0
>>  3 files changed, 31 insertions(+), 33 deletions(-)
>>  create mode 100644 tests/migration/Makefile
>>  delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
>>  rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (100%)
>>
>> diff --git a/tests/migration/Makefile b/tests/migration/Makefile
>> new file mode 100644
>> index 0000000..5d5fa07
>> --- /dev/null
>> +++ b/tests/migration/Makefile
>> @@ -0,0 +1,31 @@
>> +# To specify cross compiler prefix, use CROSS_PREFIX=
>> +#   > make CROSS_PREFIX=x86_64-linux-gnu-
>        ^ nit: this prompt symbol is weird (at least to me, what shell uses
>               this?) To me it looks like a sh/bash redirect symbol. Can
>               we change it to '$' or use nothing at all?

I will replace '>' with '$', which is indeed the commonly-used one.

>> +
>> +override define __note
>> +/* This file is automatically generated from
>> + * tests/migration/x86-a-b-bootblock.s, edit that and then run
>                                         ^ should be 'S'

OK, will do.

> 
>> + * tests/migration/rebuild-x86-bootblock.sh to update,
>> + * and then remember to send both in your patch submission.
>> + */
>> +endef
>> +export __note
>> +
>> +.PHONY: all clean
>> +all: x86-a-b-bootblock.h
>> +
>> +x86-a-b-bootblock.h: x86.bootsect
>> +    echo "$$__note" > header.tmp
>> +    xxd -i $< | sed -e 's/.*int.*//' >> header.tmp
>> +    mv header.tmp $@
> 
> The shell script this Makefile is replacing used mktemp
> for a randomly named tmp dir. Shouldn't we continue to
> use random names?>
>> +
>> +x86.bootsect: x86.boot
>> +    dd if=$< of=$@ bs=256 count=2 skip=124
>> +
>> +x86.boot: x86.o
>> +    $(CROSS_PREFIX)objcopy -O binary $< $@
>> +
>> +x86.o: x86-a-b-bootblock.S
>> +    $(CROSS_PREFIX)gcc -m32 -march=i486 -c $< -o $@
>> +
>> +clean:
>> +    @rm -rf *.boot *.o *.bootsect
> 
> We don't want to remove the generated header file when cleaning?
> 
>> diff --git a/tests/migration/rebuild-x86-bootblock.sh 
>> b/tests/migration/rebuild-x86-bootblock.sh
>> deleted file mode 100755
>> index 86cec5d..0000000
>> --- a/tests/migration/rebuild-x86-bootblock.sh
>> +++ /dev/null
>> @@ -1,33 +0,0 @@
>> -#!/bin/sh
>> -# Copyright (c) 2016-2018 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: address@hidden
>> -
>> -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.XXXXXX)
>> -cd "$ASM_WORK_DIR" &&
>> -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"
>> diff --git a/tests/migration/x86-a-b-bootblock.s 
>> b/tests/migration/x86-a-b-bootblock.S
>> similarity index 100%
>> rename from tests/migration/x86-a-b-bootblock.s
>> rename to tests/migration/x86-a-b-bootblock.S
>> -- 
>> 1.8.3.1
>>
>>
> 
> Thanks,
> drew 
>