[libvirt] [PATCH] tools: avoid text spilling into variables

Dariusz Gadomski posted 1 patch 6 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20180116150526.7655-1-dariusz.gadomski@canonical.com
tools/libvirt-guests.sh.in | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
[libvirt] [PATCH] tools: avoid text spilling into variables
Posted by Dariusz Gadomski 6 years, 2 months ago
From: Christian Ehrhardt <christian.ehrhardt@canonical.com>

While libvirt-guests.sh is running cases can let guest_is_on fail which
causes check_guests_shutdown to print output.
That output shall not spill into the users of function
check_guests_shutdown which is therefore now returning values in a
variable like guest_is_on already did.

Original-Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Modified-By: Jorge Niedbalski <niedbalski@ubuntu.com>
---
 tools/libvirt-guests.sh.in | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
index 8a158cca4..91a2f3283 100644
--- a/tools/libvirt-guests.sh.in
+++ b/tools/libvirt-guests.sh.in
@@ -329,12 +329,13 @@ guest_count()
 # check_guests_shutdown URI GUESTS
 # check if shutdown is complete on guests in "GUESTS" and returns only
 # guests that are still shutting down
+# Result is returned in "guests_shutting_down"
 check_guests_shutdown()
 {
     uri=$1
     guests=$2
 
-    guests_up=
+    guests_shutting_down=
     for guest in $guests; do
         if ! guest_is_on "$uri" "$guest" >/dev/null 2>&1; then
             eval_gettext "Failed to determine state of guest: \$guest. Not tracking it anymore."
@@ -342,10 +343,9 @@ check_guests_shutdown()
             continue
         fi
         if "$guest_running"; then
-            guests_up="$guests_up $guest"
+            guests_shutting_down="$guests_shutting_down $guest"
         fi
     done
-    echo "$guests_up"
 }
 
 # print_guests_shutdown URI BEFORE AFTER
@@ -392,8 +392,10 @@ shutdown_guests_parallel()
             guest=$1
             shift
             guests=$*
-            shutdown_guest_async "$uri" "$guest"
-            on_shutdown="$on_shutdown $guest"
+            if [ -z "$(echo $on_shutdown | grep $guest)" -a -n "$(guest_name "$uri" "$guest")" ]; then
+                shutdown_guest_async "$uri" "$guest"
+                on_shutdown="$on_shutdown $guest"
+            fi
         done
         sleep 1
 
@@ -420,7 +422,8 @@ shutdown_guests_parallel()
         fi
 
         on_shutdown_prev=$on_shutdown
-        on_shutdown=$(check_guests_shutdown "$uri" "$on_shutdown")
+        check_guests_shutdown "$uri" "$on_shutdown"
+        on_shutdown="$guests_shutting_down"
         print_guests_shutdown "$uri" "$on_shutdown_prev" "$on_shutdown"
     done
 }
-- 
2.14.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] tools: avoid text spilling into variables
Posted by Michal Privoznik 6 years, 1 month ago
On 01/16/2018 04:05 PM, Dariusz Gadomski wrote:
> From: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> 
> While libvirt-guests.sh is running cases can let guest_is_on fail which
> causes check_guests_shutdown to print output.
> That output shall not spill into the users of function
> check_guests_shutdown which is therefore now returning values in a
> variable like guest_is_on already did.
> 
> Original-Author: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Modified-By: Jorge Niedbalski <niedbalski@ubuntu.com>
> ---
>  tools/libvirt-guests.sh.in | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in
> index 8a158cca4..91a2f3283 100644
> --- a/tools/libvirt-guests.sh.in
> +++ b/tools/libvirt-guests.sh.in
> @@ -329,12 +329,13 @@ guest_count()
>  # check_guests_shutdown URI GUESTS
>  # check if shutdown is complete on guests in "GUESTS" and returns only
>  # guests that are still shutting down
> +# Result is returned in "guests_shutting_down"
>  check_guests_shutdown()
>  {
>      uri=$1
>      guests=$2
>  
> -    guests_up=
> +    guests_shutting_down=
>      for guest in $guests; do
>          if ! guest_is_on "$uri" "$guest" >/dev/null 2>&1; then
>              eval_gettext "Failed to determine state of guest: \$guest. Not tracking it anymore."
> @@ -342,10 +343,9 @@ check_guests_shutdown()
>              continue
>          fi
>          if "$guest_running"; then
> -            guests_up="$guests_up $guest"
> +            guests_shutting_down="$guests_shutting_down $guest"
>          fi
>      done
> -    echo "$guests_up"
>  }
>  
>  # print_guests_shutdown URI BEFORE AFTER
> @@ -392,8 +392,10 @@ shutdown_guests_parallel()
>              guest=$1
>              shift
>              guests=$*
> -            shutdown_guest_async "$uri" "$guest"
> -            on_shutdown="$on_shutdown $guest"
> +            if [ -z "$(echo $on_shutdown | grep $guest)" -a -n "$(guest_name "$uri" "$guest")" ]; then

We prefer if test cond1 && test cond2 over -a.

I've fixed that, ACKed and pushed.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list