[libvirt] [PATCH] tools: fix variable scope in in check_guests_shutdown

[Christian Ehrhardt]

On Thu, Mar 22, 2018 at 5:26 PM, Martin Kletzander <mkletzan at redhat.com>
wrote:

> On Mon, Mar 19, 2018 at 12:44:31PM +0100, Christian Ehrhardt wrote:
>
>> libvirt-guests.sh when run with more active guests than requested to
>> shut down in parallel will run until it times out only shutting down
>> the first set of guests.
>>
>> This patch fixes parallel shutdown by fixing a variable scope issue
>> where check_guests_shutdown unintentionally reset $guests which
>> prevented further progress.
>>
>> Fixes: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1688508
>>
>> Signed-off-by: Christian Ehrhardt <christian.ehrhardt at canonical.com>
>> ---
>> tools/libvirt-guests.sh.in | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
>> index d5e68e5..fcada31 100644
>> --- a/tools/libvirt-guests.sh.in
>> +++ b/tools/libvirt-guests.sh.in
>> @@ -333,11 +333,11 @@ guest_count()
>> check_guests_shutdown()
>> {
>>     uri=$1
>> -    guests=$2
>> +    guests_to_check=$2
>>
>>     guests_shutting_down=
>>     for guest in $guests; do
>> -        if ! guest_is_on "$uri" "$guest" >/dev/null 2>&1; then
>> +        if ! guest_is_on "$uri" "$guests_to_check" >/dev/null 2>&1; then
>>             eval_gettext "Failed to determine state of guest: \$guest.
>> Not tracking it anymore."
>>
>
> So I wanted to also send a patch for this                        ^^^^^^
> to be adjusted and realized it's not needed as that's a different
> variable.
>
> However, I also realized we're not using 'local' anywhere.  We should
> probably do that (maybe instead of renames in the future), so I'll add
> it to the BiteSizedTasks, I got to the half of the file and found out it
> can't be done with a regexp replace =)
>

Hi Martin,
I agree that a general overhaul would be nice, but at least I couldn't
affort the time this week to do so (away next week).
I guess you hit the fact that some functions intentionally do so with your
regexp :-)
Also I was unsure about different shell implementations in doing some of
the changes, so I didn't want to do a bigger change in a rush.

Thanks for adding it to the BiteSizedTasks - I think that is just the right
thing to do.


>             echo
>>             continue
>> --
>> 2.7.4
>>
>> --
>> libvir-list mailing list
>> libvir-list at redhat.com
>> https://www.redhat.com/mailman/listinfo/libvir-list
>>
>


-- 
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20180323/b6976284/attachment-0001.htm>