| Message ID | 20170628200857.1718-5-rkrcmar@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 28/06/2017 22:08, Radim Krčmář wrote: > Doing exit from a subshell won't exit the main shell, so arch-run will > try to continue without a working QEMU if search_qemu_binary() failed. Should it be "exit $?"? Apart from this looks good. I can do the change. Paolo > Also fix error message from search_qemu_binary(). > > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> > --- > I was thinking about using `set -e` instead, so the "return 2" from > search_qemu_binary() would stop the script without the explicit exit. > It required just four changes to make the script work (resulting in > better diffstat), but would likely complicate future development. > > The second alternative would be to call just "search_qemu_binary" and > set the qemu variable from it. This would allow "exit 2" from > search_qemu_binary() to also exit arch/run, but side-effects on > variables seem worse than the first alternative ... > ---
2017-06-28 22:46+0200, Paolo Bonzini: > On 28/06/2017 22:08, Radim Krčmář wrote: > > Doing exit from a subshell won't exit the main shell, so arch-run will > > try to continue without a working QEMU if search_qemu_binary() failed. > > Should it be "exit $?"? Makes sense. I was thinking about 'return 1' from search_qemu_binary(), but never actually went from it. > Apart from this looks good. I can do the change. Please do, thanks.
diff --git a/arm/run b/arm/run index 8d8918a461dc..d7d778fe1645 100755 --- a/arm/run +++ b/arm/run @@ -31,7 +31,8 @@ if [ -z "$ACCEL" ]; then fi fi -qemu=$(search_qemu_binary) +qemu=$(search_qemu_binary) || + exit 2 if ! $qemu -machine '?' 2>&1 | grep 'ARM Virtual Machine' > /dev/null; then echo "$qemu doesn't support mach-virt ('-machine virt'). Exiting." diff --git a/powerpc/run b/powerpc/run index 3f69386c642a..e43c9fd8063c 100755 --- a/powerpc/run +++ b/powerpc/run @@ -28,7 +28,8 @@ if [ -z "$ACCEL" ]; then fi fi -qemu=$(search_qemu_binary) +qemu=$(search_qemu_binary) || + exit 2 if ! $qemu -machine '?' 2>&1 | grep 'pseries' > /dev/null; then echo "$qemu doesn't support pSeries ('-machine pseries'). Exiting." diff --git a/s390x/run b/s390x/run index 6df348a93783..ce8d45c2c8fd 100755 --- a/s390x/run +++ b/s390x/run @@ -28,7 +28,8 @@ if [ -z "$ACCEL" ]; then fi fi -qemu=$(search_qemu_binary) +qemu=$(search_qemu_binary) || + exit 2 M='-machine s390-ccw-virtio' M+=",accel=$ACCEL" diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index 6e429a8748f4..994c1aa9c0cd 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -176,9 +176,9 @@ search_qemu_binary () done if [ -z "$qemu" ]; then - echo "A QEMU binary was not found." - echo "You can set a custom location by using the QEMU=<path> environment variable." - exit 2 + echo "A QEMU binary was not found." >&2 + echo "You can set a custom location by using the QEMU=<path> environment variable." >&2 + return 2 fi command -v $qemu export PATH=$save_path diff --git a/x86/run b/x86/run index a06a7c0f80c8..1099c5ccd609 100755 --- a/x86/run +++ b/x86/run @@ -9,7 +9,8 @@ if [ -z "$STANDALONE" ]; then source scripts/arch-run.bash fi -qemu=$(search_qemu_binary) +qemu=$(search_qemu_binary) || + exit 2 if ! ${qemu} -device '?' 2>&1 | grep -F -e \"testdev\" -e \"pc-testdev\" > /dev/null; then
Doing exit from a subshell won't exit the main shell, so arch-run will try to continue without a working QEMU if search_qemu_binary() failed. Also fix error message from search_qemu_binary(). Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> --- I was thinking about using `set -e` instead, so the "return 2" from search_qemu_binary() would stop the script without the explicit exit. It required just four changes to make the script work (resulting in better diffstat), but would likely complicate future development. The second alternative would be to call just "search_qemu_binary" and set the qemu variable from it. This would allow "exit 2" from search_qemu_binary() to also exit arch/run, but side-effects on variables seem worse than the first alternative ... --- arm/run | 3 ++- powerpc/run | 3 ++- s390x/run | 3 ++- scripts/arch-run.bash | 6 +++--- x86/run | 3 ++- 5 files changed, 11 insertions(+), 7 deletions(-)