Re: [Qemu-devel] [RFC PATCH RESEND 1/2] scripts: helper to keep console alive

[Eric Blake]

On 07/13/2017 05:03 AM, Philippe Mathieu-Daudé wrote:
> useful to avoid Travis CI jobs getting killed after 10min of inactivity.
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  MAINTAINERS          |  1 +
>  scripts/aliveness.sh | 32 ++++++++++++++++++++++++++++++++
>  2 files changed, 33 insertions(+)
>  create mode 100755 scripts/aliveness.sh
> 

> +++ b/scripts/aliveness.sh
> @@ -0,0 +1,32 @@
> +#! /usr/bin/env sh

Overkill.  /bin/sh exists everywhere, the only need for using 'env' is
where a program does not have a reliable installation path across platforms.

> +#
> +# Send an "Alive!" message regularly to stderr
> +#
> +# Copyright (C) 2017 Philippe Mathieu-Daudé
> +#
> +# Author: Philippe Mathieu-Daudé <address@hidden>
> +#
> +# This work is licensed under the terms of the GNU LGPL, version 2+.
> +# See the COPYING file in the top-level directory.
> +
> +TIMEOUT_S=$1
> +shift 1
> +{
> +  set -e
> +  while true
> +  do
> +    sleep ${TIMEOUT_S}
> +    echo "Alive!" >&2
> +  done
> +} &
> +WATCHDOG_PID=$!
> +
> +cleanup() {
> +    echo "killing watchdog ${WATCHDOG_PID}" >&2
> +    kill -TERM ${WATCHDOG_PID}
> +    exit $((1 - $#))

This looks odd - what are you really trying to accomplish here?  A
different exit status according to whether the watchdog exited normally
or was killed?  A reflection of the exit status of the command being run
in parallel with the watchdog?

> +}
> +trap cleanup INT

Don't you want to clean up on more than just INT?  The more typical
spelling is:

trap cleanup 0 1 2 3 15

> +
> +$*

Huh? Oh, you're expecting this to be called as "aliveness.sh $TIMEOUT
$command to run".  Some usage comments would be nice.

> +cleanup 0
> 

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

signature.asc
Description: OpenPGP digital signature