[libvirt] [PATCH] tools: fix variable scope in in check_guests_shutdown
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." echo continue -- 2.7.4
On 03/19/2018 12:44 PM, 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(-)
Oops, yes. ACKed and pushed. Michal
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 =)
echo continue -- 2.7.4
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On Thu, Mar 22, 2018 at 5:26 PM, Martin Kletzander <mkletzan@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@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@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- Christian Ehrhardt Software Engineer, Ubuntu Server Canonical Ltd
participants (3)
-
Christian Ehrhardt -
Martin Kletzander -
Michal Privoznik