[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] libvirt-guests: Enable parallel operations and improve error handling



On 02/20/2012 06:32 AM, Peter Krempa wrote:
> This patch modifies the libvirt-guests init script to enable parallel
> shiutdown on guest machines and gets rid of error messages if the client
> is unable connect to the URI specified in the config file.

Resuming where I left off (and seeing as how you took my suggestions for
improving 'virsh list', we'll need a v2 anyway),

> +        eval_gettext "Running guests on \$uri URI: "
> 
> -        list=$(list_guests "$uri")
> +        list=$(list_guests "$uri" "all")

Yeah, I guess it makes sense to list all running guests, even the
transient ones that we can't save, if only for log purposes to show that
we knew we were losing the transient ones.

Hmm, I just remembered, a while ago there was a PoC patch to allow
migration on shutdown, as an alternative to managed-save - and that
would work for transient guests!  We need to look at reviving that
patch, and figuring out how it would interact with this patch.

https://www.redhat.com/archives/libvir-list/2011-November/msg00229.html

> 
> @@ -292,13 +444,39 @@ stop() {
>              eval_gettext "Shutting down guests on \$uri URI..."; echo
>          fi
> 
> -        for guest in $list; do
> -            if "$suspending"; then
> -                suspend_guest "$uri" "$guest"
> -            else
> -                shutdown_guest "$uri" "$guest"
> -            fi
> -        done
> +        if [ "$PARALLEL_SHUTDOWN" -gt 1 ] &&
> +           ! "$suspending"; then
> +            on_shutdown=
> +            timeout=$SHUTDOWN_TIMEOUT
> +            while [ $(set_count "$on_shutdown") -gt "0" ] ||
> +                  [ $(set_count "$list") -gt "0" ]; do

$(set_count ...) forks, but if all you are doing is checking for a
non-empty set (count -gt 0), then it is faster to do:

while [ -n "$on_shutdown" ] || [ -n "$list" ]; do

> +                while [ $(set_count "$on_shutdown") -lt "$PARALLEL_SHUTDOWN" ] &&

whereas this use of set_count really is needed.

> +                      [ $(set_count "$list") -gt "0" ]; do
> +                    domain=$(set_head "$list")
> +                    shutdown_guest_async "$uri" "$domain"
> +                    on_shutdown=$(set_add "$domain" "$on_shutdown");
> +                    list=$(set_remove "$domain" "$list");
> +                done
> +                sleep 1
> +                timeout=$((timeout - 1))

My earlier patch mentioned that it might be slightly more portable to
use $(($timeout - 1)) (that is, $timeout instead of timeout); but in
looking further, I see that this is just copy-paste from existing code,
and no one has complained about it not working on dash, so no worries
about it after all.

The parallel shutdown looks reasonable, but it's a lot of code to be
embedding into the stop function; maybe it's worth factoring into a
shutdown_guest_parallel function?

> +++ b/tools/libvirt-guests.sysconf
> @@ -10,7 +10,8 @@
>  #           libvirtd
>  #ON_BOOT=start
> 
> -# number of seconds to wait between each guest start
> +# number of seconds to wait between each guest start. Set to 0 to allow parallel
> +# startup.
>  #START_DELAY=0
> 
>  # action taken on host shutdown
> @@ -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/

> +# on shutdown at any time will not exceed number set in this variable.
> +#PARALLEL_SHUTDOWN=0
> +
> +# number of seconds we're willing to wait for a guest to shut down. If parallel
> +# shutdown is enabled, this timeout applies as a timeout for shutting down all guests.
>  #SHUTDOWN_TIMEOUT=0
> 
>  # If non-zero, try to bypass the file system cache when saving and

Looking forward to v2.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]