On 02/28/2012 11:00 AM, Peter Krempa wrote:
> With this patch, it's possible to shut down guests in parallel. Parallel
> startup was possible before, but this functionality was not documented
> properly.
>
> To enable parallel startup set the START_DELAY to 0.
>
> Parallel shutdown has a configurable parameter PARALLEL_SHUTDOWN that
> defines the number of machines being shut down in parallel. Enabling
> this feature changes the semantics of SHUTDOWN_TIMEOUT parameter that is
> applied as a cumulative timeout to shutdown all guests on a URI.
> ---
> tools/libvirt-guests.init.sh | 140 +++++++++++++++++++++++++++++++++++++++--
> tools/libvirt-guests.sysconf | 10 +++-
> 2 files changed, 141 insertions(+), 9 deletions(-)
>
Missing an initialization of PARALLEL_SHUTDOWN=0 alongside
SHUTDOWN_TIMEOUT=0 before sourcing the user's values.
> +
> +# check_domains_shutdown URI GUESTS
> +# check if shutdown is complete on guests in "GUESTS" and returns only
> +# guests that are still shutting down
> +check_domains_shutdown()
> +{
> + uri=$1
> + guests=$2
> +
> + guests_up=
> + for guest in $guests; do
> + guest_is_on "$uri" "$dom" 2>&1 > /dev/null || continue
You want '>/dev/null 2>&1' (order matters, when silencing a command and
using it just for its side effects). But are we sure we want to
silently ignore a guest_is_on failure, or should we emit a message
stating that we are no longer able to track the shutdown status of that
guest?
> +print_domains_shutdown()
> +{
> + uri=$1
> + before=$2
> + after=$3
> +
> + for guest in $before; do
> + found=false
> + for running in $after; do
> + if [ $guest = $running ]; then
> + found=true
> + break
> + fi
> + done
slightly faster to do:
for guest in $before; do
found=false;
case " $after " in
*" $guest "*) found=true ;;
esac
But what you have is functionally correct.
> +
> + if ! "$found"; then
> + name=$(guest_name "$uri" "$guest")
> + eval_gettext "Shutdown of guest \$name complete."
> + echo
> + fi
> + done
> +}
> +
> +# shutdown_guests_parallel URI GUESTS
> +# Shutdown guests GUESTS on machine URI in parallel
> +shutdown_guests_parallel()
> +{
> + uri=$1
> + guests=$2
> +
> + on_shutdown=
> + timeout=$SHUTDOWN_TIMEOUT
If SHUTDOWN_TIMEOUT is the default of 0, then this function times out
right away; shouldn't that really mean that we have no maximum timeout,
and wait until everything has shut down? I think you need another
variable, based on whether $timeout is 0 on entry, and skip the
$((timeout - 1)) calculation if that variable is true.
> + while [ -n "$on_shutdown" ] || [ -n "$guests" ]; do
> + while [ -n "$guests" ] &&
> + [ $(guest_count "$on_shutdown") -lt "$PARALLEL_SHUTDOWN" ]; do
> + guest=$(guest_first $guests)
> + guests=$(guest_remove "$guest" "$guests")
Faster to avoid these two sub-shells by doing:
set -- $guests
guest=$1
shift
guests=$*
and might even get rid of the need for some helper functions. But what
you have is functionally correct.
> @@ -23,7 +24,12 @@
> # value suitable for your guests.
> #ON_SHUTDOWN=suspend
>
> -# number of seconds we're willing to wait for a guest to shut down
> +# If set to non-zero, shutdown will suspend domains concurently. Number of domains
s/concurently/concurrently/
I think it's probably worth a v2 (or is it v3?) for just this patch.
--
Eric Blake eblake@xxxxxxxxxx +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list