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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- 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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2024 Red Hat, Inc.