mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 19:26:48 +00:00
fix(instances): deleting a VM's data takes its snapshot files too
Deleting a VM with its data deleted only the images its disks pointed at. With external snapshots that is the newest overlay: the image the snapshots were taken on, the older overlays and the memory files stayed on disk, with nothing referring to them once the VM and its snapshot metadata were gone. The files the VM's snapshots were taken on, down each disk's backing chain, the overlays of snapshots kept off the chain after a revert, and the snapshots' memory files are now the VM's own too. A template the VM grew from ends the chain and stays, as does an overlay of a disk detached since. The backing references of the VM's own volumes no longer count as another domain's use, and a file another VM uses keeps the files below it. A backing file that cannot be read (other than one in no pool) stops the split before anything is deleted, instead of ending the chain there.
This commit is contained in:
catborise
parent
ff4ffab382
commit
08cad04468
4 changed files with 186 additions and 6 deletions
|
|
@ -556,7 +556,7 @@ A rollback leaves the clone on a detached HEAD at the restored commit; the updat
|
|||
- gstfsd finds the guest's operating system the way libguestfs inspects it, so a root on LVM works, and it always answers (an error when it finds no or several operating systems). Update `/usr/local/bin/gstfsd` on every compute (see [Adding a Compute Node](#adding-a-compute-node), step 3).
|
||||
- A theme picked on the Settings page rewrites two tracked files; the update and rollback steps put back the shipped theme. Pick yours again afterwards, also after Docker recreates the container.
|
||||
- With the bundled nginx config, browsers revalidate static files on each load (304 when unchanged), so an update or a theme change shows without a hard reload. Files cached before this update can stay until a hard reload (Ctrl+F5) of each affected page, or until the site's browser cache is cleared once; an external caching proxy may need a purge.
|
||||
- External snapshots use libvirt's snapshot API and need libvirt 9.9 or newer on the compute hosts. They work on UEFI VMs and running, paused or shut-off VMs, can save memory too, and a revert keeps disks added later (it detaches them). A VM's snapshots form one chain: after a revert, delete the newer snapshots before taking another. Snapshot names no longer get an `s1.` prefix; existing ones keep theirs and can still be reverted and deleted, except that a snapshot an older version took of a UEFI VM can only be deleted. On NFS, libvirt removes merged snapshot files as the qemu user, who needs write access to the image directory; files it could not remove are named in a warning after the delete or revert.
|
||||
- External snapshots use libvirt's snapshot API and need libvirt 9.9 or newer on the compute hosts. They work on UEFI VMs and running, paused or shut-off VMs, can save memory too, and a revert keeps disks added later (it detaches them). A VM's snapshots form one chain: after a revert, delete the newer snapshots before taking another. Snapshot names no longer get an `s1.` prefix; existing ones keep theirs and can still be reverted and deleted, except that a snapshot an older version took of a UEFI VM can only be deleted. On NFS, libvirt removes merged snapshot files as the qemu user, who needs write access to the image directory; files it could not remove are named in a warning after the delete or revert. Deleting a VM with its data removes its snapshot files too, except a template it grew from and files another VM uses.
|
||||
- Docker no longer publishes port 6080; the console goes through nginx on the panel's port (`WS_PUBLIC_PORT`, default 80).
|
||||
|
||||
## Usage
|
||||
|
|
|
|||
|
|
@ -275,6 +275,52 @@ class LiveDataLossTestCase(TestCase):
|
|||
self.assertFalse(livetest.volume_exists(self.conn, overlay), "the VM's own image is still there")
|
||||
self.assertTrue(livetest.volume_exists(self.conn, base), "the base image was deleted")
|
||||
|
||||
def test_destroy_deletes_the_external_snapshot_chain_but_not_its_template(self):
|
||||
# The VM grew from a template; its snapshots added overlays above its
|
||||
# disk and saved memory. Deleting the disks takes all of those, which
|
||||
# nothing else uses once the VM and its snapshot metadata are gone.
|
||||
template = livetest.create_volume(self.conn, P + "r14-ext-template")
|
||||
pool = self.conn.storagePoolLookupByName(livetest.POOL)
|
||||
disk = pool.createXML(
|
||||
f"<volume><name>{P}r14-ext.qcow2</name><capacity unit='MiB'>64</capacity>"
|
||||
f"<target><format type='qcow2'/></target><backingStore><path>{template}</path>"
|
||||
"<format type='qcow2'/></backingStore></volume>",
|
||||
0,
|
||||
).path()
|
||||
dom, inst = self.vm("r14-ext", [disk])
|
||||
dom.create()
|
||||
inst.proxy.create_external_snapshot("a", memory=True)
|
||||
inst.proxy.create_external_snapshot("b")
|
||||
memory = os.path.join(livetest.POOL_PATH, f"{dom.UUIDString()}.a.mem")
|
||||
pool.refresh(0)
|
||||
made = {name for name in pool.listVolumes() if name.startswith(P + "r14-ext.")} - {P + "r14-ext.qcow2"}
|
||||
self.assertEqual(len(made), 2, made) # one overlay per snapshot
|
||||
|
||||
response = self.post("destroy", inst, {"delete_disk": "1"})
|
||||
|
||||
self.assertEqual(response.status_code, 302)
|
||||
self.assertFalse(self.domain_exists(dom.UUIDString()), "VM is still defined")
|
||||
pool.refresh(0)
|
||||
left = {name for name in pool.listVolumes() if name.startswith(P + "r14-ext") and "template" not in name}
|
||||
self.assertEqual(left, set(), "files of the VM or its snapshots stayed")
|
||||
self.assertFalse(livetest.volume_exists(self.conn, memory), "the memory file stayed")
|
||||
self.assertTrue(livetest.volume_exists(self.conn, template), "the template was deleted")
|
||||
self.assertEqual([str(m) for m in get_messages(response.wsgi_request) if "kept" in str(m)], [])
|
||||
|
||||
def test_destroy_after_a_revert_deletes_the_newer_snapshots_files_too(self):
|
||||
base = livetest.create_volume(self.conn, P + "r14-rev")
|
||||
dom, inst = self.vm("r14-rev", [base])
|
||||
inst.proxy.create_external_snapshot("a")
|
||||
inst.proxy.create_external_snapshot("b")
|
||||
inst.proxy.revert_external_snapshot("a") # b's overlay is off the disk's chain now
|
||||
|
||||
response = self.post("destroy", inst, {"delete_disk": "1"})
|
||||
|
||||
self.assertEqual(response.status_code, 302)
|
||||
pool = self.conn.storagePoolLookupByName(livetest.POOL)
|
||||
pool.refresh(0)
|
||||
self.assertEqual({name for name in pool.listVolumes() if name.startswith(P + "r14-rev")}, set())
|
||||
|
||||
# Delete a volume only after it is detached
|
||||
|
||||
def _delete_attached_volume(self, pause):
|
||||
|
|
|
|||
|
|
@ -1479,8 +1479,9 @@ class wvmInstance(wvmConnect):
|
|||
iso.extend(name for name in names if name.lower().endswith(".iso"))
|
||||
return iso
|
||||
|
||||
def paths_used_by_other_domains(self):
|
||||
"""Paths another domain uses as a disk or as a backing file of one."""
|
||||
def paths_used_by_other_domains(self, own=()):
|
||||
"""Paths another domain uses as a disk or as a backing file of one.
|
||||
The volumes in own are this VM's: their backing files do not count."""
|
||||
uuid = self.instance.UUIDString()
|
||||
used = set()
|
||||
for dom in self.wvm.listAllDomains():
|
||||
|
|
@ -1499,19 +1500,73 @@ class wvmInstance(wvmConnect):
|
|||
pool.refresh(0)
|
||||
for vol in pool.listAllVolumes():
|
||||
with contextlib.suppress(libvirtError):
|
||||
used.update(etree.fromstring(vol.XMLDesc(0)).xpath("./backingStore/path/text()"))
|
||||
if vol.path() not in own:
|
||||
used.update(etree.fromstring(vol.XMLDesc(0)).xpath("./backingStore/path/text()"))
|
||||
return used
|
||||
|
||||
def split_disk_paths_by_use(self):
|
||||
"""This VM's disk paths, split into (only this VM, also used by another domain)."""
|
||||
"""This VM's disk paths, split into (only this VM, also used by another
|
||||
domain). With external snapshots, the files below a disk that the
|
||||
snapshots were taken on, and their memory files, are the VM's too."""
|
||||
self.refresh_instance_pools()
|
||||
own = [disk["path"] for disk in self.get_disk_devices() if disk["path"]]
|
||||
used_elsewhere = self.paths_used_by_other_domains()
|
||||
own += self._snapshot_chain_files(own)
|
||||
used_elsewhere = self.paths_used_by_other_domains(own)
|
||||
# a kept file keeps the files below it
|
||||
for path in [path for path in own if path in used_elsewhere]:
|
||||
while (path := self._backing_path(path)) and path not in used_elsewhere:
|
||||
used_elsewhere.add(path)
|
||||
return (
|
||||
[path for path in own if path not in used_elsewhere],
|
||||
[path for path in own if path in used_elsewhere],
|
||||
)
|
||||
|
||||
def _snapshot_chain_files(self, tops):
|
||||
"""The files of this VM's external snapshots that these disk images
|
||||
grow from or that grow from them: the backing files snapshots were
|
||||
taken on, down the chain, the overlays of snapshots off the chain
|
||||
(newer ones kept after a revert), and the snapshots' memory files. A
|
||||
backing file the VM started from (a template) is no snapshot point
|
||||
and ends the chain; an overlay that does not grow from the chain (a
|
||||
disk detached since) stays."""
|
||||
points, overlays, memory = set(), set(), []
|
||||
for snap in self.instance.listAllSnapshots(VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL):
|
||||
tree = etree.fromstring(snap.getXMLDesc(0))
|
||||
memory += tree.xpath("memory/@file")
|
||||
overlays.update(tree.xpath("disks/disk/source/@file"))
|
||||
saved = self._saved_definition(tree)
|
||||
if saved is not None:
|
||||
points.update(self._disk_sources(saved).values())
|
||||
files = list(tops)
|
||||
for path in tops:
|
||||
while (path := self._backing_path(path)) in points and path not in files:
|
||||
files.append(path)
|
||||
grew = True
|
||||
while grew:
|
||||
grew = False
|
||||
for top in overlays.difference(files):
|
||||
if top in files: # reached by a branch in this pass
|
||||
continue
|
||||
branch, path = [top], top
|
||||
while (path := self._backing_path(path)) in points | overlays and path not in files:
|
||||
branch.append(path)
|
||||
if path in files:
|
||||
files += branch
|
||||
grew = True
|
||||
return files[len(tops):] + memory
|
||||
|
||||
def _backing_path(self, path):
|
||||
"""The backing file of a volume; None if it has none or is in no pool.
|
||||
Any other error is raised: a chain cut short would let a file a kept
|
||||
one needs count as deletable."""
|
||||
try:
|
||||
vol = self.get_volume_by_path(path)
|
||||
except libvirtError as err:
|
||||
if err.get_error_code() != VIR_ERR_NO_STORAGE_VOL:
|
||||
raise
|
||||
return None
|
||||
return util.get_xml_path(vol.XMLDesc(0), "/volume/backingStore/path")
|
||||
|
||||
def _snapshotCreateXML(self, xml, flag):
|
||||
self.instance.snapshotCreateXML(xml, flag)
|
||||
|
||||
|
|
|
|||
|
|
@ -551,3 +551,82 @@ class KindTestCase(unittest.TestCase):
|
|||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
|
||||
class DestroyFilesTestCase(unittest.TestCase):
|
||||
"""Deleting a VM with its disks takes the files its external snapshots
|
||||
made or were taken on, not a template below them or a file another VM
|
||||
still grows from."""
|
||||
|
||||
TOP = "<domain><devices><disk type='file' device='disk'><source file='/pool/a.two'/><target dev='vda'/></disk></devices></domain>"
|
||||
|
||||
def split(self, other_overlay_on=None, top="/pool/a.two", extra=None):
|
||||
one = chain_snap("one", "/pool/a.one", memory="/pool/vm.one.mem")
|
||||
two = chain_snap("two", "/pool/a.two")
|
||||
snaps = [(one, "/pool/a.qcow2"), (two, "/pool/a.one")] # each with the file it was taken on
|
||||
if extra:
|
||||
snaps.append((chain_snap("old", extra[0]), extra[1]))
|
||||
for snap, point in snaps:
|
||||
snap.getXMLDesc.return_value = snap.getXMLDesc.return_value.replace(
|
||||
"<devices/>", f"<devices><disk type='file' device='disk'><source file='{point}'/><target dev='vda'/></disk></devices>"
|
||||
)
|
||||
p = proxy(external=[snap for snap, _ in snaps], xml=self.TOP)
|
||||
p.get_disk_devices = lambda: [{"path": top}]
|
||||
backing = {
|
||||
"/pool/a.two": "/pool/a.one", "/pool/a.one": "/pool/a.qcow2", "/pool/a.qcow2": "/templates/base.qcow2",
|
||||
}
|
||||
if top == "/pool/a.reverted":
|
||||
backing[top] = "/pool/a.one"
|
||||
if extra:
|
||||
backing[extra[0]] = extra[1]
|
||||
if other_overlay_on:
|
||||
backing["/pool/other.qcow2"] = other_overlay_on
|
||||
|
||||
def volume(path):
|
||||
parent = f"<backingStore><path>{backing[path]}</path></backingStore>" if path in backing else ""
|
||||
vol = MagicMock(**{"XMLDesc.return_value": f"<volume>{parent}</volume>"})
|
||||
vol.path.return_value = path
|
||||
return vol
|
||||
|
||||
p.get_volume_by_path = volume
|
||||
pool = MagicMock(**{"isActive.return_value": True})
|
||||
pool.listAllVolumes.return_value = [volume(path) for path in backing]
|
||||
p.wvm.listAllStoragePools.return_value = [pool]
|
||||
p.wvm.listAllDomains.return_value = []
|
||||
return p.split_disk_paths_by_use()
|
||||
|
||||
def test_a_backing_file_that_cannot_be_read_is_an_error_not_the_chain_end(self):
|
||||
p = proxy()
|
||||
missing = libvirtError("no volume")
|
||||
missing.err = (libvirt.VIR_ERR_NO_STORAGE_VOL,)
|
||||
p.get_volume_by_path = MagicMock(side_effect=missing)
|
||||
self.assertIsNone(p._backing_path("/elsewhere/base.qcow2")) # in no pool
|
||||
p.get_volume_by_path = MagicMock(side_effect=libvirtError("connection lost"))
|
||||
with self.assertRaises(libvirtError):
|
||||
p._backing_path("/pool/a.one")
|
||||
|
||||
def test_the_snapshot_chain_and_memory_go_but_not_the_template(self):
|
||||
delete, keep = self.split()
|
||||
self.assertEqual(sorted(delete), ["/pool/a.one", "/pool/a.qcow2", "/pool/a.two", "/pool/vm.one.mem"])
|
||||
self.assertEqual(keep, [])
|
||||
|
||||
def test_a_file_another_vm_grows_from_keeps_the_files_below_it(self):
|
||||
delete, keep = self.split(other_overlay_on="/pool/a.one")
|
||||
self.assertEqual(sorted(delete), ["/pool/a.two", "/pool/vm.one.mem"])
|
||||
self.assertEqual(sorted(keep), ["/pool/a.one", "/pool/a.qcow2"])
|
||||
|
||||
def test_newer_snapshots_kept_after_a_revert_go_too(self):
|
||||
# reverted to one: the disk is a new overlay on a.one, and a.two of
|
||||
# snapshot two branches off it
|
||||
delete, keep = self.split(top="/pool/a.reverted")
|
||||
self.assertEqual(
|
||||
sorted(delete), ["/pool/a.one", "/pool/a.qcow2", "/pool/a.reverted", "/pool/a.two", "/pool/vm.one.mem"]
|
||||
)
|
||||
self.assertEqual(keep, [])
|
||||
|
||||
def test_a_snapshot_file_off_the_disks_chain_stays(self):
|
||||
# a disk detached since: its snapshot overlay grows from a file the
|
||||
# VM's disks do not
|
||||
delete, _ = self.split(extra=("/pool/gone.old", "/pool/gone.qcow2"))
|
||||
self.assertNotIn("/pool/gone.old", delete)
|
||||
self.assertNotIn("/pool/gone.qcow2", delete)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue