[Libguestfs] [p2v PATCH 1/4] Makefile.am: factor out "make-physical-machine.sh"

Eric Blake eblake at redhat.com
Fri Aug 26 12:39:31 UTC 2022


On Fri, Aug 26, 2022 at 01:39:05PM +0200, Laszlo Ersek wrote:
> Extract and somewhat generalize the recipe for the $(PHYSICAL_MACHINE)
> target to a separate shell script. In preparation for the multiple steps
> we're going to introduce later, redirect virt-builder to a temp file at
> first (placed in the same directory as the finally expected disk image),
> and rename that file upon success.
> 
> Signed-off-by: Laszlo Ersek <lersek at redhat.com>
> ---

> +++ b/make-physical-machine.sh
> @@ -0,0 +1,37 @@
> +#!/bin/bash -

See the response in the other thread about not needing the - here.
Are we sure that /bin/bash is on all systems where this script will be
run, or is it better as '#!/bin/env bash'?

> +
> +set -e -u -C

My personal opinion is that 'set -e' is a crutch that should be
avoided because of its unintended interaction with functions; but I'm
not adamant enough about it to tell people to rip it out of scripts.
For short scripts, like this one, it's easy enough to check that we
aren't tripping over any of -e's surprises.

> +
> +disk=
> +
> +cleanup()
> +{
> +  set +e
> +  if test -n "$disk"; then
> +    rm -f -- "$disk"
> +    disk=
> +  fi
> +}
> +
> +trap cleanup EXIT

Is it intentional that you are not also cleaning up on signals like
INT and HUP?

> +
> +output=$1
> +outdir=$(dirname -- "$output")
> +disk=$(mktemp -p "$outdir" physical-machine.tmp.XXXXXXXXXX)
> +virt-builder --format raw -o "$disk" fedora-35
> +mv -- "$disk" "$output"
> +disk=

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


More information about the Libguestfs mailing list