Bug 1335585 - libvirt-guests: filter out xen domain-0
Summary: libvirt-guests: filter out xen domain-0
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Virtualization Tools
Classification: Community
Component: libvirt
Version: unspecified
Hardware: Unspecified
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Libvirt Maintainers
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2016-05-12 14:50 UTC by Chris Hoefler
Modified: 2016-10-10 17:24 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-10-10 17:24:56 UTC
Embargoed:


Attachments (Terms of Use)
patch for /usr/lib/libvirt/libvirt-guests.sh (326 bytes, patch)
2016-05-12 14:50 UTC, Chris Hoefler
no flags Details | Diff

Description Chris Hoefler 2016-05-12 14:50:48 UTC
Created attachment 1156707 [details]
patch for /usr/lib/libvirt/libvirt-guests.sh

When executing libvirt-guests.sh stop manually or from the systemd service, a single guest on a host can be gracefully shut down, but the remaining guest(s) are not shut down (they end up forcefully killed by the reboot).

After digging around, a discovered the problem has to do with the way the libvirt-guests.sh script enumerates guests. The list_guests() function discovers running guests by executing "virsh list --uuid", which returns a a list of running guest uuids separated by newlines.

> virsh list --uuid
be82edbf-c42b-5aff-47a6-510f3694e338
ec326c8d-1f20-42ab-8ace-8825e7636994

These uuids are captured in a string, and later in the script stop() function, written to the file /var/lib/libvirt/libvirt-guests. But the newlines aren't removed, so that file looks like this:

> cat /var/lib/libvirt/libvirt-guests
default be82edbf-c42b-5aff-47a6-510f3694e338
ec326c8d-1f20-42ab-8ace-8825e7636994

Note the newline between the uuids that belong to the default URI.

When this file is read back to start shutting down guests, this loop,
  while read uri list; do

only grabs the first uuid to shutdown in the first iteration. The second uuid is picked up in the next loop iteration, but because it isn't preceded by a URI, it doesn't execute the graceful shutdown.

The list_guests() function could be fixed to output the uuids as a space (instead of newline) delimited string, but not knowing how that would affect other parts of the script, I opted to just edit the $list string right before it gets written to /var/lib/libvirt/libvirt-guests.
  list=$(echo $list | paste -s -d \s)

I've attached a patch to this bugzilla.

Comment 1 Cole Robinson 2016-05-15 23:09:10 UTC
Thanks for the patch, but I can't reproduce the issue in my hacked up testing. 

What libvirt version are you using?
What host distro is this?
Do you know what the default shell on your system is?

Comment 2 Chris Hoefler 2016-05-17 15:04:23 UTC
(In reply to Cole Robinson from comment #1)
> Thanks for the patch, but I can't reproduce the issue in my hacked up
> testing. 
> 
> What libvirt version are you using?
> What host distro is this?
> Do you know what the default shell on your system is?

Ubuntu 16.04 LTS
libvirt 1.3.1
/bin/sh is symlinked to /bin/dash

Comment 3 Cole Robinson 2016-05-17 15:36:27 UTC
Does using /bin/bash reproduce the problem? (without your patch)

That's obviously not a solution, I'm just trying to determine if this is a shell compatibility/why I can't reproduce

Comment 4 Chris Hoefler 2016-05-18 04:04:13 UTC
A good thought because dash does occasionally have compatibility issues with bash. But no, I tried by changing the shebang to #!/bin/bash, and also by explicitly invoking the bash shell with source. I get the same problem in both cases.

However, I think you are on to something. I noticed the list_guests() function has,
  echo "$list" | grep -v 00000000-0000-0000-0000-000000000000

as the last line. The quotes around $list inhibit word splitting, so the newlines between uuids don't get interpreted as delimiters. I removed the quotes and now the script works with both bash and dash as expected. The weird part is that word splitting *should* have happened at the original command substitution,
  list=$(run_virsh_c "$uri" list --uuid $persistent)

but, for some reason, it didn't. It gets a seconds chance at the echo statement, but only if the quotes are absent.

Maybe your shell has different IFS behavior?

I'm running,
  GNU bash, version 4.3.42(1)-release (x86_64-pc-linux-gnu)
and,
  dash, 0.5.8-2.1ubuntu2

Comment 5 Cole Robinson 2016-05-19 14:07:39 UTC
Sigh, that change to grep out 0000... (xen domain-0) is an ubuntu patch which hasn't been submitted upstream. Search for 'libvirt-guests' here:

https://launchpad.net/ubuntu/xenial/+source/libvirt/+changelog

The patch is by Stefan Bader. So please file a bug against ubuntu, and ask him to submit his patches upstream next time

Repurposing this bug to track upstreaming some kind of patch to skip domain-0 in libvirt-guests

Comment 6 Cole Robinson 2016-10-10 17:24:56 UTC
Stefan's fixed patches are upstream now

commit cc38d5661ff11cbe749fa8932e74300f406a7780
Author: Stefan Bader <stefan.bader>
Date:   Fri Oct 7 09:56:33 2016 +0200

    tools: Exclude Xen dom0 from libvirt-guests.sh list
    
    With newer versions of libvirt Domain-0 is again visible in the list of
    running guests but it should not be considered as a guest for shutdown
    or suspend.
    
    Signed-off-by Stefan Bader <stefan.bader>

commit 69722fd7ac854f95eac65a4bf3fd86e65fbc470f
Author: Stefan Bader <stefan.bader>
Date:   Fri Oct 7 09:56:32 2016 +0200

    tools: Ignore newlines in libvirt-guests.sh guest list
    
    The list file expects all guest UUIDs on the same line as the URI
    which the guests run on. This does not happen when the list is
    echo'ed in quotes. When stripping the quotes, newlines get transformed
    into spaces. Without this, only the first guest on the list is actually
    handled.
    
    Based on a fix by Omar Siam <simar>
    
    Bug-Ubuntu: http://bugs.launchpad.net/bugs/1591695
    
    Signed-off-by: Stefan Bader <stefan.bader>


Note You need to log in before you can comment on or make changes to this bug.