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

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

 





On Thu, Mar 22, 2018 at 5:26 PM, Martin Kletzander <mkletzan@xxxxxxxxxx> 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@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@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



--
Christian Ehrhardt
Software Engineer, Ubuntu Server
Canonical Ltd
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux