Bug 1450141

Summary: libvirt-guests.sh fails to shutdown guests in parallel
Product: [Community] Virtualization Tools Reporter: christoph.wolff
Component: libvirtAssignee: Libvirt Maintainers <libvirt-maint>
Status: CLOSED NEXTRELEASE QA Contact:
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: unspecifiedCC: crobinso, dariusz.gadomski, fabio.bardella, jorge.niedbalski, libvirt-maint, pkrempa, rbalakri
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-02-23 13:30:55 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Attachments:
Description Flags
Proposed patch for libvirt-guests.sh
none
check guest shutdown function extended to don't consider guests already on the on_shutdown list. none

Description christoph.wolff 2017-05-11 15:55:14 UTC
Description of problem:

On a brand new installation of Ubuntu Server 16.04.2 LTS the libvirt-guests.sh script fails to shutdown my VMs if run with PARALLEL_SHUTDOWN > 0, instead running forever thereby preventing me from shutting down my system. I debugged the script with 'set -x' and found out that the problem lies in the `check_guests_shutdown()` function (line 337). If a guest is shutdown, it runs eval_gettext() to print that the state of the guest cannot be determined. This is used as a return value on line 428, causing the script to interpret the error message as a list of VM IDs and trying to shutdown these. This of course fails again and the cycle continues. For now I have commented out the eval_gettext line and the script works fine again.

I have reported the bug on the Ubuntu bug tracker already (see the external bug [#1688508]), but since the script is the same as upstream I was told to report it here.

Also after some discussion there I came to the conclusion that the problem might have to do with my VMs being transient, causing the check about the VM status to fail. You can read more about it in that thread.

Version-Release number of selected component (if applicable): 1.3.1 (I know it's old, but the script is largely the same as the one in master)


How reproducible:


Steps to Reproduce:
1. Create transient VMs
2. Use `libvirt-guests.sh stop` with PARALLEL_SHUTDOWN > 0

Actual results:

The script loops indefinitely.

Expected results:
The VMs are shut down and the script exits normally.

Additional info:

I am not using libvirt by itself but with OpenNebula, which is basically a fancy frontend for KVM.

Comment 1 Peter Krempa 2017-05-11 16:19:20 UTC
Hmm, you are right. The message was supposed to be an error message and should not go into the list of VMs for shutdown. I'll try to fix it since I wrote that feature some time ago.

Comment 2 Fabio Bardella 2017-10-17 10:17:36 UTC
Hi! We're also experiencing this issue (see: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1688508/). There's a proposed patch there, I'm attaching it to this bug report. 

Is this something that will being fixed anytime soon ? Thanks

Comment 3 Fabio Bardella 2017-10-17 10:20:32 UTC
Created attachment 1339646 [details]
Proposed patch for libvirt-guests.sh

From: https://bugs.launchpad.net/ubuntu/+source/libvirt/+bug/1688508/

Comment 4 Jorge Niedbalski 2017-12-07 20:46:26 UTC
Created attachment 1364464 [details]
check guest shutdown function extended to don't consider guests already on the on_shutdown list.

Hello,

I am attaching a new version of the patch proposed by Christian,
which extends the check_guest_shutdown function to execute the 
async shutdown call just in case the following condition is met (guest isn't already on the shutdown list).

+            if [ -z "$(echo $on_shutdown | grep $guest)" -a -n "$(guest_name "$uri" "$guest")" ]; 

I hope you can consider this new addition.

Comment 5 Cole Robinson 2017-12-07 20:55:21 UTC
Thanks for the patch, please send it to libvir-list though to give it more visibility, in git format-patch format with a nice commit message describing the problem

Comment 6 Dariusz Gadomski 2018-01-23 10:41:19 UTC
The patch has been shared in the list [1].

[1] https://www.redhat.com/archives/libvir-list/2018-January/msg00526.html

Comment 7 Peter Krempa 2018-02-23 13:30:55 UTC
The patch was pushed now so this should be fixed:

commit ff02d1af4041ea4cf6bea68aac66d93a970e92f1
Author: Christian Ehrhardt <christian.ehrhardt>
Date:   Tue Jan 16 16:05:26 2018 +0100

    tools: avoid text spilling into variables
    
    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>
    Modified-By: Jorge Niedbalski <niedbalski>
    Signed-off-by: Michal Privoznik <mprivozn>