diff --git a/README.md b/README.md index 1e480a96..cd515be4 100644 --- a/README.md +++ b/README.md @@ -593,7 +593,7 @@ When the restored version predates the worker, `supervisorctl` reports that `web - Cloning a UEFI VM no longer defines a storage pool on the host's NVRAM directory; the clone gets its own copy of the VM's UEFI variables. - The console .vv file finds the VM by its UUID; without a listen address or password it gives 127.0.0.1 and an empty password instead of "None". - A running VM without a guest agent shows the IPv4 address the host learned by ARP (the lookup always failed before). -- After a live migration the VM's NVRAM file is deleted on the source, when it is in libvirt's `/var/lib/libvirt/qemu/nvram` and the destination does not share that directory (a VM created there later with the same name would start with those UEFI variables). +- After a migration the VM's NVRAM file is deleted on the source, when it is in libvirt's `/var/lib/libvirt/qemu/nvram` and the destination does not share that directory (a VM created there later with the same name would start with those UEFI variables). - 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. @@ -606,6 +606,7 @@ When the restored version predates the worker, `supervisorctl` reports that `web - Migrating a VM runs as a job: the page returns at once, the job panel shows its progress, and a running migration can be cancelled there (the VM keeps running on its source host). The 540 s limit is gone. The API's migrate answers 202 with the job and needs a new `submission_key` per request. - Uploading an ISO is tracked as a job: the storage page's job panel shows it and can cancel it, and the page no longer reloads when it ends. The upload dialog closes once the upload starts; the browser still sends the file, so stay on the page until it ends. An upload left idle for an hour is ended and its partial file deleted. - A running or paused VM can be cloned: libvirt copies its disks from one instant while it keeps running (file systems are frozen for a moment when the guest agent is ready). It needs libvirt 11.10 or later on the host, local file disks and no TPM; the VM cannot be changed from the panel until the copy ends, and the copy can be cancelled. Cancelling a clone of a shut-off VM has no effect: it shows as cancelling until the copy ends. A target pool that fills up during the copy can pause the source VM; a clone that ends uncertain keeps the VM locked until a superuser resolves the job. +- A shut-off UEFI VM can be migrated: its NVRAM file is copied to the destination host (unless both hosts share the NVRAM directory). A shut-off VM with TPM state still has to be started and migrated live. - Manual installs that keep their own `settings.py` copy the SQLite `OPTIONS` and `TEST` entries from `settings.py.template` and add `"operations"` to `INSTALLED_APPS`. - The SQLite database must be on a local file system (WAL). - Outside compose, stop the Docker container with `docker stop -t 95` so the worker can finish running jobs. diff --git a/instances/tests/test_live_jobs.py b/instances/tests/test_live_jobs.py index 56ae67e4..57fc0f3d 100644 --- a/instances/tests/test_live_jobs.py +++ b/instances/tests/test_live_jobs.py @@ -9,11 +9,12 @@ import os import time import unittest +import libvirt from django.contrib.auth import get_user_model from django.core.files.uploadedfile import SimpleUploadedFile from django.test import TransactionTestCase from django.urls import reverse -from vrtManager.connection import connection_manager +from vrtManager.connection import connection_manager, wvmConnect from vrtManager.instance import wvmInstance from appsettings.settings import get_settings @@ -144,7 +145,7 @@ class LiveJobsTestCase(TransactionTestCase): "machine": "q35"}, "key-smoke003")) self.assertEqual(self.conn.lookupByUUIDString(op.intent["uuid"]).name(), P + "smoke-new") - def test_a_migration_runs_as_a_job_there_and_back(self): + def second_host(self): if not os.environ.get("TEST_LIBVIRT_SSH_HOST"): self.skipTest("Set TEST_LIBVIRT_SSH_HOST to a second host to run the live migration test") dest = Compute.objects.create(name=P + "dest", hostname=os.environ["TEST_LIBVIRT_SSH_HOST"], @@ -154,6 +155,10 @@ class LiveJobsTestCase(TransactionTestCase): dest_inventory = livetest.inventory(dest_conn) self.addCleanup(lambda: self.assertEqual(livetest.inventory(dest_conn), dest_inventory)) self.addCleanup(livetest.cleanup, dest_conn) + return dest, dest_conn + + def test_a_migration_runs_as_a_job_there_and_back(self): + dest, dest_conn = self.second_host() dom = livetest.define_vm(self.conn, P + "migrate", []) dom.create() uuid = dom.UUIDString() @@ -167,6 +172,34 @@ class LiveJobsTestCase(TransactionTestCase): self.assertFalse(self.conn.lookupByUUIDString(uuid).isActive()) # defined on the source again self.assertEqual(Instance.objects.get(uuid=uuid).compute, self.compute) + def test_a_shut_off_uefi_vm_migrates_with_its_nvram_there_and_back(self): + dest, _dest_conn = self.second_host() + hosts = {self.compute.pk: wvmConnect(*self.args), dest.pk: wvmConnect(dest.hostname, dest.login, "", 2)} + dom = livetest.define_vm(self.conn, P + "uefi", [], uefi=True) + dom.createWithFlags(libvirt.VIR_DOMAIN_START_PAUSED) # libvirt makes the NVRAM file + dom.destroy() + name = os.path.basename(wvmInstance(*self.args, None, uuid=dom.UUIDString()).get_nvram()) + + def nvram(compute): + host = hosts[compute.pk] + with host._nvram_pool() as pool: + vol = host._find_volume(pool, name) + if vol is None: + return None + stream, chunks = host.wvm.newStream(0), [] + vol.download(stream, 0, 0, 0) + stream.recvAll(lambda _stream, data, out: out.append(data), chunks) + stream.finish() + return b"".join(chunks) + + content = nvram(self.compute) + instance = Instance.objects.create(compute=self.compute, name=dom.name(), uuid=dom.UUIDString()) + for n, (source, target) in enumerate(((self.compute, dest), (dest, self.compute))): + offline = {"instance": instance.pk, "target": target.pk, "offline": True} + self.run_job(jobs.submit("instance.migrate", self.admin, offline, f"key-uefi{n:04d}")) + self.assertEqual(nvram(target), content) + self.assertIsNone(nvram(source)) + def upload(self, compute, key, chunks): """Post chunks of a two-chunk ISO upload to the storage view; returns the job.""" self.client.force_login(self.admin) diff --git a/vrtManager/connection.py b/vrtManager/connection.py index b12d510a..7f8c7159 100644 --- a/vrtManager/connection.py +++ b/vrtManager/connection.py @@ -869,29 +869,34 @@ class wvmConnect(object): whether a file was deleted.""" if os.path.dirname(path) != NVRAM_DIR: return False - marker = ( - f"webvirtcloud-check-{uuid.uuid4().hex}0" - "" - ) - with self._nvram_pool() as pool: - mine = pool.createXML(marker, 0) - try: - # Creating the same file on destination fails when both see one - # directory: the create is exclusive on the file server, unlike - # a directory listing, which an NFS client may cache. - with destination._nvram_pool() as other: - try: - other.createXML(marker, 0).delete(0) - except libvirtError: - return False # shared, or not known to be local - finally: - mine.delete(0) + with self._nvram_pool() as pool, destination._nvram_pool() as other: + if self._nvram_shared(pool, other): + return False vol = self._find_volume(pool, os.path.basename(path)) if vol is None: return False vol.delete(0) return True + @staticmethod + def _nvram_shared(pool, other): + """Whether pools on NVRAM_DIR of two hosts are one directory, or other + is not known to be one of its own: creating the same file in both + fails then. The create is exclusive on the file server, unlike a + directory listing, which an NFS client may cache.""" + marker = ( + f"webvirtcloud-check-{uuid.uuid4().hex}0" + "" + ) + mine = pool.createXML(marker, 0) + try: + other.createXML(marker, 0).delete(0) + except libvirtError: + return True + finally: + mine.delete(0) + return False + @contextlib.contextmanager def _nvram_pool(self): """An active, refreshed pool on NVRAM_DIR: libvirt reads and deletes diff --git a/vrtManager/instance.py b/vrtManager/instance.py index 4fdbc853..2ed7ee38 100644 --- a/vrtManager/instance.py +++ b/vrtManager/instance.py @@ -1,4 +1,5 @@ import contextlib +import io import ipaddress import json import logging @@ -18,6 +19,7 @@ try: VIR_DOMAIN_XML_SECURE, VIR_DOMAIN_XML_INACTIVE, VIR_DOMAIN_XML_MIGRATABLE, + VIR_DOMAIN_UNDEFINE_KEEP_NVRAM, VIR_ERR_NO_DOMAIN, VIR_ERR_NO_STORAGE_POOL, VIR_ERR_NO_STORAGE_VOL, @@ -158,30 +160,65 @@ class wvmInstances(wvmConnect): # Once supported hosts all run a fixed libvirt, this branch and # move_definition can go: the flags above are those for # dom.migrate. - self.move_definition(dom) + self.move_definition(dom, conn) else: dom.migrate(self.wvm, flags, None, uri, 0) return ", ".join(mode) @staticmethod def refuse_local_state(xml): - """A shut-off VM moves without its UEFI variables (NVRAM) and TPM - state, which libvirt keeps in files of the source host: on the - destination it would start with new ones (lost boot entries and - enrolled keys, a new TPM that no longer unlocks disks). A live - migration carries both.""" - tree = ElementTree.fromstring(xml) - state = [] - if tree.find("os/nvram") is not None: - state.append("NVRAM") - if tree.find("devices/tpm/backend[@type='emulator']") is not None: - state.append("TPM") - if state: + """A shut-off VM moves without its TPM state, a file of the source + host: on the destination it would get a new TPM that no longer + unlocks its disks. A live migration carries it. (The UEFI variables + are copied: copy_nvram.)""" + if ElementTree.fromstring(xml).find("devices/tpm/backend[@type='emulator']") is not None: raise util.OperationError( - f"A shut-off VM with {' and '.join(state)} state cannot be migrated: the state stays on the source " - "host. Start the VM and migrate it live." + "A shut-off VM with TPM state cannot be migrated: the state stays on the source host. " + "Start the VM and migrate it live." ) + def copy_nvram(self, conn): + """Copies the UEFI variables file of the shut-off VM of conn (on the + source) from libvirt's NVRAM directory there to the same path here, + unless both hosts share that directory. Returns the copy's name.""" + path = conn.get_nvram() + if not path or os.path.dirname(path) != NVRAM_DIR: + return None + name = os.path.basename(path) + with conn._nvram_pool() as source, self._nvram_pool() as target: + vol = self._find_volume(source, name) + if vol is None: + return None # never started: libvirt creates it at the first start + if self._nvram_shared(source, target): + if self._find_volume(target, name) is None: + raise util.OperationError(f"The destination host cannot see the NVRAM file {path}") + return None + chunks = [] + stream = conn.wvm.newStream(0) + vol.download(stream, 0, 0, 0) + stream.recvAll(lambda _stream, data, out: out.append(data), chunks) + stream.finish() + data = b"".join(chunks) + copy = target.createXML( + f"{util.xml_escape(name)}{len(data)}" + "", + 0, + ) + try: + stream = self.wvm.newStream(0) + copy.upload(stream, 0, len(data), 0) + stream.sendAll(lambda _stream, size, buffer: buffer.read(size), io.BytesIO(data)) + stream.finish() + except Exception: + copy.delete(0) + raise + return name + + def delete_nvram(self, name): + if name: + with self._nvram_pool() as pool: + pool.storageVolLookupByName(name).delete(0) + def missing_disks(self, definitions): """(target, source) of the disks in the VM definitions (XML) that are not volumes of this host's active storage pools: the VM's own files, @@ -252,10 +289,11 @@ class wvmInstances(wvmConnect): return None raise - def move_definition(self, dom): + def move_definition(self, dom, conn): """Migrates a shut-off VM: that is only its definition, as the disks - are on storage both hosts share (moveto refuses a VM with NVRAM or - TPM state). The destination's qemu migration hook does not run for it. + are on storage both hosts share, and its UEFI variables file + (copy_nvram; moveto refuses a VM with TPM state). The destination's + qemu migration hook does not run for it. Workaround, to be removed in a later version: libvirt's own offline migration (VIR_MIGRATE_OFFLINE) fails with "operation failed: domain @@ -278,9 +316,15 @@ class wvmInstances(wvmConnect): xml = dom.XMLDesc(VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_MIGRATABLE) autostart = dom.autostart() - new = self.wvm.defineXML(xml) + nvram = self.copy_nvram(conn) try: - dom.undefineFlags(0) + new = self.wvm.defineXML(xml) + except Exception: + self.delete_nvram(nvram) + raise + try: + # libvirt refuses to undefine a VM with NVRAM without this; the file stays for the cleanup + dom.undefineFlags(VIR_DOMAIN_UNDEFINE_KEEP_NVRAM) try: started = dom.isActive() except libvirtError as error: @@ -292,7 +336,8 @@ class wvmInstances(wvmConnect): dom.connect().defineXML(xml).setAutostart(autostart) raise util.OperationError("The VM was started on the source host during the migration") except Exception as error: - self.undo_definition(dom, new, error) + self.undo_definition(dom, new, error) # raises when the destination keeps the VM + self.delete_nvram(nvram) raise @staticmethod @@ -308,7 +353,7 @@ class wvmInstances(wvmConnect): f"{error}; the VM may now be defined only on the destination host: check both hosts" ) from error try: - new.undefineFlags(0) + new.undefineFlags(VIR_DOMAIN_UNDEFINE_KEEP_NVRAM) except libvirtError as undo_error: raise util.OperationError( f"{error}; the VM is now defined on both hosts, remove it from the destination: {undo_error}" @@ -447,7 +492,8 @@ class wvmInstance(wvmConnect): return util.get_xml_path(self._XMLDesc(0), "/domain/devices/emulator") def get_nvram(self): - return util.get_xml_path(self._XMLDesc(0), "/domain/os/nvram") + xml = self._XMLDesc(0) # a path as text, or a since libvirt 8.5 + return util.get_xml_path(xml, "/domain/os/nvram/source/@file") or util.get_xml_path(xml, "/domain/os/nvram") def get_loader(self): xml = self._XMLDesc(0) diff --git a/vrtManager/tests/test_migration.py b/vrtManager/tests/test_migration.py index 4b09cd31..194dbc7c 100644 --- a/vrtManager/tests/test_migration.py +++ b/vrtManager/tests/test_migration.py @@ -1,7 +1,9 @@ """Migration: the source definition never stays behind; a shut-off VM's definition is moved by the app, not by libvirt's offline migration.""" +import contextlib import unittest +from unittest import mock import libvirt from django.conf import settings @@ -156,10 +158,14 @@ class FakeHost: class FakeSource: - def __init__(self, state, **domain): + def __init__(self, state, nvram=None, **domain): self.state = state + self.nvram = nvram self.instance = FakeDomain(**domain) + def get_nvram(self): + return self.nvram + reads = 0 def get_status(self): @@ -209,23 +215,18 @@ class ShutOffMigrationTestCase(unittest.TestCase): destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) self.assertEqual(host.defined, ["vm"]) self.assertEqual(source.instance.calls, []) - self.assertEqual(source.instance.undefined, 0) + self.assertEqual(source.instance.undefined, libvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM) - def test_a_vm_with_nvram_or_tpm_state_is_refused(self): + def test_a_vm_with_tpm_state_is_refused(self): """That state stays on the source host: the VM would start on the - destination with new UEFI variables and a new TPM.""" - cases = { - "nvram": "/var/lib/libvirt/qemu/nvram/vm_VARS.fd", - "tpm": "", - } - for case, xml in cases.items(): - with self.subTest(case=case): - host = FakeHost() - source = FakeSource(5, xml=f"{xml}") - with self.assertRaisesRegex(util.OperationError, "migrate it live"): - destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) - self.assertEqual(host.defined, []) - self.assertIsNone(source.instance.undefined) + destination with a new TPM.""" + host = FakeHost() + xml = "" + source = FakeSource(5, xml=xml) + with self.assertRaisesRegex(util.OperationError, "migrate it live"): + destination(host).moveto(source, "vm", live=False, unsafe=False, offline=True) + self.assertEqual(host.defined, []) + self.assertIsNone(source.instance.undefined) def test_a_tpm_without_local_state_moves(self): xml = "" @@ -425,3 +426,48 @@ class DestinationDisksTestCase(unittest.TestCase): xml = with_disks("") with self.assertRaisesRegex(util.OperationError, "vdc \\(p/v\\)"): self.migrate(host, xml) + + +class NvramCopyTestCase(unittest.TestCase): + """A shut-off move copies the UEFI variables file to the destination.""" + + PATH = "/var/lib/libvirt/qemu/nvram/vm_VARS.fd" + + def setUp(self): + self.source = FakeSource(5, nvram=self.PATH, xml="vm") + self.source.wvm = mock.MagicMock() + self.source.wvm.newStream.return_value.recvAll.side_effect = lambda fn, out: fn(None, b"vars", out) + self.source_pool = mock.MagicMock() + self.source._nvram_pool = lambda: contextlib.nullcontext(self.source_pool) + self.host = FakeHost() + self.dest = destination(self.host) + self.dest_pool = mock.MagicMock() + self.dest._nvram_pool = lambda: contextlib.nullcontext(self.dest_pool) + self.dest.wvm.newStream = mock.MagicMock() + self.shared = mock.patch.object(wvmInstances, "_nvram_shared", return_value=False) + self.shared.start() + self.addCleanup(self.shared.stop) + + def move(self): + self.dest.moveto(self.source, "vm", live=False, unsafe=False, offline=True) + + def test_the_file_is_copied_before_the_definition(self): + self.move() + copy = self.dest_pool.createXML.return_value + self.assertIn("vm_VARS.fd4", self.dest_pool.createXML.call_args[0][0]) + copy.upload.assert_called_once_with(self.dest.wvm.newStream.return_value, 0, 4, 0) + self.assertEqual(self.host.defined, ["vm"]) + + def test_a_shared_directory_is_not_copied(self): + self.shared.stop() + with mock.patch.object(wvmInstances, "_nvram_shared", return_value=True): + self.move() + self.dest_pool.createXML.assert_not_called() + self.shared.start() + + def test_the_copy_is_deleted_when_the_definition_fails(self): + self.host.defineXML = mock.MagicMock(side_effect=libvirt.libvirtError("define failed")) + with self.assertRaisesRegex(libvirt.libvirtError, "define failed"): + self.move() + self.dest_pool.storageVolLookupByName.assert_called_once_with("vm_VARS.fd") + self.dest_pool.storageVolLookupByName.return_value.delete.assert_called_once_with(0) diff --git a/vrtManager/tests/test_remove_nvram.py b/vrtManager/tests/test_remove_nvram.py index aac09f6d..4eb544a2 100644 --- a/vrtManager/tests/test_remove_nvram.py +++ b/vrtManager/tests/test_remove_nvram.py @@ -109,7 +109,7 @@ class RemoveNvramTestCase(TestCase): with self.assertRaises(libvirt.libvirtError): connect([source]).remove_nvram(PATH, destination) self.assertFalse(source.vols["vm_VARS.fd"].delete.called) - source.created[0].delete.assert_called_once_with(0) + self.assertEqual(source.created, []) # both pools are opened before the check def test_the_transient_pool_is_stopped_when_the_delete_fails(self): transient = pool(NVRAM_DIR, ["vm_VARS.fd"])