On Sun, Mar 29, 2020 at 04:32:52AM +0530, Prathamesh Chavan wrote:
Redeclared variables in script functions marked as local. Variables `guest_running` and `guests_shutting_down` in the functions 'guest_is_on` and `check_guests_shutdown` were untouched, as the functions returned values in these variables. Signed-off-by: Prathamesh Chavan <pc44800@xxxxxxxxx> --- I'm submitting this as my BiteSizedTask for GSoC'20 I've cc'd Martin Kletzander, as he is mentor for this task, and to ensure that the needful was done by me.
Thanks for sending the patch.
After running `make check`, I received "PASS: 98 and SKIP: 7" as the result. Travis-ci build report can be found at [1]. For my project, 'Introducing job control to the storage driver', right now I'm going through the `/src/qemu/THREADS.txt` as well as previous work done by Tucker DiNapoli during GSoC'14, as well as the proposal of Taowei Luo for GSoC'15. Their email threads are surely helping me get a better picture of the project. Apart from this, please let me know if there is anything else, which could help me with this project. Thansk!
Discussing the project would be a good start. You're doing well that you're reading what was done before, but also make sure to check out what is happening lately. I'm only quickly looking through the code below, I have nowhere to test it currently and who knows when I even looked at some libvirt code lately =)
[1]: https://travis-ci.org/github/pratham-pc/libvirt/builds/668209157 tools/libvirt-guests.sh.in | 104 +++++++++++++++++++------------------ 1 file changed, 54 insertions(+), 50 deletions(-) diff --git a/tools/libvirt-guests.sh.in b/tools/libvirt-guests.sh.in index a881f6266e..90a18fee49 100644 --- a/tools/libvirt-guests.sh.in +++ b/tools/libvirt-guests.sh.in @@ -65,7 +65,7 @@ retval() { # If URI is "default" virsh is called without the "-c" argument # (using libvirt's default connection) run_virsh() { - uri=$1 + local uri=$1 shift if [ "x$uri" = xdefault ]; then @@ -86,7 +86,7 @@ run_virsh_c() { # check if URI is reachable test_connect() { - uri=$1 + local uri=$1 if run_virsh "$uri" connect 2>/dev/null; then return 0; @@ -103,8 +103,8 @@ test_connect() # --transient: list only transient guests # [none]: list both persistent and transient guests list_guests() { - uri=$1 - persistent=$2 + local uri=$1 + local persistent=$2 list=$(run_virsh_c "$uri" list --uuid $persistent)
I don't think this variable is supposed to be global.
if [ $? -ne 0 ]; then
@@ -118,8 +118,8 @@ list_guests() {
# guest_name URI UUID
# return name of guest UUID on URI
guest_name() {
- uri=$1
- uuid=$2
+ local uri=$1
+ local uuid=$2
run_virsh "$uri" domname "$uuid" 2>/dev/null
}
@@ -128,8 +128,8 @@ guest_name() {
# check if guest UUID on URI is running
# Result is returned by variable "guest_running"
guest_is_on() {
- uri=$1
- uuid=$2
+ local uri=$1
+ local uuid=$2
guest_running=false
id=$(run_virsh "$uri" domid "$uuid")
It seems you skipped some functions after the first empty line.
@@ -161,13 +161,13 @@ start() {
return 0
fi
- isfirst=true
- bypass=
- sync_time=false
+ local isfirst=true
+ local bypass=
+ local sync_time=false
test "x$BYPASS_CACHE" = x0 || bypass=--bypass-cache
test "x$SYNC_TIME" = x0 || sync_time=true
while read uri list; do
- configured=false
+ local configured=false
set -f
for confuri in $URIS; do
set +f
@@ -186,7 +186,7 @@ start() {
eval_gettext "Resuming guests on \$uri URI..."; echo
for guest in $list; do
- name=$(guest_name "$uri" "$guest")
+ local name=$(guest_name "$uri" "$guest")
It would be nice to have the info about the variable being local in one place, at the top of the function. Most of us are used to our old C ways here, so keeping this in a similar fashion could be nice. I understand that this is not an interesting task and it is also one of the easiest ones, but the idea for that was created when this actually solved a real bug. This will probably be a one-off since not much is happening in the script, but we could have a calmer sleep if the variables are marked properly. Have a nice day, Martin
Attachment:
signature.asc
Description: PGP signature
