[PATCH v3 00/15] Grub-shell improvements

* [PATCH v3 00/15] Grub-shell improvements
@ 2022-02-10 21:51 Glenn Washburn
  2022-02-10 21:51 ` [PATCH v3 01/15] grub-shell: Allow specifying non-default trim line contents Glenn Washburn
                   ` (14 more replies)
  0 siblings, 15 replies; 16+ messages in thread
From: Glenn Washburn @ 2022-02-10 21:51 UTC (permalink / raw)
  To: Daniel Kiper, grub-devel; +Cc: Glenn Washburn

Updates since v2:
* Remove the QEMU output log if not running in debug mode
* The change in v1 to use firmware files had a crucial bug in that the pflash
  devine was missing a unit number, which I believe was being defaulted to 0.
  This was causing the VARS pflash devices to be used instead of (as opposed to
  in addition to) the CODE pflash causing the QEMU boot to hang.
* Better handling for when the VARS file doesn't exist
* Remove the QEMU output pipe after waiting for all processes to terminate and
  do the wait before potentially removing files in working directory
* Added 3 new patches 8, 9, and 10, which in order
 1. Prevent removing working directory files if QEMU did not finish successfully
 2. Create run.sh in working directory to easily re-run the QEMU part of the
    test that generated the working directory
 3. Separate out GRUB_QEMU_OPTS so that its easy to see what, if any, extra QEMU
    options were passed during the test (which could be why its failing)

Updates since v1:
* Improve QEMU logging patch to make sure all data is written to the pipe before
  exiting (otherwise tests can fail because they don't get the last bit of QEMU
  output).
* Improve QEMU firmware handling to prefer using the -bios option (for older
  setups) and prefer firmware files found in the source directory so that
  system firmware can be overridden and systems without packages providing the
  firmware can be used.

Most of these patches aid in debugging test failures. Some, like the trim
patches, extend grub-shell functionality for some new tests cases not in this
patch series. I've included these patches as a group even though most of these
patches to not rely on each other. I expect that there will be some that may
not be desired for inclusion, but hope that the ones that can, will be
selectively applied.

Patch #14 is meant to address an obvious problem, a using non-existant QEMU
machine type. But I'm not sure its correct because I haven't gotten the
affected tests working.

Patch #15 improves EFI target tests by loading the EFI bios via pflash devices,
as is standard on many distros, if no binary usable with -bios is available.
Assumptions about the name and location of these firmware binaries are based
on Debian, however properly named firmware binaries found in the source
directory will be used first.

Glenn

Glenn Washburn (15):
  grub-shell: Allow specifying non-default trim line contents
  grub-shell: Trim line should always be matched from the beginning of
    the line
  grub-shell: Only show grub-mkrescue output if it returns an error
  grub-shell: Allow setting default timeout via
    GRUB_SHELL_DEFAULT_TIMEOUT envvar
  grub-shell: Put all generated files into working dir and use better
    file names
  grub-shell: Add grub output logfile with grub-shell --debug
  grub-shell: Set exit status to qemu exit status
  grub-shell: Only cleanup working directory file if QEMU does not fail
    or timeout
  grub-shell: Create run.sh in working directory for easily running test
    again
  grub-shell: Add $GRUB_QEMU_OPTS to run.sh to easily see unofficial
    QEMU arguments
  tests: Allow turning on shell tracing from environment variables
  grub-shell: Add --verbose to mkrescue when $debug is greater than 2
  grub-shell: Only turn on qemu head when large debug value is specified
  grub-shell: Use malta qemu-mips machine type instead off non-existant
    indy
  grub-shell: Add flexibility in QEMU firmware handling

 tests/util/grub-fs-tester.in |   2 +
 tests/util/grub-shell.in     | 229 ++++++++++++++++++++++++++++++-----
 2 files changed, 198 insertions(+), 33 deletions(-)

Range-diff against v2:
 1:  52df3299f =  1:  52df3299f grub-shell: Allow specifying non-default trim line contents
 2:  7c8264aeb =  2:  7c8264aeb grub-shell: Trim line should always be matched from the beginning of the line
 3:  c17da94e7 =  3:  c17da94e7 grub-shell: Only show grub-mkrescue output if it returns an error
 4:  27717b949 =  4:  27717b949 grub-shell: Allow setting default timeout via GRUB_SHELL_DEFAULT_TIMEOUT envvar
 5:  17dd72798 =  5:  17dd72798 grub-shell: Put all generated files into working dir and use better file names
 6:  428698acd !  6:  9be47ff05 grub-shell: Add grub output logfile with grub-shell --debug
    @@ tests/util/grub-shell.in: elif [ x$boot = xemu ]; then
      fi
      if [ x$boot = xcoreboot ]; then
          test -n "$debug" || rm -f "${imgfile}"
    + fi
    + test -n "$debug" || rm -f "${isofile}"
    + test -n "$debug" || rm -rf "${rom_directory}"
    +-test -n "$debug" || rm -f "${tmpfile}" "${cfgfile}"
    ++test -n "$debug" || rm -f "${tmpfile}" "${cfgfile}" "${goutfile}"
    ++
    + exit 0
    + 
    + 
 7:  cdd28473c !  7:  65e3da9c1 grub-shell: Set exit status to qemu exit status
    @@ tests/util/grub-shell.in: elif [ x$boot = xemu ]; then
     +    setup_qemu_logger
     +    timeout -s KILL $timeout "${qemu}" ${qemuopts} ${serial_null} -serial file:/dev/stdout -${device}"${isofile}" ${bootdev} > "$work_directory/qemu-pipe" || ret=$?
      fi
    ++
    ++wait
    ++rm -f "$work_directory/qemu-pipe"
    ++
      if [ x$boot = xcoreboot ]; then
          test -n "$debug" || rm -f "${imgfile}"
    -@@ tests/util/grub-shell.in: fi
    - test -n "$debug" || rm -f "${isofile}"
    + fi
    +@@ tests/util/grub-shell.in: test -n "$debug" || rm -f "${isofile}"
      test -n "$debug" || rm -rf "${rom_directory}"
    - test -n "$debug" || rm -f "${tmpfile}" "${cfgfile}"
    + test -n "$debug" || rm -f "${tmpfile}" "${cfgfile}" "${goutfile}"
    + 
     -exit 0
    -+rm -f "$work_directory/qemu-pipe"
    -+wait
     +exit $ret

 -:  --------- >  8:  250cdc08a grub-shell: Only cleanup working directory file if QEMU does not fail or timeout
 -:  --------- >  9:  39c6078bd grub-shell: Create run.sh in working directory for easily running test again
 -:  --------- > 10:  74539e4a7 grub-shell: Add $GRUB_QEMU_OPTS to run.sh to easily see unofficial QEMU arguments
 8:  d82bb5b49 = 11:  aed2c4949 tests: Allow turning on shell tracing from environment variables
 9:  16e95b1aa = 12:  34a61113c grub-shell: Add --verbose to mkrescue when $debug is greater than 2
10:  20e3f04db = 13:  2d390a7bc grub-shell: Only turn on qemu head when large debug value is specified
11:  10c55d9f3 = 14:  799fbdfd0 grub-shell: Use malta qemu-mips machine type instead off non-existant indy
12:  059ef632e ! 15:  80171fcab grub-shell: Add flexibility in QEMU firmware handling
    @@ Commit message
         it stores firmware files. If no firmware files are found, print an error
         message telling the user that those files must exist and exit with error.

    +    Do not load the system 32-bit ARM firmware VARS file because it must be
    +    writable to prevent a data exception and boot failure. So in order to use
    +    the VARS file, it must be copied to a writable location, but its quite large
    +    at 64M and is not needed to boot successfully.
    +
      ## tests/util/grub-shell.in ##
     @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platform}" in
      	boot=cd
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	if [ -f "$bios" ]; then
     +	    qemuopts="-bios $bios $qemuopts"
     +	elif [ -f "$pflash_code" ]; then
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++	    qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
    ++	    if [ -f "$pflash_vars" ]; then
    ++		qemuopts="-drive if=pflash,format=raw,unit=1,readonly=on,file=$pflash_vars $qemuopts"
    ++	    fi
     +	else
     +	    bios=/usr/share/qemu/OVMF32.fd
     +	    pflash_code=/usr/share/OVMF/OVMF32_CODE_4M.secboot.fd
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	    if [ -f "$bios" ]; then
     +		qemuopts="-bios $bios $qemuopts"
     +	    elif [ -f "$pflash_code" ]; then
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++		qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
    ++		qemuopts="-drive if=pflash,format=raw,unit=1,readonly=on,file=$pflash_vars $qemuopts"
     +	    else
     +		echo "Firmware not found, please make sure $bios or both $pflash_code and $pflash_vars exist." >&2
     +		exit 1
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	if [ -f "$bios" ]; then
     +	    qemuopts="-bios $bios $qemuopts"
     +	elif [ -f "$pflash_code" ]; then
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++	    qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
    ++	    if [ -f "$pflash_vars" ]; then
    ++		qemuopts="-drive if=pflash,format=raw,unit=1,readonly=on,file=$pflash_vars $qemuopts"
    ++	    fi
     +	else
     +	    bios=/usr/share/qemu/OVMF.fd
     +	    pflash_code=/usr/share/OVMF/OVMF_CODE.fd
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	    if [ -f "$bios" ]; then
     +		qemuopts="-bios $bios $qemuopts"
     +	    elif [ -f "$pflash_code" ]; then
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++		qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
    ++		qemuopts="-drive if=pflash,format=raw,unit=1,readonly=on,file=$pflash_vars $qemuopts"
     +	    else
     +		echo "Firmware not found, please make sure $bios or both $pflash_code and $pflash_vars exist." >&2
     +		exit 1
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	if [ -f "$bios" ]; then
     +	    qemuopts="-bios $bios $qemuopts"
     +	elif [ -f "$pflash_code" ]; then
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++	    qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
    ++	    if [ -f "$pflash_vars" ]; then
    ++		qemuopts="-drive if=pflash,format=raw,unit=1,readonly=on,file=$pflash_vars $qemuopts"
    ++	    fi
     +	else
     +	    bios=/usr/share/qemu-efi-aarch64/QEMU_EFI.fd
     +	    pflash_code=/usr/share/AAVMF/AAVMF_CODE.fd
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	    if [ -f "$bios" ]; then
     +		qemuopts="-bios $bios $qemuopts"
     +	    elif [ -f "$pflash_code" ]; then
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++		qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
    ++		qemuopts="-drive if=pflash,format=raw,unit=1,readonly=on,file=$pflash_vars $qemuopts"
     +	    else
     +		echo "Firmware not found, please make sure $bios or both $pflash_code and $pflash_vars exist." >&2
     +		exit 1
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	if [ -f "$bios" ]; then
     +	    qemuopts="-bios $bios $qemuopts"
     +	elif [ -f "$pflash_code" ]; then
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+	    qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++	    qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
    ++	    if [ -f "$pflash_vars" ]; then
    ++		qemuopts="-drive if=pflash,format=raw,unit=1,file=$pflash_vars $qemuopts"
    ++	    fi
     +	else
     +	    bios=/usr/share/AAVMF/AAVMF32.fd
     +	    pflash_code=/usr/share/AAVMF/AAVMF32_CODE.fd
    @@ tests/util/grub-shell.in: case "${grub_modinfo_target_cpu}-${grub_modinfo_platfo
     +	    if [ -f "$bios" ]; then
     +		qemuopts="-bios $bios $qemuopts"
     +	    elif [ -f "$pflash_code" ]; then
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_code $qemuopts"
    -+		qemuopts="-drive if=pflash,format=raw,readonly=on,file=$pflash_vars $qemuopts"
    ++		# NOTE: Do not use the pflash VARS file because it cannot be
    ++		# used in readonly. So it would need to be copied to a writable
    ++		# path, but its large at 64M and not needed for running tests.
    ++		qemuopts="-drive if=pflash,format=raw,unit=0,readonly=on,file=$pflash_code $qemuopts"
     +	    else
     +		echo "Firmware not found, please make sure $bios or both $pflash_code and $pflash_vars exist." >&2
     +		exit 1
-- 
2.27.0

^ permalink raw reply	[flat|nested] 16+ messages in thread