mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 11:16:47 +00:00
fix(migration): remove the NVRAM file a live migration leaves on the source
libvirt undefines the source domain after a migration but keeps its NVRAM file. A VM created later on that host with the same name reuses the file and starts with the old VM's UEFI variables (boot entries, enrolled keys): checked on libvirt 11.10, where only VIR_DOMAIN_START_RESET_NVRAM recreates it from the template. After a successful migration the file is now deleted on the source, as a volume of the pool on libvirt's NVRAM directory or of a transient one. Only a file in /var/lib/libvirt/qemu/nvram, and only when the destination cannot create a marker file the source just created there (an exclusive create also fails on shared storage whose listing an NFS client caches). A failed removal is logged with the migration, which stands and is recorded.
This commit is contained in:
catborise
parent
e14a4cc10d
commit
975b7f0baf
6 changed files with 345 additions and 2 deletions
|
|
@ -534,6 +534,7 @@ A rollback leaves the clone on a detached HEAD at the restored commit; the updat
|
|||
- A VM that vanishes from its compute keeps its owners for `INSTANCE_OWNERSHIP_RETENTION_DAYS` (30) in case the same UUID comes back; superusers list and remove these records on the "Removed VMs" admin page.
|
||||
- The log rotates (10 MB × 5) in `data/webvirtcloud.log`.
|
||||
- A disk can be grown while its VM runs or is paused, not only when it is shut off.
|
||||
- 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).
|
||||
- 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).
|
||||
- Docker no longer publishes port 6080; the console goes through nginx on the panel's port (`WS_PUBLIC_PORT`, default 80).
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@ from computes.models import Compute
|
|||
from django.contrib.auth import get_user_model
|
||||
from django.test import TestCase
|
||||
from django.urls import reverse
|
||||
from libvirt import libvirtError
|
||||
from logs.models import Logs
|
||||
from rest_framework.test import APIClient
|
||||
|
||||
|
|
@ -22,11 +23,18 @@ class MigrationLogTests(TestCase):
|
|||
self.dest = Compute.objects.create(name="ml-dst", hostname="198.51.100.9", login="root", password="", type=2)
|
||||
self.vm = Instance.objects.create(compute=self.source, name="vm", uuid="u-ml")
|
||||
|
||||
def migrate(self, outcome, **request):
|
||||
def migrate(self, outcome, nvram=None, remove_nvram=True, **request):
|
||||
self.proxy = MagicMock()
|
||||
self.proxy.get_autostart.return_value = False
|
||||
self.proxy.get_nvram.return_value = nvram
|
||||
if isinstance(remove_nvram, Exception):
|
||||
self.proxy.remove_nvram.side_effect = remove_nvram
|
||||
else:
|
||||
self.proxy.remove_nvram.return_value = remove_nvram
|
||||
with patch.object(Compute, "status", True), \
|
||||
patch("instances.utils.wvmInstances") as instances, \
|
||||
patch("instances.utils.wvmInstance"), \
|
||||
patch.object(Instance, "proxy", MagicMock()), \
|
||||
patch.object(Instance, "proxy", self.proxy), \
|
||||
patch.object(Instance, "autostart", False):
|
||||
if isinstance(outcome, Exception):
|
||||
instances.return_value.moveto.side_effect = outcome
|
||||
|
|
@ -34,6 +42,40 @@ class MigrationLogTests(TestCase):
|
|||
instances.return_value.moveto.return_value = outcome
|
||||
return migrate_instance(self.dest, self.vm, self.admin, **request)
|
||||
|
||||
def test_the_nvram_file_left_on_the_source_is_removed(self):
|
||||
self.migrate("live", nvram="/var/lib/libvirt/qemu/nvram/vm_VARS.fd", live=True)
|
||||
path, destination = self.proxy.remove_nvram.call_args[0]
|
||||
self.assertEqual(path, "/var/lib/libvirt/qemu/nvram/vm_VARS.fd")
|
||||
self.assertIsNotNone(destination)
|
||||
self.assertEqual(Logs.objects.get().message, "Instance is migrated(live) to 198.51.100.9")
|
||||
|
||||
def test_a_vm_without_nvram_removes_nothing(self):
|
||||
self.migrate("live", live=True)
|
||||
self.assertFalse(self.proxy.remove_nvram.called)
|
||||
|
||||
def test_a_failed_nvram_removal_is_logged_but_the_migration_stands(self):
|
||||
self.migrate("live", nvram="/var/lib/libvirt/qemu/nvram/vm_VARS.fd",
|
||||
remove_nvram=libvirtError("cannot delete"), live=True)
|
||||
self.assertEqual(
|
||||
Logs.objects.get().message,
|
||||
"Instance is migrated(live) to 198.51.100.9; removing its NVRAM file "
|
||||
"/var/lib/libvirt/qemu/nvram/vm_VARS.fd on the source failed: cannot delete",
|
||||
)
|
||||
self.vm.refresh_from_db()
|
||||
self.assertEqual(self.vm.compute, self.dest)
|
||||
|
||||
def test_any_nvram_removal_error_still_records_the_migration(self):
|
||||
self.migrate("live", nvram="/var/lib/libvirt/qemu/nvram/vm_VARS.fd",
|
||||
remove_nvram=AttributeError("unexpected"), live=True)
|
||||
self.vm.refresh_from_db()
|
||||
self.assertEqual(self.vm.compute, self.dest)
|
||||
self.assertIn("on the source failed: unexpected", Logs.objects.get().message)
|
||||
|
||||
def test_a_failed_migration_removes_nothing(self):
|
||||
with self.assertRaises(util.OperationError):
|
||||
self.migrate(util.OperationError("no"), nvram="/var/lib/libvirt/qemu/nvram/vm_VARS.fd", live=True)
|
||||
self.assertFalse(self.proxy.remove_nvram.called)
|
||||
|
||||
def test_success_names_the_mode_used(self):
|
||||
self.migrate("non-live, compressed", live=False, compress=True)
|
||||
log = Logs.objects.get()
|
||||
|
|
|
|||
|
|
@ -190,6 +190,7 @@ def migrate_instance(
|
|||
)
|
||||
|
||||
autostart = instance.proxy.get_autostart()
|
||||
nvram = instance.proxy.get_nvram()
|
||||
mode = conn_migrate.moveto(
|
||||
instance.proxy,
|
||||
instance.name,
|
||||
|
|
@ -236,6 +237,25 @@ def migrate_instance(
|
|||
if conn_new is not None:
|
||||
conn_new.close()
|
||||
|
||||
# libvirt keeps the NVRAM file on the source; a VM created there
|
||||
# later with the same name would start with these variables.
|
||||
if nvram:
|
||||
conn_new = None
|
||||
try:
|
||||
conn_new = wvmInstances(
|
||||
new_compute.hostname, new_compute.login, new_compute.password, new_compute.type
|
||||
)
|
||||
instance.proxy.remove_nvram(nvram, conn_new)
|
||||
except Exception as error: # the migration stands and is recorded
|
||||
msg = _("%(msg)s; removing its NVRAM file %(path)s on the source failed: %(error)s") % {
|
||||
"msg": msg,
|
||||
"path": nvram,
|
||||
"error": error,
|
||||
}
|
||||
finally:
|
||||
if conn_new is not None:
|
||||
conn_new.close()
|
||||
|
||||
try:
|
||||
record_migration(instance, new_compute)
|
||||
except Exception as error:
|
||||
|
|
|
|||
|
|
@ -1,8 +1,10 @@
|
|||
import contextlib
|
||||
import os
|
||||
import re
|
||||
import socket
|
||||
import threading
|
||||
import time
|
||||
import uuid
|
||||
from urllib.parse import quote
|
||||
|
||||
import libvirt
|
||||
|
|
@ -87,6 +89,10 @@ def _after_fork_in_child():
|
|||
os.register_at_fork(after_in_child=_after_fork_in_child)
|
||||
|
||||
|
||||
# libvirt's per-host directory of the VMs' UEFI variables (NVRAM)
|
||||
NVRAM_DIR = "/var/lib/libvirt/qemu/nvram"
|
||||
|
||||
|
||||
class wvmConnection(object):
|
||||
"""
|
||||
class representing a single connection stored in the Connection Manager
|
||||
|
|
@ -832,6 +838,66 @@ class wvmConnect(object):
|
|||
def get_volume_by_path(self, path):
|
||||
return self.wvm.storageVolLookupByPath(path)
|
||||
|
||||
def remove_nvram(self, path, destination):
|
||||
"""Deletes the NVRAM file of a VM that migrated from this host to
|
||||
destination (a wvmConnect). Only a file in libvirt's per-host NVRAM
|
||||
directory, and only when destination does not see that directory too
|
||||
(shared storage would hold the migrated VM's own file). Returns
|
||||
whether a file was deleted."""
|
||||
if os.path.dirname(path) != NVRAM_DIR:
|
||||
return False
|
||||
marker = (
|
||||
f"<volume><name>webvirtcloud-check-{uuid.uuid4().hex}</name><capacity>0</capacity>"
|
||||
"<target><format type='raw'/></target></volume>"
|
||||
)
|
||||
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)
|
||||
vol = self._find_volume(pool, os.path.basename(path))
|
||||
if vol is None:
|
||||
return False
|
||||
vol.delete(0)
|
||||
return True
|
||||
|
||||
@contextlib.contextmanager
|
||||
def _nvram_pool(self):
|
||||
"""An active, refreshed pool on NVRAM_DIR: libvirt reads and deletes
|
||||
files only as volumes of a pool. A transient one when none exists."""
|
||||
for pool in self.wvm.listAllStoragePools(libvirt.VIR_CONNECT_LIST_STORAGE_POOLS_ACTIVE):
|
||||
# a pool without a target path (rbd, gluster, ...) holds no files
|
||||
if (util.get_xml_path(pool.XMLDesc(0), "/pool/target/path") or "").rstrip("/") == NVRAM_DIR:
|
||||
pool.refresh(0)
|
||||
yield pool
|
||||
return
|
||||
pool = self.wvm.storagePoolCreateXML(
|
||||
f"<pool type='dir'><name>webvirtcloud-nvram-{uuid.uuid4().hex[:8]}</name>"
|
||||
f"<target><path>{NVRAM_DIR}</path></target></pool>",
|
||||
0,
|
||||
)
|
||||
try:
|
||||
yield pool
|
||||
finally:
|
||||
pool.destroy()
|
||||
|
||||
@staticmethod
|
||||
def _find_volume(pool, name):
|
||||
try:
|
||||
return pool.storageVolLookupByName(name)
|
||||
except libvirtError as err:
|
||||
if err.get_error_code() == libvirt.VIR_ERR_NO_STORAGE_VOL:
|
||||
return None
|
||||
raise
|
||||
|
||||
def get_network(self, net):
|
||||
return self.wvm.networkLookupByName(net)
|
||||
|
||||
|
|
|
|||
88
vrtManager/test_live_nvram.py
Normal file
88
vrtManager/test_live_nvram.py
Normal file
|
|
@ -0,0 +1,88 @@
|
|||
"""Live check that the NVRAM file a migrated VM leaves behind is removed,
|
||||
and kept when the destination sees the same directory.
|
||||
|
||||
Runs only with TEST_LIBVIRT_HOST set (see instances/livetest.py). The
|
||||
deletion needs a second host: TEST_LIBVIRT_SSH_HOST (and _LOGIN).
|
||||
"""
|
||||
|
||||
import os
|
||||
import unittest
|
||||
|
||||
import libvirt
|
||||
from django.test import SimpleTestCase
|
||||
from lxml import etree
|
||||
|
||||
from instances import livetest
|
||||
from vrtManager.connection import CONN_SSH, NVRAM_DIR, connection_manager, wvmConnect
|
||||
|
||||
NAME = livetest.PREFIX + "nvram"
|
||||
PATH = f"{NVRAM_DIR}/{NAME}_VARS.fd"
|
||||
|
||||
|
||||
class LiveRemoveNvramTestCase(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)
|
||||
cls.inventory = livetest.inventory(cls.conn)
|
||||
cls.pools = {p.name() for p in cls.conn.listAllStoragePools()}
|
||||
super().setUpClass()
|
||||
|
||||
@classmethod
|
||||
def tearDownClass(cls):
|
||||
livetest.cleanup(cls.conn)
|
||||
super().tearDownClass()
|
||||
if livetest.inventory(cls.conn) != cls.inventory:
|
||||
raise AssertionError("Live tests changed objects outside the test namespace")
|
||||
if {p.name() for p in cls.conn.listAllStoragePools()} != cls.pools:
|
||||
raise AssertionError("A storage pool was left behind")
|
||||
|
||||
def leave_nvram(self):
|
||||
"""What a migration with VIR_MIGRATE_UNDEFINE_SOURCE leaves: the file
|
||||
without a domain."""
|
||||
dom = self.conn.defineXML(
|
||||
f"<domain type='kvm'><name>{NAME}</name><memory unit='MiB'>128</memory><vcpu>1</vcpu>"
|
||||
"<os firmware='efi'><type arch='x86_64' machine='q35'>hvm</type></os>"
|
||||
"<features><acpi/></features><devices/></domain>"
|
||||
)
|
||||
dom.createWithFlags(libvirt.VIR_DOMAIN_START_PAUSED) # creates the file
|
||||
self.assertEqual(etree.fromstring(dom.XMLDesc(0)).findtext("os/nvram"), PATH)
|
||||
dom.destroy()
|
||||
dom.undefineFlags(libvirt.VIR_DOMAIN_UNDEFINE_KEEP_NVRAM)
|
||||
|
||||
def tearDown(self):
|
||||
livetest.cleanup(self.conn)
|
||||
host = wvmConnect(*self.args)
|
||||
with host._nvram_pool() as pool:
|
||||
vol = host._find_volume(pool, os.path.basename(PATH))
|
||||
if vol is not None:
|
||||
vol.delete(0)
|
||||
|
||||
def exists(self):
|
||||
host = wvmConnect(*self.args)
|
||||
with host._nvram_pool() as pool:
|
||||
return host._find_volume(pool, os.path.basename(PATH)) is not None
|
||||
|
||||
def test_a_directory_both_connections_see_is_kept(self):
|
||||
self.leave_nvram()
|
||||
self.assertFalse(wvmConnect(*self.args).remove_nvram(PATH, wvmConnect(*self.args)))
|
||||
self.assertTrue(self.exists())
|
||||
|
||||
def test_the_file_is_deleted_when_the_destination_is_another_host(self):
|
||||
other_host = os.environ.get("TEST_LIBVIRT_SSH_HOST")
|
||||
if not other_host or other_host == self.args[0]:
|
||||
self.skipTest("Set TEST_LIBVIRT_SSH_HOST to a second host")
|
||||
other = wvmConnect(other_host, os.environ.get("TEST_LIBVIRT_SSH_LOGIN", "root"), "", CONN_SSH)
|
||||
other_pools = {p.name() for p in other.wvm.listAllStoragePools()}
|
||||
self.leave_nvram()
|
||||
self.assertTrue(wvmConnect(*self.args).remove_nvram(PATH, other))
|
||||
self.assertFalse(self.exists())
|
||||
self.assertEqual({p.name() for p in other.wvm.listAllStoragePools()}, other_pools)
|
||||
126
vrtManager/test_remove_nvram.py
Normal file
126
vrtManager/test_remove_nvram.py
Normal file
|
|
@ -0,0 +1,126 @@
|
|||
"""After a migration took a VM away, its NVRAM file is removed from the
|
||||
source when it is in libvirt's per-host NVRAM directory and the destination
|
||||
does not share that directory; a VM later created on the source with the same
|
||||
name would otherwise start with the old UEFI variables."""
|
||||
|
||||
from unittest import TestCase
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
import libvirt
|
||||
|
||||
from vrtManager.connection import NVRAM_DIR, wvmConnect
|
||||
|
||||
PATH = NVRAM_DIR + "/vm_VARS.fd"
|
||||
|
||||
|
||||
def error(code):
|
||||
err = libvirt.libvirtError("error %d" % code)
|
||||
err.err = (code, libvirt.VIR_FROM_STORAGE, str(err), 2, None, None, None, 0, 0)
|
||||
return err
|
||||
|
||||
|
||||
def pool(path, volumes=(), create_error=None):
|
||||
p = MagicMock()
|
||||
p.XMLDesc.return_value = f"<pool type='dir'><name>p</name><target><path>{path}</path></target></pool>"
|
||||
p.vols = {name: MagicMock() for name in volumes}
|
||||
p.created = []
|
||||
|
||||
def lookup(name):
|
||||
if name not in p.vols:
|
||||
raise error(libvirt.VIR_ERR_NO_STORAGE_VOL)
|
||||
return p.vols[name]
|
||||
|
||||
def create(xml, flags):
|
||||
if create_error is not None:
|
||||
raise create_error
|
||||
vol = MagicMock()
|
||||
p.created.append(vol)
|
||||
return vol
|
||||
|
||||
p.storageVolLookupByName.side_effect = lookup
|
||||
p.createXML.side_effect = create
|
||||
return p
|
||||
|
||||
|
||||
def connect(pools=(), transient=None):
|
||||
conn = wvmConnect.__new__(wvmConnect)
|
||||
conn.wvm = MagicMock()
|
||||
conn.wvm.listAllStoragePools.return_value = list(pools)
|
||||
conn.wvm.storagePoolCreateXML.return_value = transient
|
||||
return conn
|
||||
|
||||
|
||||
def local_destination():
|
||||
return connect([pool(NVRAM_DIR)])
|
||||
|
||||
|
||||
class RemoveNvramTestCase(TestCase):
|
||||
def test_a_file_outside_the_nvram_directory_is_kept(self):
|
||||
conn = connect()
|
||||
for path in ("/srv/shared/nvram/vm_VARS.fd", NVRAM_DIR + "/sub/vm_VARS.fd", "relative_VARS.fd"):
|
||||
with self.subTest(path=path):
|
||||
self.assertFalse(conn.remove_nvram(path, local_destination()))
|
||||
self.assertFalse(conn.wvm.method_calls)
|
||||
|
||||
def test_a_pool_on_the_directory_is_used(self):
|
||||
existing = pool(NVRAM_DIR + "/", ["vm_VARS.fd"])
|
||||
conn = connect([pool("/var/lib/libvirt/images"), existing])
|
||||
self.assertTrue(conn.remove_nvram(PATH, local_destination()))
|
||||
existing.vols["vm_VARS.fd"].delete.assert_called_once_with(0)
|
||||
self.assertFalse(conn.wvm.storagePoolCreateXML.called)
|
||||
self.assertFalse(existing.destroy.called)
|
||||
|
||||
def test_pools_without_a_target_path_are_skipped(self):
|
||||
rbd = MagicMock()
|
||||
rbd.XMLDesc.return_value = "<pool type='rbd'><name>ceph</name><source><name>rbd</name></source></pool>"
|
||||
transient = pool(NVRAM_DIR, ["vm_VARS.fd"])
|
||||
conn = connect([rbd], transient)
|
||||
self.assertTrue(conn.remove_nvram(PATH, local_destination()))
|
||||
transient.vols["vm_VARS.fd"].delete.assert_called_once_with(0)
|
||||
|
||||
def test_without_a_pool_a_transient_one_is_used_and_stopped(self):
|
||||
transient = pool(NVRAM_DIR, ["vm_VARS.fd"])
|
||||
conn = connect([pool("/var/lib/libvirt/images")], transient)
|
||||
self.assertTrue(conn.remove_nvram(PATH, local_destination()))
|
||||
self.assertIn(f"<path>{NVRAM_DIR}</path>", conn.wvm.storagePoolCreateXML.call_args[0][0])
|
||||
transient.vols["vm_VARS.fd"].delete.assert_called_once_with(0)
|
||||
transient.destroy.assert_called_once_with()
|
||||
|
||||
def test_a_directory_the_destination_shares_is_left_alone(self):
|
||||
# the destination cannot create the marker the source just created
|
||||
source = pool(NVRAM_DIR, ["vm_VARS.fd"])
|
||||
destination = connect([pool(NVRAM_DIR, create_error=error(libvirt.VIR_ERR_STORAGE_VOL_EXIST))])
|
||||
self.assertFalse(connect([source]).remove_nvram(PATH, destination))
|
||||
self.assertFalse(source.vols["vm_VARS.fd"].delete.called)
|
||||
source.created[0].delete.assert_called_once_with(0) # the marker is gone
|
||||
|
||||
def test_both_markers_are_removed(self):
|
||||
source = pool(NVRAM_DIR, ["vm_VARS.fd"])
|
||||
other = pool(NVRAM_DIR)
|
||||
self.assertTrue(connect([source]).remove_nvram(PATH, connect([other])))
|
||||
self.assertEqual(source.createXML.call_args, other.createXML.call_args) # one name
|
||||
source.created[0].delete.assert_called_once_with(0)
|
||||
other.created[0].delete.assert_called_once_with(0)
|
||||
|
||||
def test_when_the_destination_cannot_be_checked_nothing_is_deleted(self):
|
||||
source = pool(NVRAM_DIR, ["vm_VARS.fd"])
|
||||
destination = connect([], None)
|
||||
destination.wvm.storagePoolCreateXML.side_effect = error(libvirt.VIR_ERR_INTERNAL_ERROR)
|
||||
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)
|
||||
|
||||
def test_the_transient_pool_is_stopped_when_the_delete_fails(self):
|
||||
transient = pool(NVRAM_DIR, ["vm_VARS.fd"])
|
||||
transient.vols["vm_VARS.fd"].delete.side_effect = error(libvirt.VIR_ERR_INTERNAL_ERROR)
|
||||
conn = connect([], transient)
|
||||
with self.assertRaises(libvirt.libvirtError):
|
||||
conn.remove_nvram(PATH, local_destination())
|
||||
transient.destroy.assert_called_once_with()
|
||||
|
||||
def test_a_missing_file_is_nothing_to_remove(self):
|
||||
transient = pool(NVRAM_DIR)
|
||||
conn = connect([], transient)
|
||||
self.assertFalse(conn.remove_nvram(PATH, local_destination()))
|
||||
transient.destroy.assert_called_once_with()
|
||||
Loading…
Add table
Add a link
Reference in a new issue