Skip to content

Commit

Permalink
test: Fix libvirt resource cleanup
Browse files Browse the repository at this point in the history
As `addCleanup` handlers are called in reverse order, the resource
cleanup happened in the wrong order: It first shut down network, then
pools, then machines. This often led to "busy" errors if e. g. a domain
was using an NFS pool that we tried to remove.

To avoid the confusion, bundle all these calls in a single `stop_all()`
helper which we can write in "natural order". Also avoid the `for` loop,
which just made things unnecessarily complicated and caused qemu:///system
resources to get cleaned up twice.

We need to be a little creative with cleaning up running domains.
TestMachinesVirtualization.testLibvirt sometimes leaves its VM in a
shutting down state, so that it goes away during the iteration.
Intercept and accept a "no such domain" error on destroy.

This fixes `TestMachinesDisks.testAddDiskNFS`, whose failure was
previously hidden due to `|| true`.
  • Loading branch information
martinpitt authored and jelly committed Feb 4, 2024
1 parent dd6d58f commit ae6eaab
Showing 1 changed file with 27 additions and 29 deletions.
56 changes: 27 additions & 29 deletions test/machineslib.py
Original file line number Diff line number Diff line change
Expand Up @@ -370,35 +370,33 @@ def setUp(self):
if not hasMonolithicDaemon(m.image):
self.addCleanup(m.execute, "systemctl stop virtstoraged.service virtnetworkd.service")

# Stop all domains
for connection in ["system", "session"]:
cmd = f"for d in $(virsh -c qemu:///{connection} list --name); do virsh -c qemu:///{connection} destroy $d; done"
if connection == "session":
cmd += f"; for d in $(virsh -c qemu:///{connection} list --all --name); do virsh -c qemu:///{connection} undefine $d; done"
cmd = f"runuser -l admin -c '{cmd}'"
self.addCleanup(m.execute, cmd)

# Cleanup pools
self.addCleanup(m.execute, "rm -rf /run/libvirt/storage/*")

# Stop all pools
for connection in ["system", "session"]:
cmd = f"for n in $(virsh -c qemu:///{connection} pool-list --name); do virsh -c qemu:///{connection} pool-destroy $n; done"
if connection == "session":
cmd += f"; for d in $(virsh -c qemu:///{connection} pool-list --all --name); do virsh -c qemu:///{connection} pool-undefine $d; done"
cmd = f"runuser -l admin -c '{cmd}'"
self.addCleanup(m.execute, cmd)

# Cleanup networks
self.addCleanup(m.execute, "rm -rf /run/libvirt/network/test_network*")

# Stop all networks
for connection in ["system", "session"]:
cmd = f"for n in $(virsh -c qemu:///{connection} net-list --name); do virsh -c qemu:///{connection} net-destroy $n; done"
if connection == "session":
cmd += f"; for d in $(virsh -c qemu:///{connection} net-list --all --name); do virsh -c qemu:///{connection} net-undefine $d; done"
cmd = f"runuser -l admin -c '{cmd}'"
self.addCleanup(m.execute, cmd)
def stop_all():
# domains
# this is a race condition: a test may leave a domain shutting down, so it may go away while iterating
m.execute('for d in $(virsh -c qemu:///system list --name); do '
' if ! out=$(virsh -c qemu:///system destroy $d 2>&1); then '
' echo "$out" | grep -q "domain is not running"; '
" fi; done")
m.execute("runuser -l admin -c 'for d in $(virsh -c qemu:///session list --all --name); do "
"virsh -c qemu:///session undefine $d; done'")

# pools
m.execute("rm -rf /run/libvirt/storage/*")
m.execute("for n in $(virsh -c qemu:///system pool-list --name); do virsh -c qemu:///system pool-destroy $n; done")
m.execute("runuser -l admin -c 'for d in $(virsh -c qemu:///session pool-list --name); do "
"virsh -c qemu:///session pool-destroy $d; done'")
m.execute("runuser -l admin -c 'for d in $(virsh -c qemu:///session pool-list --name --all); do "
"virsh -c qemu:///session pool-undefine $d; done'")

# networks
m.execute("rm -rf /run/libvirt/network/test_network*")
m.execute("for n in $(virsh -c qemu:///system net-list --name); do virsh -c qemu:///system net-destroy $n; done")
m.execute("runuser -l admin -c 'for d in $(virsh -c qemu:///session net-list --name); do "
"virsh -c qemu:///session net-destroy $d; done'")
m.execute("runuser -l admin -c 'for d in $(virsh -c qemu:///session net-list --name --all); do "
"virsh -c qemu:///session net-undefine $d; done'")

self.addCleanup(stop_all)

# we don't have configuration to open the firewall for local libvirt machines, so just stop firewalld
if not hasMonolithicDaemon(m.image):
Expand Down

0 comments on commit ae6eaab

Please sign in to comment.