diff --git a/README.md b/README.md index f052db96..1098b3aa 100644 --- a/README.md +++ b/README.md @@ -556,6 +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. - Docker no longer publishes port 6080; the console goes through nginx on the panel's port (`WS_PUBLIC_PORT`, default 80). ## Usage diff --git a/instances/models.py b/instances/models.py index f49d500b..0bf4f468 100644 --- a/instances/models.py +++ b/instances/models.py @@ -2,7 +2,12 @@ from computes.models import Compute from django.db import models from django.utils.functional import cached_property from django.utils.translation import gettext_lazy as _ -from libvirt import VIR_DOMAIN_XML_SECURE, libvirtError +from libvirt import ( + VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL, + VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL, + VIR_DOMAIN_XML_SECURE, + libvirtError, +) from vrtManager.instance import wvmInstance from webvirtcloud.settings import QEMU_CONSOLE_LISTENER_ADDRESSES @@ -253,13 +258,24 @@ class Instance(models.Model): def video_models(self): return self.proxy.get_video_models(self.arch, self.machine) - @cached_property - def snapshots(self) -> list[dict]: - return sorted(self.proxy.get_snapshot(), reverse=True, key=lambda k: k["date"]) + def _snapshots(self, flag): + return sorted(self.proxy.get_snapshot(flag), reverse=True, key=lambda k: k["date"]) @cached_property - def external_snapshots(self): - return sorted(self.proxy.get_external_snapshots(), reverse=True, key=lambda k: k["date"]) + def snapshots(self) -> list[dict]: + return self._snapshots(VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL | VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL) + + @cached_property + def internal_snapshots(self) -> list[dict]: + return self._snapshots(VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL) + + @cached_property + def external_snapshots(self) -> list[dict]: + return self._snapshots(VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL) + + @cached_property + def snapshot_jobs(self) -> list[dict]: + return self.proxy.get_snapshot_jobs() @cached_property def inst_xml(self): diff --git a/instances/templates/instances/snapshots_tab.html b/instances/templates/instances/snapshots_tab.html index 6cecd372..9700b592 100755 --- a/instances/templates/instances/snapshots_tab.html +++ b/instances/templates/instances/snapshots_tab.html @@ -1,6 +1,7 @@ {% load i18n %} {% load bootstrap_icons %}
+ {% with busy=instance.snapshot_jobs %}
+ {% if busy %} +
+ {% trans "A snapshot job is running:" %} + {% for job in busy %}{{ job.dev }} {% widthratio job.cur job.end 100 %}%{% if not forloop.last %}, {% endif %}{% endfor %}. + {% trans "Reload the page to see its progress." %} +
+ {% endif %}
{% if instance.status != 5 %}

{% trans "With running machine, internal snapshots may take more than an hour, depending on how much memory has on your instance and how large the disk is." %}

@@ -34,116 +42,137 @@ | - {% if instance.external_snapshots|length > 0 %} - + {% if instance.external_snapshots or busy %} + {% else %} - + {% endif %}
- {% if instance.status != 5 %} -

{% trans "You can get external snapshots within this tab." %}

- {% else %} -

{% trans "Create an external snapshot" %}

- {% endif %} -

{% trans "External snapshots are experimental in this stage, use it if you know what you are doing. 'Revert Snapshot' may require manual operation with CLI." %}

-
+

{% trans "Create an external snapshot. Each disk keeps its state in a new file next to it." %}

+ {% csrf_token %}
- + | - {% if instance.external_snapshots|length > 0 or instance.snapshots|length > 0 %} - + {% if instance.internal_snapshots or busy %} + {% else %} - + {% endif %}
-

{% trans "WebVirtCloud supports only one external snapshot at the moment." %}

+
+ + +
+

{% trans "A VM with snapshots cannot be migrated." %}

- {% if instance.snapshots or instance.external_snapshots %} + {% if instance.internal_snapshots or instance.external_snapshots %}

{% trans "Choose a snapshot for restore/delete" %}

+ {% if instance.internal_snapshots %} +
{% trans "Internal" %}
- +
- + - {% if instance.snapshots %} - {% for snap in instance.snapshots %} - - - - - - - - {% endfor %} - {% elif instance.external_snapshots %} - {% for ext_snap in instance.external_snapshots %} - - - - - + + + + - - - {% endfor %} - {% endif %} + {% endif %} + + + + + {% endfor %}
{% trans "Date" %} {% trans "Name" %}{% trans "Type - Description" %}{% trans "Description" %} {% trans "Action" %}
{{ snap.date|date:"M d H:i:s" }}{{ snap.name }}({% trans "Internal" %}) - {{ snap.description }} -
- {% csrf_token %} - - {% if instance.status == 5 %} - - {% else %} - - {% endif %} -
- 		-
{% csrf_token %} - - -
-
{{ ext_snap.date|date:"M d H:i:s" }}{{ ext_snap.name }}({% trans "External" %}) - {{ ext_snap.description }} -
- {% csrf_token %} - - - -
{{ snap.date|date:"M d H:i:s" }}{{ snap.name }}{{ snap.description }} + + {% csrf_token %} + + {% if instance.status == 5 %} + - - -
{% csrf_token %} - - -
-
+
{% csrf_token %} + + +
+
+ {% endif %} + {% if instance.external_snapshots %} +
{% trans "External" %}
+
+ + + + + + + + + {% for ext_snap in instance.external_snapshots %} + + + + + + + + {% endfor %} + +
{% trans "Date" %}{% trans "Name" %}{% trans "Description" %}{% trans "Action" %}
{{ ext_snap.date|date:"M d H:i:s" }} + {{ ext_snap.name }} + {% if ext_snap.current %}{% trans "current" %}{% endif %} + {% if ext_snap.memory %}{% trans "memory" %}{% endif %} + {{ ext_snap.description }} +
+ {% csrf_token %} + + +
+ 		+
{% csrf_token %} + + +
+
+
+ {% endif %} {% else %}

{% trans "You do not have any snapshots" %}

{% endif %}
+ {% endwith %} diff --git a/instances/tests/test_destroy.py b/instances/tests/test_destroy.py index 61a02521..f42dd89e 100644 --- a/instances/tests/test_destroy.py +++ b/instances/tests/test_destroy.py @@ -50,7 +50,6 @@ class DestroyOrderTestCase(TestCase): def test_failed_undefine_deletes_no_disk(self): proxy = self.destroy(undefine_error=libvirtError("refused")) proxy.get_volume_by_path.assert_not_called() - proxy.delete_all_disks.assert_not_called() self.assertTrue(Instance.objects.filter(pk=self.instance.pk).exists()) def test_destroy_and_delete_vol_hold_the_compute_exclusively(self): diff --git a/instances/tests/test_live_dataloss.py b/instances/tests/test_live_dataloss.py index 6e131c1f..7629c333 100644 --- a/instances/tests/test_live_dataloss.py +++ b/instances/tests/test_live_dataloss.py @@ -92,7 +92,6 @@ class LiveDataLossTestCase(TestCase): # External snapshots - @unittest.expectedFailure def test_revert_external_snapshot_keeps_disk_added_later(self): base = livetest.create_volume(self.conn, P + "r03-a") dom, inst = self.vm("r03-a", [base]) @@ -104,11 +103,11 @@ class LiveDataLossTestCase(TestCase): f"", libvirt.VIR_DOMAIN_AFFECT_CONFIG, ) - self.post("revert_external_snapshot", inst, {"name": "s1.snap", "date": "", "desc": ""}) + self.post("revert_external_snapshot", inst, {"name": "snap"}) self.assertTrue(livetest.volume_exists(self.conn, later), "revert deleted a disk the snapshot never had") + self.assertEqual(set(self.disk_sources(dom, libvirt.VIR_DOMAIN_XML_INACTIVE)), {"vda"}) - @unittest.expectedFailure def test_delete_older_external_snapshot_keeps_newer_snapshot_files(self): base = livetest.create_volume(self.conn, P + "r03-b") dom, inst = self.vm("r03-b", [base]) @@ -116,11 +115,11 @@ class LiveDataLossTestCase(TestCase): self.post("create_external_snapshot", inst, {"name": "newer"}) sources = set(self.disk_sources(dom).values()) - self.post("delete_external_snapshot", inst, {"name": "s1.older"}) + self.post("delete_external_snapshot", inst, {"name": "older"}) for path in sources: self.assertTrue(livetest.volume_exists(self.conn, path), f"{path} was deleted") - self.assertIn("s1.newer", dom.snapshotListNames(0)) + self.assertIn("newer", dom.snapshotListNames(0)) # Internal snapshots of UEFI VMs diff --git a/instances/tests/test_live_snapshots.py b/instances/tests/test_live_snapshots.py new file mode 100644 index 00000000..2c73fb0e --- /dev/null +++ b/instances/tests/test_live_snapshots.py @@ -0,0 +1,144 @@ +"""Live checks for external snapshots on libvirt's own API. Runs only with +TEST_LIBVIRT_HOST set (see instances/tests/livetest.py).""" + +import os +import unittest + +import libvirt +from django.test import SimpleTestCase +from lxml import etree +from vrtManager import util +from vrtManager.connection import connection_manager +from vrtManager.instance import wvmInstance + +from . import livetest + +P = livetest.PREFIX + + +class LiveExternalSnapshotTestCase(SimpleTestCase): + @classmethod + def setUpClass(cls): + if not livetest.enabled(): + raise unittest.SkipTest("Set TEST_LIBVIRT_HOST to run the live libvirt tests") + cls.args = ( + os.environ["TEST_LIBVIRT_HOST"], + os.environ.get("TEST_LIBVIRT_LOGIN", ""), + os.environ.get("TEST_LIBVIRT_PASSWORD", ""), + int(os.environ.get("TEST_LIBVIRT_TYPE", 4)), + ) + cls.conn = connection_manager.get_connection(*cls.args) + livetest.cleanup(cls.conn) + livetest.ensure_pool(cls.conn) + cls.inventory = livetest.inventory(cls.conn) + super().setUpClass() + + @classmethod + def tearDownClass(cls): + try: + livetest.remove_pool(cls.conn) + finally: + super().tearDownClass() + if livetest.inventory(cls.conn) != cls.inventory: + raise AssertionError("Live tests changed objects outside the test namespace") + + def tearDown(self): + livetest.cleanup(self.conn) + + def vm(self, name, state="stopped", uefi=False, extra_devices=""): + base = livetest.create_volume(self.conn, P + name) + dom = livetest.define_vm(self.conn, P + name, [base], uefi=uefi, extra_devices=extra_devices) + if state in ("running", "paused"): + dom.create() + if state == "paused": + dom.suspend() + return dom, wvmInstance(*self.args, None, uuid=dom.UUIDString()) + + def top(self, dom): + return etree.fromstring(dom.XMLDesc(0)).xpath("string(devices/disk[@device='disk']/source/@file)") + + def test_create_revert_delete_keep_the_state(self): + for state, expected in (("stopped", libvirt.VIR_DOMAIN_SHUTOFF), + ("running", libvirt.VIR_DOMAIN_RUNNING), + ("paused", libvirt.VIR_DOMAIN_PAUSED)): + with self.subTest(state): + dom, vm = self.vm("snap-" + state, state) + base = self.top(dom) + vm.create_external_snapshot("one", "first") + vm.create_external_snapshot("two") + vm.revert_external_snapshot("one") + self.assertEqual(dom.state()[0], expected) + vm.delete_external_snapshot("two") + vm.delete_external_snapshot("one") + self.assertEqual(dom.snapshotNum(0), 0) + self.assertEqual(dom.state()[0], expected) + self.assertTrue(livetest.volume_exists(self.conn, base)) + + def test_memory_snapshot_resumes_and_its_file_goes_with_it(self): + dom, vm = self.vm("snap-mem", "running") + vm.create_external_snapshot("m", memory=True) + mem = os.path.join(livetest.POOL_PATH, f"{dom.UUIDString()}.m.mem") + self.assertTrue(livetest.volume_exists(self.conn, mem)) + dom.suspend() + vm.revert_external_snapshot("m") + self.assertEqual(dom.state()[0], libvirt.VIR_DOMAIN_RUNNING) + vm.delete_external_snapshot("m") + self.assertFalse(livetest.volume_exists(self.conn, mem)) + + def test_uefi_vm_keeps_its_loader(self): + dom, vm = self.vm("snap-uefi", "running", uefi=True) + os_before = etree.tostring(etree.fromstring(dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)).find("os")) + vm.create_external_snapshot("u") + vm.revert_external_snapshot("u") + vm.delete_external_snapshot("u") + self.assertEqual(etree.tostring(etree.fromstring(dom.XMLDesc(libvirt.VIR_DOMAIN_XML_INACTIVE)).find("os")), os_before) + self.assertEqual(dom.snapshotNum(0), 0) + + def test_back_to_back_reverts_keep_a_running_vm_running(self): + # Without the retry, the second revert within one second stopped the VM. + dom, vm = self.vm("snap-collide", "running") + vm.create_external_snapshot("a") + for _ in range(3): + vm.revert_external_snapshot("a") + vm.revert_external_snapshot("a") + self.assertEqual(dom.state()[0], libvirt.VIR_DOMAIN_RUNNING) + + def test_a_mixed_vm_can_be_cleaned_up(self): + dom, vm = self.vm("snap-mixed-clean") + dom.snapshotCreateXML("i", 0) + dom.snapshotCreateXML( + "e", libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY + ) + # libvirt refuses an external snapshot whose parent is internal, so the + # internal one goes first + vm.snapshot_delete("i") + vm.delete_external_snapshot("e") + self.assertEqual(dom.snapshotNum(0), 0) + + def test_chain_stays_linear(self): + dom, vm = self.vm("snap-linear") + vm.create_external_snapshot("a") + vm.create_external_snapshot("b") + vm.revert_external_snapshot("a") + with self.assertRaisesRegex(util.OperationError, "Newer snapshots"): + vm.create_external_snapshot("c") + vm.delete_external_snapshot("b") + vm.create_external_snapshot("c") + + def test_refusals(self): + iso = livetest.create_volume(self.conn, P + "snap-media") + cdrom = (f"" + "") + _, vm = self.vm("snap-cdrom", extra_devices=cdrom) + with self.assertRaisesRegex(util.OperationError, "Eject"): + vm.create_external_snapshot("x") + + dom, vm = self.vm("snap-saved", "running") + dom.managedSave(0) + with self.assertRaisesRegex(util.OperationError, "saved state"): + vm.create_external_snapshot("x") + + dom, vm = self.vm("snap-mixed") + dom.snapshotCreateXML("i", 0) + with self.assertRaisesRegex(util.OperationError, "internal snapshots"): + vm.create_external_snapshot("x") diff --git a/instances/tests/test_page_controls.py b/instances/tests/test_page_controls.py index 8ff71945..effcfa6b 100644 --- a/instances/tests/test_page_controls.py +++ b/instances/tests/test_page_controls.py @@ -69,7 +69,7 @@ class PageControls: } for getter in ("get_networks", "get_ifaces", "get_storages", "get_disk_devices", "get_media_devices", "get_net_devices", "get_snapshot", - "get_external_snapshots"): + "get_snapshot_jobs"): getattr(proxy, getter).return_value = [] proxy.get_console_type.return_value = "vnc" proxy.get_status.return_value = status diff --git a/instances/tests/test_pending_edits.py b/instances/tests/test_pending_edits.py index 56e93e44..1727a345 100644 --- a/instances/tests/test_pending_edits.py +++ b/instances/tests/test_pending_edits.py @@ -45,7 +45,7 @@ class PendingEditsTestCase(TestCase): proxy.get_max_cpus.return_value = [1, 2, 3, 4] proxy.get_vcpus.return_value = {} for getter in ("get_networks", "get_ifaces", "get_storages", "get_media_devices", "get_net_devices", - "get_snapshot", "get_external_snapshots"): + "get_snapshot", "get_snapshot_jobs"): getattr(proxy, getter).return_value = [] proxy.get_disk_devices.side_effect = lambda **kw: config if kw.get("config") else live proxy.get_all_qos.side_effect = lambda **kw: qos[1] if kw.get("config") else qos[0] diff --git a/instances/tests/test_snapshot_views.py b/instances/tests/test_snapshot_views.py new file mode 100644 index 00000000..bbd09584 --- /dev/null +++ b/instances/tests/test_snapshot_views.py @@ -0,0 +1,124 @@ +"""The snapshot views call the backend once and show its refusals as +messages; they no longer start, pause or stop the VM themselves.""" + +from datetime import datetime +from unittest.mock import PropertyMock, patch + +import libvirt +from django.contrib.auth import get_user_model +from django.contrib.messages import get_messages +from django.test import TestCase +from django.urls import reverse +from lxml import html as lxml_html +from vrtManager import util + +from computes.models import Compute +from instances.models import Instance + + +class SnapshotViewsTestCase(TestCase): + def setUp(self): + compute = Compute.objects.create(name="snap-compute", hostname="127.0.0.1:1", login="root", password="", type=1) + self.instance = Instance.objects.create( + compute=compute, name="snap-vm", uuid="dddddddd-eeee-ffff-0000-222222222222" + ) + admin = get_user_model().objects.create_superuser("snap_admin", "s@example.com", "pw") + self.client.force_login(admin) + + def post(self, view, data, error=None): + with patch("instances.models.wvmInstance") as mock_wvm: + proxy = mock_wvm.return_value + method = { + "create_external_snapshot": proxy.create_external_snapshot, + "revert_external_snapshot": proxy.revert_external_snapshot, + "delete_external_snapshot": proxy.delete_external_snapshot, + "revert_snapshot": proxy.snapshot_revert, + "delete_snapshot": proxy.snapshot_delete, + "snapshot": proxy.create_snapshot, + }[view] + if error: + method.side_effect = util.OperationError(error) + response = self.client.post( + reverse(f"instances:{view}", args=[self.instance.id]), + data, + HTTP_REFERER=reverse("instances:instance", args=[self.instance.id]), + ) + self.assertEqual(response.status_code, 302) + return proxy, method, [str(m) for m in get_messages(response.wsgi_request)] + + def test_create_passes_name_description_and_memory(self): + _, method, _ = self.post("create_external_snapshot", {"name": "a", "description": "d", "memory": "on"}) + method.assert_called_once_with("a", "d", True) + _, method, _ = self.post("create_external_snapshot", {"name": "a", "description": ""}) + method.assert_called_once_with("a", "", False) + + def test_revert_and_delete_call_the_backend_once_and_leave_power_alone(self): + for view in ("revert_external_snapshot", "delete_external_snapshot"): + with self.subTest(view): + proxy, method, _ = self.post(view, {"name": "a"}) + method.assert_called_once_with("a") + for power in ("start", "force_shutdown", "shutdown"): + getattr(proxy, power).assert_not_called() + + def test_refusals_become_messages(self): + for view in ("create_external_snapshot", "revert_external_snapshot", "delete_external_snapshot", + "revert_snapshot", "delete_snapshot", "snapshot"): + with self.subTest(view): + _, _, messages = self.post(view, {"name": "a"}, error="refused for a reason") + self.assertTrue(any("refused for a reason" in m for m in messages), messages) + + +class SnapshotTabTestCase(TestCase): + setUp = SnapshotViewsTestCase.setUp # same VM and superuser, without rerunning the view tests + + def page(self, external=(), internal=(), jobs=(), status=1, nvram=None): + with patch("instances.models.wvmInstance") as mock_wvm, patch.object( + Compute, "status", new_callable=PropertyMock, return_value=True + ): + proxy = mock_wvm.return_value + for getter in ("get_networks", "get_ifaces", "get_storages", "get_disk_devices", + "get_media_devices", "get_net_devices"): + getattr(proxy, getter).return_value = [] + proxy.get_snapshot.side_effect = lambda flag: { + libvirt.VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL: list(internal), + libvirt.VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL: list(external), + }.get(flag, [*internal, *external]) + proxy.get_snapshot_jobs.return_value = list(jobs) + proxy.get_status.return_value = status + proxy.get_nvram.return_value = nvram + proxy.get_memory.return_value = proxy.get_cur_memory.return_value = 1024 + proxy.get_max_memory.return_value = 4096 + proxy.get_vcpu.return_value = proxy.get_cur_vcpu.return_value = 1 + proxy.get_max_cpus.return_value = [1] + proxy.get_vcpus.return_value = {} + proxy.get_console_type.return_value = "vnc" + response = self.client.get(reverse("instances:instance", args=[self.instance.id])) + self.assertEqual(response.status_code, 200) + return lxml_html.fromstring(response.content) + + def snap(self, name, **kw): + return {"name": name, "date": datetime(2026, 1, 1), "description": "", "memory": False, + "current": False, "detaches": [], **kw} + + def test_revert_confirm_names_detached_disks_and_nvram(self): + doc = self.page(external=[self.snap("e", detaches=[{"dev": "vdb", "path": "/pool/later.qcow2"}])], + nvram="/var/lib/libvirt/qemu/nvram/x_VARS.fd") + [button] = doc.xpath("//button[@name='revert_external_snapshot']") + text = button.get("data-confirm") + self.assertIn("vdb (/pool/later.qcow2)", text) + self.assertIn("NVRAM", text) + + def test_memory_checkbox_only_for_a_running_vm(self): + self.assertFalse(self.page(status=1).xpath("//input[@name='memory']")[0].get("disabled")) + self.assertIsNotNone(self.page(status=5).xpath("//input[@name='memory']")[0].get("disabled")) + + def test_a_running_job_shows_progress_and_disables_actions(self): + doc = self.page(external=[self.snap("e")], jobs=[{"dev": "vda", "cur": 1, "end": 4}]) + self.assertIn("vda 25%", doc.text_content()) + self.assertTrue(all(b.get("disabled") is not None + for b in doc.xpath("//div[@id='snapshots']//button[@type='submit']"))) + + def test_lists_are_separate(self): + doc = self.page(internal=[self.snap("i")], external=[self.snap("e")]) + self.assertEqual(doc.xpath("//table[@id='internal-snapshots']//strong/text()"), ["i"]) + self.assertEqual(doc.xpath("//table[@id='external-snapshots']//strong/text()"), ["e"]) diff --git a/instances/views.py b/instances/views.py index 7365a8ac..a353cfec 100755 --- a/instances/views.py +++ b/instances/views.py @@ -35,7 +35,6 @@ from libvirt import (VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_DOMAIN_UNDEFINE_MANAGED_SAVE, VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA, VIR_DOMAIN_UNDEFINE_NVRAM, - VIR_DOMAIN_START_PAUSED, libvirtError) from logs.views import addlogmsg from webvirtcloud.middleware import error_text @@ -1342,7 +1341,11 @@ def delete_snapshot(request, pk): if not request.user.has_perm("instances.snapshot_instances"): raise PermissionDenied snap_name = request.POST.get("name", "") - instance.proxy.snapshot_delete(snap_name) + try: + instance.proxy.snapshot_delete(snap_name) + except libvirtError as err: + messages.error(request, _("Snapshot was not deleted: %(err)s") % {"err": error_text(request, err)}) + return _back(request, pk, "managesnapshot") msg = _("Delete snapshot: %(snap)s") % {"snap": snap_name} addlogmsg(request.user.username, instance.compute.name, instance.name, msg) return _back(request, pk, "managesnapshot") @@ -1376,9 +1379,14 @@ def create_external_snapshot(request, pk): raise PermissionDenied name = request.POST.get("name", "") desc = request.POST.get("description", "") - instance.proxy.create_external_snapshot("s1." + name, instance, desc=desc) - msg = _("Create external snapshot: %(snap)s") % {"snap": name} - addlogmsg(request.user.username, instance.compute.name, instance.name, msg) + memory = request.POST.get("memory") == "on" + try: + instance.proxy.create_external_snapshot(name, desc, memory) + except libvirtError as err: + messages.error(request, _("Snapshot was not created: %(err)s") % {"err": error_text(request, err)}) + else: + msg = _("Create external snapshot: %(snap)s") % {"snap": name} + addlogmsg(request.user.username, instance.compute.name, instance.name, msg) return _back(request, pk, "managesnapshot") @@ -1388,15 +1396,15 @@ def revert_external_snapshot(request, pk): instance = get_instance(request.user, pk, perm_type="change") if not request.user.has_perm("instances.snapshot_instances"): raise PermissionDenied - instance_state = True if instance.proxy.get_status() != 5 else False name = request.POST.get("name", "") - date = request.POST.get("date", "") - desc = request.POST.get("desc", "") - instance.proxy.force_shutdown() if instance_state else None - instance.proxy.revert_external_snapshot(name, date, desc) - instance.proxy.start() if instance_state else None - msg = _("Revert external snapshot: %(snap)s") % {"snap": name} - addlogmsg(request.user.username, instance.compute.name, instance.name, msg) + try: + instance.proxy.revert_external_snapshot(name) + except libvirtError as err: + messages.error(request, _("Snapshot was not reverted: %(err)s") % {"err": error_text(request, err)}) + else: + messages.success(request, _("Successful revert snapshot: ") + name) + msg = _("Revert external snapshot: %(snap)s") % {"snap": name} + addlogmsg(request.user.username, instance.compute.name, instance.name, msg) return _back(request, pk, "managesnapshot") @@ -1406,18 +1414,14 @@ def delete_external_snapshot(request, pk): instance = get_instance(request.user, pk, perm_type="change") if not request.user.has_perm("instances.snapshot_instances"): raise PermissionDenied - instance_state = True if instance.proxy.get_status() == 5 else False name = request.POST.get("name", "") - - instance.proxy.start(VIR_DOMAIN_START_PAUSED) if instance_state else None - try: instance.proxy.delete_external_snapshot(name) + except libvirtError as err: + messages.error(request, _("Snapshot was not deleted: %(err)s") % {"err": error_text(request, err)}) + else: msg = _("Delete external snapshot: %(snap)s") % {"snap": name} addlogmsg(request.user.username, instance.compute.name, instance.name, msg) - finally: - instance.proxy.force_shutdown() if instance_state else None - return _back(request, pk, "managesnapshot") diff --git a/vrtManager/instance.py b/vrtManager/instance.py index a5ac0b52..8e479be8 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -3,6 +3,7 @@ import ipaddress import json import logging import os.path +import re import string import threading import time @@ -31,14 +32,13 @@ try: VIR_MIGRATE_PERSIST_DEST, VIR_MIGRATE_UNDEFINE_SOURCE, VIR_MIGRATE_UNSAFE, + VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC, VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY, - VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY, + VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED, + VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING, + VIR_ERR_CONFIG_UNSUPPORTED, VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL, VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL, - VIR_DOMAIN_BLOCK_COMMIT_DELETE, - VIR_DOMAIN_BLOCK_COMMIT_ACTIVE, - VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT, - VIR_DOMAIN_START_PAUSED, libvirtError, ) from libvirt_qemu import VIR_DOMAIN_QEMU_AGENT_COMMAND_DEFAULT, qemuAgentCommand @@ -59,6 +59,9 @@ from vrtManager.storage import wvmStorage, wvmStorages # with secrets. Live XML would drop pending changes, non-secure XML the VNC # password. PERSISTENT_XML = VIR_DOMAIN_XML_INACTIVE | VIR_DOMAIN_XML_SECURE + +# External snapshot names become part of overlay and memory file names. +SNAPSHOT_NAME = re.compile(r"[A-Za-z0-9_-][A-Za-z0-9._-]{0,63}") # the longest window wvmInstance.usage() averages over USAGE_MAX_WINDOW = 30 @@ -1509,17 +1512,12 @@ class wvmInstance(wvmConnect): [path for path in own if path in used_elsewhere], ) - def delete_all_disks(self): - self.refresh_instance_pools() - disks = self.get_disk_devices() - for disk in disks: - vol = self.get_volume_by_path(disk.get("path")) - vol.delete(0) - def _snapshotCreateXML(self, xml, flag): self.instance.snapshotCreateXML(xml, flag) def create_snapshot(self, name, desc=None): + if self.instance.snapshotListNames(VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL): + raise util.OperationError("The VM has external snapshots; internal snapshots cannot be mixed with them") state = "shutoff" if self.get_status() == 5 else "running" name_esc = util.xml_escape(name) desc_esc = util.xml_escape(desc) if desc else "" @@ -1540,139 +1538,184 @@ class wvmInstance(wvmConnect): """ self._snapshotCreateXML(xml, 0) - def change_snapshot_xml(self): - try: - raw_xml = self._XMLDesc(VIR_DOMAIN_XML_SECURE) - tree = ElementTree.fromstring(raw_xml) - loader = tree.find("./os/loader") - if loader is not None and loader.get("type") == "pflash": - loader.set("type", "rom") - self._defineXML(ElementTree.tostring(tree, encoding="unicode")) - return True - return False - except Exception as e: - logger.error("Failed to change snapshot XML loader: %s", e) - raise - - def recover_snapshot_xml(self): - try: - raw_xml = self._XMLDesc(VIR_DOMAIN_XML_SECURE) - tree = ElementTree.fromstring(raw_xml) - loader = tree.find("./os/loader") - if loader is not None and loader.get("type") == "rom": - loader.set("type", "pflash") - self._defineXML(ElementTree.tostring(tree, encoding="unicode")) - return True - return False - except Exception as e: - logger.error("Failed to recover snapshot XML loader: %s", e) - raise - - def create_external_snapshot(self, name, date=None, desc=None): - creation_time = time.time() - state = "shutoff" if self.get_status() == 5 else "running" - name_esc = util.xml_escape(name) - desc_esc = util.xml_escape(desc) if desc else "" - # - xml = """ - %s - %s - %s - %d - """ % ( - name_esc, - desc_esc, - state, - creation_time, + def create_external_snapshot(self, name, desc="", memory=False): + """A disk-only snapshot, or with memory too (running VMs only). Every + disk gets a libvirt-named overlay next to it; the memory file goes + next to the first disk.""" + if not SNAPSHOT_NAME.fullmatch(name): + raise util.OperationError( + "Snapshot names are 1-64 letters, digits, '.', '_' or '-' and do not start with '.'" + ) + self._check_snapshot_state() + if self.instance.snapshotListNames(VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL): + raise util.OperationError("The VM has internal snapshots; external snapshots cannot be mixed with them") + if name in self.instance.snapshotListNames(0): + raise util.OperationError(f"A snapshot named {name} already exists") + if self.instance.hasCurrentSnapshot(0): + if self.instance.snapshotCurrent(0).numChildren(0): + raise util.OperationError("Newer snapshots exist after the last revert; delete them first") + elif self.instance.snapshotListNames(0): + # only after changes outside WebVirtCloud; a create would start a second root + raise util.OperationError("No snapshot is current; revert to a snapshot first") + tree = etree.fromstring(self._XMLDesc(0)) + for disk in tree.xpath("devices/disk"): + dev = disk.xpath("string(target/@dev)") + if disk.get("device") in ("cdrom", "floppy"): + if disk.xpath("source/@file|source/@dev|source/@name"): + raise util.OperationError(f"Eject the media from {dev} first") + elif disk.get("type") != "file": + raise util.OperationError(f"Disk {dev} is not a file; external snapshots need file disks") + memory_xml = self._snapshot_memory_xml(name, tree) if memory else "" + xml = ( + f"{name}" + f"{util.xml_escape(desc)}{memory_xml}" ) - - changed = self.change_snapshot_xml() - try: - xml += self._XMLDesc(VIR_DOMAIN_XML_SECURE) - xml += """0 - """ - - self._snapshotCreateXML(xml, VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY) - finally: - if changed: - self.recover_snapshot_xml() + flags = VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC + if not memory: + flags |= VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY + self._snapshotCreateXML(xml, flags) + self.refresh_instance_pools() + + def _snapshot_memory_xml(self, name, tree): + """The element of a snapshot that saves memory: a file in the + storage pool of the first disk, which must have room for it.""" + if self.get_status() != 1: + raise util.OperationError("Memory is saved only from a running VM") + disks = list(self._disk_sources(tree).values()) + if not disks: + raise util.OperationError("The VM has no disk to keep its memory next to") + # the top overlay is known to its pool only after a refresh + self.refresh_instance_pools() + try: + pool = self.get_volume_by_path(disks[0]).storagePoolLookupByVolume() + except libvirtError: + raise util.OperationError(f"{disks[0]} is not in a storage pool; memory is saved only into one") from None + if pool.info()[3] < self.get_cur_memory() * 1024 * 1024: + raise util.OperationError(f"Storage pool {pool.name()} has less free space than the VM's memory") + path = os.path.join(os.path.dirname(disks[0]), f"{self.get_uuid()}.{name}.mem") + return f"" + + def revert_external_snapshot(self, name): + """libvirt returns the disks and the definition to the snapshot's. A + disk-only snapshot keeps the VM running, paused or shut off as it is; + a snapshot with memory resumes the VM as it was saved.""" + snap = self._snapshot(name, external=True) + self._check_snapshot_state() + tree = etree.fromstring(snap.getXMLDesc(0)) + # Snapshots of UEFI VMs taken before WebVirtCloud used libvirt's + # snapshot API saved the loader switched to rom; a revert would + # restore that. + saved = self._saved_definition(tree) + saved_loader = saved.xpath("string(os/loader/@type)") if saved is not None else "" + current_loader = util.get_xml_path(self._XMLDesc(PERSISTENT_XML), "/domain/os/loader/@type") + if saved_loader == "rom" and current_loader == "pflash": + raise util.OperationError( + "This snapshot was taken by an older version on a UEFI VM and cannot be reverted; delete it instead" + ) + if tree.find("memory[@snapshot='external']") is not None: + flags = 0 + else: + flags = {1: VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING, 3: VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED}.get( + self.get_status(), 0 + ) + # libvirt names the new overlays after the second, so a revert within + # the second of the previous one finds them taken, after it has + # already stopped a running VM; one retry a second later finishes it. + try: + self.instance.revertToSnapshot(snap, flags) + except libvirtError as err: + if err.get_error_code() != VIR_ERR_CONFIG_UNSUPPORTED or "already exists" not in str(err): + raise + time.sleep(1) + try: + self.instance.revertToSnapshot(snap, flags) + except libvirtError as retry_err: + state = "running" if self.instance.isActive() else "shut off" + raise util.OperationError( + f"The revert failed twice; try again in a moment ({retry_err}). The VM is {state}." + ) from None self.refresh_instance_pools() - def get_external_snapshots(self): - return self.get_snapshot(VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL) - def delete_external_snapshot(self, name): - disk_info = self.get_disk_devices() - for disk in disk_info: - target_dev = disk["dev"] - backing_file = disk["backing_file"] - snap_source_file = disk["path"] - self.instance.blockCommit(target_dev, backing_file, snap_source_file, - flags=VIR_DOMAIN_BLOCK_COMMIT_DELETE| - VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) - while True: - info = self.instance.blockJobInfo(target_dev, 0) - if info.get('cur') == info.get('end'): - self.instance.blockJobAbort(target_dev,flags=VIR_DOMAIN_BLOCK_JOB_ABORT_PIVOT) - time.sleep(2) - break - # Check again pool for snapshot delta volume; if it exist, remove it manually - with contextlib.suppress(libvirtError): - vol_snap = self.get_volume_by_path(snap_source_file) - pool = vol_snap.storagePoolLookupByVolume() - pool.refresh(0) - vol_snap.delete(0) + """libvirt merges the snapshot's overlays and removes its files.""" + snap = self._snapshot(name, external=True) + self._check_snapshot_state() + snap.delete(0) + self.refresh_instance_pools() - snap = self.instance.snapshotLookupByName(name, 0) - snap.delete(VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) + @staticmethod + def _disk_sources(xml): + """Target dev -> source file of the disks (not CD-ROMs) in a domain definition.""" + tree = etree.fromstring(xml) if isinstance(xml, str) else xml + return { + disk.xpath("string(target/@dev)"): disk.xpath("string(source/@file|source/@dev|source/@name)") + for disk in tree.xpath("devices/disk[@device='disk']") + } - - def revert_external_snapshot(self, name, date, desc): - snap = self.instance.snapshotLookupByName(name, 0) - snap_xml = snap.getXMLDesc(0) - snapXML = ElementTree.fromstring(snap_xml) + @staticmethod + def _saved_definition(tree): + """The definition a revert to this snapshot restores as the persistent one.""" + definition = tree.find("inactiveDomain") + return tree.find("domain") if definition is None else definition - self.start(flags=VIR_DOMAIN_START_PAUSED) if self.get_status() == 5 else None - self.delete_all_disks() - - self.force_shutdown() - - snap.delete(VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY) - - disks = snapXML.findall('inactiveDomain/devices/disk') - if not disks: disks = snapXML.findall('domain/devices/disk') - for disk in disks: - self.instance.updateDeviceFlags(ElementTree.tostring(disk).decode("UTF-8")) - name = name.replace("s1", "s2") - self.create_external_snapshot(name, date, desc) - - def get_snapshot(self, flag=VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL|VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL): + def get_snapshot(self, flag): + """Snapshots of one kind (VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL or + _EXTERNAL; both flags for all), with whether each saved memory, is the + current one, and which disks of the persistent definition a revert to + it detaches.""" + current_disks = self._disk_sources(self._XMLDesc(PERSISTENT_XML)) snapshots = [] - snapshot_list = self.instance.snapshotListNames(flag) - for snapshot in snapshot_list: - snap = self.instance.snapshotLookupByName(snapshot, 0) - snap_description = util.get_xml_path( - snap.getXMLDesc(0), "/domainsnapshot/description" - ) - snap_time_create = util.get_xml_path( - snap.getXMLDesc(0), "/domainsnapshot/creationTime" - ) + for snap in self.instance.listAllSnapshots(flag): + tree = etree.fromstring(snap.getXMLDesc(0)) + definition = self._saved_definition(tree) + saved_disks = self._disk_sources(definition) if definition is not None else {} snapshots.append( { - "date": datetime.fromtimestamp(int(snap_time_create)), - "name": snapshot, - "description": snap_description, + "date": datetime.fromtimestamp(int(tree.findtext("creationTime"))), + "name": snap.getName(), + "description": tree.findtext("description") or "", + "memory": tree.find("memory[@snapshot='external']") is not None, + "current": bool(snap.isCurrent(0)), + "detaches": [ + {"dev": dev, "path": path} for dev, path in current_disks.items() if dev not in saved_disks + ], } ) return snapshots + def get_snapshot_jobs(self): + """Block jobs (snapshot merges) running on the VM's disks.""" + if not self.instance.isActive(): + return [] + jobs = [] + for dev in self._disk_sources(self._XMLDesc(0)): + info = self.instance.blockJobInfo(dev, 0) + if info: + jobs.append({"dev": dev, "cur": info["cur"], "end": info["end"]}) + return jobs + + def _snapshot(self, name, external): + """The named snapshot; a snapshot of the other kind is refused.""" + flag = VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL if external else VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL + if name not in self.instance.snapshotListNames(flag): + kind = "external" if external else "internal" + raise util.OperationError(f"The VM has no {kind} snapshot named {name}") + return self.instance.snapshotLookupByName(name, 0) + + def _check_snapshot_state(self): + """Refusals shared by external snapshot create, revert and delete. The + internal/external mix is checked only on create, so a VM that is + already mixed can still be cleaned up.""" + if self.instance.hasManagedSaveImage(0): + raise util.OperationError("The VM has a saved state; start it or remove the saved state first") + if self.get_snapshot_jobs(): + raise util.OperationError("A snapshot job is still running on this VM; try again when it ends") + def snapshot_delete(self, snapshot): - snap = self.instance.snapshotLookupByName(snapshot, 0) - snap.delete(0) + self._snapshot(snapshot, external=False).delete(0) def snapshot_revert(self, snapshot): - snap = self.instance.snapshotLookupByName(snapshot, 0) - self.instance.revertToSnapshot(snap, 0) + self.instance.revertToSnapshot(self._snapshot(snapshot, external=False), 0) def get_managed_save_image(self): return self.instance.hasManagedSaveImage(0) diff --git a/vrtManager/tests/test_external_snapshots.py b/vrtManager/tests/test_external_snapshots.py new file mode 100644 index 00000000..f8b65010 --- /dev/null +++ b/vrtManager/tests/test_external_snapshots.py @@ -0,0 +1,394 @@ +"""External snapshots go through libvirt's snapshot API; WebVirtCloud only +checks preconditions and picks the flags. libvirt is mocked here; the live +cases are in instances/tests/test_live_snapshots.py.""" + +import unittest +from datetime import datetime +from unittest.mock import MagicMock + +from django.conf import settings + +if not settings.configured: + settings.configure(MAC_OUI="52:54:10") + +import libvirt +from libvirt import libvirtError +from vrtManager import util +from vrtManager.instance import PERSISTENT_XML, wvmInstance + +INTERNAL = libvirt.VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL +EXTERNAL = libvirt.VIR_DOMAIN_SNAPSHOT_LIST_EXTERNAL + +DOMAIN = """ + + + + + +""" + + +def snapshot_xml(name, memory=False, disks=("vda",)): + devices = "".join( + f"" for d in disks + ) + mem = "" if memory else "" + return ( + f"{name}d & e" + f"1700000000{mem}" + f"{devices}" + ) + + +def fake_snap(name, memory=False, disks=("vda",), current=False, children=0): + snap = MagicMock() + snap.getName.return_value = name + snap.getXMLDesc.return_value = snapshot_xml(name, memory, disks) + snap.isCurrent.return_value = current + snap.numChildren.return_value = children + return snap + + +def proxy(status=1, internal=(), external=(), xml=DOMAIN, jobs=None, managed_save=False): + """A wvmInstance without a connection. internal/external are fake snapshots.""" + p = wvmInstance.__new__(wvmInstance) + p._read_cache = None + p._XMLDesc = lambda flags: xml + p.get_status = lambda: status + p.get_uuid = lambda: "11111111-2222-3333-4444-555555555555" + p.get_cur_memory = lambda: 1024 + p.refresh_instance_pools = MagicMock() + p._snapshotCreateXML = MagicMock() + p.wvm = MagicMock() + dom = MagicMock() + snaps = {s.getName(): s for s in (*internal, *external)} + + def list_names(flag): + return [s.getName() for s in {INTERNAL: internal, EXTERNAL: external}.get(flag, (*internal, *external))] + + def list_all(flag): + return list({INTERNAL: internal, EXTERNAL: external}[flag]) + + dom.snapshotListNames.side_effect = list_names + dom.listAllSnapshots.side_effect = list_all + dom.snapshotLookupByName.side_effect = lambda name, flags: snaps[name] + current = [s for s in snaps.values() if s.isCurrent()] + dom.hasCurrentSnapshot.return_value = bool(current) + dom.snapshotCurrent.return_value = current[0] if current else None + dom.hasManagedSaveImage.return_value = managed_save + dom.isActive.return_value = status in (1, 3) + dom.blockJobInfo.side_effect = lambda dev, flags: (jobs or {}).get(dev, {}) + p.instance = dom + return p + + +class SnapshotListTestCase(unittest.TestCase): + def test_entries_carry_memory_current_and_description(self): + p = proxy(external=[fake_snap("one", memory=True, current=True)]) + [entry] = p.get_snapshot(EXTERNAL) + self.assertEqual(entry["name"], "one") + self.assertEqual(entry["description"], "d & e") + self.assertEqual(entry["date"], datetime.fromtimestamp(1700000000)) + self.assertTrue(entry["memory"]) + self.assertTrue(entry["current"]) + + def test_detaches_lists_disks_missing_from_the_snapshot(self): + p = proxy(external=[fake_snap("one", disks=("vda",))]) + [entry] = p.get_snapshot(EXTERNAL) + self.assertEqual(entry["detaches"], [{"dev": "vdb", "path": "/pool/later.qcow2"}]) + + def test_detaches_comes_from_the_persistent_definition(self): + # vdc was hot-plugged into the running VM only; vdb waits for the next start + live = DOMAIN.replace("/pool/later.qcow2", "/pool/live-only.qcow2").replace("dev='vdb'", "dev='vdc'") + p = proxy(external=[fake_snap("one")]) + p._XMLDesc = lambda flags: DOMAIN if flags == PERSISTENT_XML else live + [entry] = p.get_snapshot(EXTERNAL) + self.assertEqual(entry["detaches"], [{"dev": "vdb", "path": "/pool/later.qcow2"}]) + + def test_lists_are_by_kind(self): + p = proxy(internal=[fake_snap("i")], external=[fake_snap("e")]) + self.assertEqual([s["name"] for s in p.get_snapshot(INTERNAL)], ["i"]) + self.assertEqual([s["name"] for s in p.get_snapshot(EXTERNAL)], ["e"]) + + +class SnapshotJobsTestCase(unittest.TestCase): + def test_running_jobs_are_reported_per_disk(self): + p = proxy(jobs={"vda": {"cur": 5, "end": 10, "type": 3, "bandwidth": 0}}) + self.assertEqual(p.get_snapshot_jobs(), [{"dev": "vda", "cur": 5, "end": 10}]) + + def test_a_stopped_vm_has_no_jobs(self): + p = proxy(status=5) + self.assertEqual(p.get_snapshot_jobs(), []) + p.instance.blockJobInfo.assert_not_called() + + +class SnapshotLookupTestCase(unittest.TestCase): + def test_a_name_of_the_other_kind_is_refused(self): + p = proxy(internal=[fake_snap("i")], external=[fake_snap("e")]) + with self.assertRaises(util.OperationError): + p._snapshot("i", external=True) + with self.assertRaises(util.OperationError): + p._snapshot("e", external=False) + self.assertEqual(p._snapshot("e", external=True).getName(), "e") + + +class SnapshotStateTestCase(unittest.TestCase): + def test_managed_save_is_refused(self): + with self.assertRaisesRegex(util.OperationError, "saved state"): + proxy(managed_save=True)._check_snapshot_state() + + def test_running_job_is_refused(self): + with self.assertRaisesRegex(util.OperationError, "still running"): + proxy(jobs={"vda": {"cur": 1, "end": 2}})._check_snapshot_state() + + def test_a_mixed_vm_passes_so_it_can_be_cleaned_up(self): + proxy(internal=[fake_snap("i")], external=[fake_snap("e")])._check_snapshot_state() + + def test_a_clean_vm_passes(self): + proxy()._check_snapshot_state() + + +DISK_ONLY_ATOMIC = libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY | libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC + + +def created(p): + xml, flags = p._snapshotCreateXML.call_args[0] + return xml, flags + + +class CreateTestCase(unittest.TestCase): + def test_disk_only_snapshot(self): + p = proxy() + p.create_external_snapshot("snap-1", "desc") + xml, flags = created(p) + self.assertEqual(flags, DISK_ONLY_ATOMIC) + self.assertIn("snap-1", xml) + self.assertNotIn(" & 'y'") + xml, _ = created(p) + self.assertIn("<x> &", xml) + self.assertNotIn("", xml) + + def test_memory_snapshot_goes_into_the_first_disks_pool(self): + p = proxy() + p.get_volume_by_path = MagicMock() + pool = p.get_volume_by_path.return_value.storagePoolLookupByVolume.return_value + pool.info.return_value = [2, 0, 0, 2 * 1024**3] + p.create_external_snapshot("m", memory=True) + xml, flags = created(p) + p.get_volume_by_path.assert_called_once_with("/pool/a.qcow2") + self.assertEqual(flags, libvirt.VIR_DOMAIN_SNAPSHOT_CREATE_ATOMIC) + self.assertIn( + "", xml + ) + + def refused(self, p, pattern, **kwargs): + with self.assertRaisesRegex(util.OperationError, pattern): + p.create_external_snapshot(kwargs.pop("name", "s"), **kwargs) + p._snapshotCreateXML.assert_not_called() + + def test_invalid_names_are_refused(self): + for name in ("", ".hidden", "a/b", "a b", "x" * 65, ""): + with self.subTest(name=name): + self.refused(proxy(), "Snapshot names", name=name) + + def test_names_are_checked_only_on_create(self): + # snapshots made by the old code are named s1. and stay usable + p = proxy(external=[fake_snap("s1.old", current=True)]) + p.instance.revertToSnapshot = MagicMock() + p.revert_external_snapshot("s1.old") + p.instance.revertToSnapshot.assert_called_once() + + def test_a_taken_name_is_refused(self): + self.refused(proxy(external=[fake_snap("s", current=True)]), "already exists") + + def test_shared_state_checks_apply(self): + self.refused(proxy(managed_save=True), "saved state") + + def test_internal_snapshots_are_refused(self): + self.refused(proxy(internal=[fake_snap("i")]), "internal snapshots") + + def test_newer_snapshots_after_a_revert_are_refused(self): + self.refused(proxy(external=[fake_snap("old", current=True, children=1)]), "Newer snapshots") + + def test_snapshots_without_a_current_one_are_refused(self): + # a create would start a second root + self.refused(proxy(external=[fake_snap("old")]), "revert to a snapshot first") + + def test_a_disk_that_is_not_a_file_is_refused(self): + xml = "" + self.refused(proxy(xml=xml), "vda is not a file") + + def test_cdrom_with_media_is_refused(self): + xml = DOMAIN.replace("", "/ovmf/CODE.fd") + snap = fake_snap("s1.old", current=True) + snap.getXMLDesc.return_value = snapshot_xml("s1.old").replace( + "", "/ovmf/CODE.fd" + ) + p = proxy(external=[snap], xml=uefi) + with self.assertRaisesRegex(util.OperationError, "older version"): + p.revert_external_snapshot("s1.old") + p.instance.revertToSnapshot.assert_not_called() + + + def test_an_old_running_uefi_snapshot_is_judged_by_its_inactive_definition(self): + # a running VM's snapshot keeps the live definition (pflash) and the + # persistent one (rom); the revert restores the persistent one + uefi = DOMAIN.replace("", "/ovmf/CODE.fd") + snap = fake_snap("s1.old", current=True) + snap.getXMLDesc.return_value = snapshot_xml("s1.old").replace( + "", "/ovmf/CODE.fd" + ).replace( + "", + "/ovmf/CODE.fd" + "", + ) + p = proxy(external=[snap], xml=uefi) + with self.assertRaisesRegex(util.OperationError, "older version"): + p.revert_external_snapshot("s1.old") + p.instance.revertToSnapshot.assert_not_called() + + def test_a_failed_retry_still_reports_the_vm_state(self): + # the collision already stopped the VM; another error on the retry + # must not hide that + p = proxy(external=[fake_snap("s")]) + p.instance.revertToSnapshot.side_effect = [self.collision(), libvirtError("disk busy")] + p.instance.isActive.return_value = False + with unittest.mock.patch("vrtManager.instance.time.sleep"): + with self.assertRaisesRegex(util.OperationError, "disk busy.*The VM is shut off"): + p.revert_external_snapshot("s") + + +class DeleteTestCase(unittest.TestCase): + def test_delete_is_one_libvirt_call(self): + snap = fake_snap("s", current=True) + p = proxy(external=[snap]) + p.delete_external_snapshot("s") + snap.delete.assert_called_once_with(0) + p.refresh_instance_pools.assert_called_once() + + def test_running_job_is_refused(self): + snap = fake_snap("s") + p = proxy(external=[snap], jobs={"vda": {"cur": 1, "end": 2}}) + with self.assertRaisesRegex(util.OperationError, "still running"): + p.delete_external_snapshot("s") + snap.delete.assert_not_called() + + +class KindTestCase(unittest.TestCase): + def test_internal_endpoints_refuse_external_names(self): + snap = fake_snap("e") + p = proxy(external=[snap]) + for call in (p.snapshot_revert, p.snapshot_delete): + with self.subTest(call.__name__), self.assertRaises(util.OperationError): + call("e") + snap.delete.assert_not_called() + p.instance.revertToSnapshot.assert_not_called() + + def test_external_endpoints_refuse_internal_names(self): + snap = fake_snap("i") + p = proxy(internal=[snap]) + for call in (p.revert_external_snapshot, p.delete_external_snapshot): + with self.subTest(call.__name__), self.assertRaises(util.OperationError): + call("i") + snap.delete.assert_not_called() + + def test_internal_create_refuses_a_vm_with_external_snapshots(self): + p = proxy(external=[fake_snap("e")]) + with self.assertRaisesRegex(util.OperationError, "external snapshots"): + p.create_snapshot("i") + p._snapshotCreateXML.assert_not_called() + + +if __name__ == "__main__": + unittest.main() diff --git a/vrtManager/tests/test_xml_safety.py b/vrtManager/tests/test_xml_safety.py index 3703963f..06bca9ee 100644 --- a/vrtManager/tests/test_xml_safety.py +++ b/vrtManager/tests/test_xml_safety.py @@ -55,28 +55,10 @@ class TestXmlSafety(unittest.TestCase): with self.assertRaises(ValueError): wn.set_qos("inbound", "not_a_number", 200, 300) - def test_snapshot_escaping_and_recovery_on_failure(self): - inst = wvmInstance.__new__(wvmInstance) - inst.get_status = MagicMock(return_value=5) # shutoff - inst.change_snapshot_xml = MagicMock() - inst.recover_snapshot_xml = MagicMock() - inst._XMLDesc = MagicMock(return_value="") - inst._snapshotCreateXML = MagicMock(side_effect=RuntimeError("libvirt exploded")) - - bad_name = "snap"" - bad_desc = "desc&" - - with self.assertRaises(RuntimeError): - inst.create_snapshot(bad_name, bad_desc) - - inst.change_snapshot_xml.assert_not_called() - inst.recover_snapshot_xml.assert_not_called() - def test_snapshot_xml_payload_is_escaped(self): inst = wvmInstance.__new__(wvmInstance) + inst.instance = MagicMock(**{"snapshotListNames.return_value": []}) # no snapshots yet inst.get_status = MagicMock(return_value=5) # shutoff - inst.change_snapshot_xml = MagicMock() - inst.recover_snapshot_xml = MagicMock() inst._XMLDesc = MagicMock(return_value="") inst._snapshotCreateXML = MagicMock(return_value=None) @@ -92,73 +74,20 @@ class TestXmlSafety(unittest.TestCase): self.assertIn("<inject>&quotes", created_xml) self.assertIn("<tag>&and", created_xml) - def test_external_snapshot_recovery(self): - inst = wvmInstance.__new__(wvmInstance) - inst.get_status = MagicMock(return_value=1) - inst.change_snapshot_xml = MagicMock() - inst.recover_snapshot_xml = MagicMock() - inst._XMLDesc = MagicMock(return_value="") - inst._snapshotCreateXML = MagicMock(return_value=None) - inst.refresh_instance_pools = MagicMock() - - inst.create_external_snapshot("ext_snap_1", desc="normal snapshot") - - inst.change_snapshot_xml.assert_called_once() - inst.recover_snapshot_xml.assert_called_once() - inst.refresh_instance_pools.assert_called_once() - - def test_secure_boot_loader_change_and_recovery(self): - inst = wvmInstance.__new__(wvmInstance) - inst.wvm = MagicMock() - uefi_xml = "/usr/share/OVMF/OVMF.fd" - inst._XMLDesc = MagicMock(return_value=uefi_xml) - - inst.change_snapshot_xml() - inst.wvm.defineXML.assert_called_once() - changed_xml = inst.wvm.defineXML.call_args[0][0] - self.assertIn("type=\"rom\"", changed_xml) - self.assertIn("secure=\"yes\"", changed_xml) - - # Test recovery from rom back to pflash - inst._XMLDesc = MagicMock(return_value=changed_xml) - inst.wvm.defineXML.reset_mock() - inst.recover_snapshot_xml() - inst.wvm.defineXML.assert_called_once() - recovered_xml = inst.wvm.defineXML.call_args[0][0] - self.assertIn("type=\"pflash\"", recovered_xml) - self.assertIn("secure=\"yes\"", recovered_xml) - def test_original_rom_loader_preserved(self): inst = wvmInstance.__new__(wvmInstance) + inst.instance = MagicMock(**{"snapshotListNames.return_value": []}) # no snapshots yet inst._XMLDesc = MagicMock(return_value="/usr/share/rom.bin") inst._defineXML = MagicMock() inst._snapshotCreateXML = MagicMock(return_value=None) inst.get_status = MagicMock(return_value=5) - # change_snapshot_xml should return False and not define XML - self.assertFalse(inst.change_snapshot_xml()) - inst._defineXML.assert_not_called() - inst.create_snapshot("snap_rom", "testing rom preservation") inst._defineXML.assert_not_called() - def test_malformed_xml_raises_in_change(self): - inst = wvmInstance.__new__(wvmInstance) - inst._XMLDesc = MagicMock(return_value="/path") - inst._defineXML = MagicMock(side_effect=RuntimeError("libvirt defineXML failed")) - with self.assertRaises(RuntimeError): - inst.recover_snapshot_xml() - - def test_snapshot_revert_rom_loader_preserved(self): inst = wvmInstance.__new__(wvmInstance) - inst.instance = MagicMock() + inst.instance = MagicMock(**{"snapshotListNames.return_value": ["snap_rom"]}) mock_snap = MagicMock() inst.instance.snapshotLookupByName = MagicMock(return_value=mock_snap) inst.instance.revertToSnapshot = MagicMock() @@ -173,6 +102,7 @@ class TestXmlSafety(unittest.TestCase): def test_internal_snapshot_never_rewrites_a_pflash_loader(self): # libvirt decides; the definition is not switched to rom. inst = wvmInstance.__new__(wvmInstance) + inst.instance = MagicMock(**{"snapshotListNames.return_value": []}) # no snapshots yet inst.get_status = MagicMock(return_value=5) inst._XMLDesc = MagicMock( return_value="/usr/share/OVMF/OVMF.fd"