diff --git a/build.js b/build.js index 33d4be5ba..9164e9ab4 100755 --- a/build.js +++ b/build.js @@ -84,6 +84,7 @@ const context = await esbuild.context({ loader: { ".js": "jsx", ".py": "text", + ".sh": "text", }, metafile: !!args.metafile, minify: production, diff --git a/src/components/vm/snapshots/get-available-space.sh b/src/components/vm/snapshots/get-available-space.sh new file mode 100644 index 000000000..72797ddf2 --- /dev/null +++ b/src/components/vm/snapshots/get-available-space.sh @@ -0,0 +1,7 @@ +#! /bin/bash + +set -eu + +path=$1 +while ! test -e "$path"; do path=$(dirname "$path"); done +stat -f -c '{ "unit": %S, "free": %a }' "$path" diff --git a/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx b/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx index 94fe11f31..f27e021d9 100644 --- a/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx +++ b/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx @@ -30,9 +30,11 @@ import { DialogsContext } from 'dialogs.jsx'; import { ModalError } from "cockpit-components-inline-notification.jsx"; import { FileAutoComplete } from "cockpit-components-file-autocomplete.jsx"; import { snapshotCreate, snapshotGetAll } from "../../../libvirtApi/snapshot.js"; -import { getSortedBootOrderDevices, LIBVIRT_SYSTEM_CONNECTION } from "../../../helpers.js"; +import { getSortedBootOrderDevices, LIBVIRT_SYSTEM_CONNECTION, dirname } from "../../../helpers.js"; import { domainGet } from '../../../libvirtApi/domain.js'; +import get_available_space_sh from "./get-available-space.sh"; + const _ = cockpit.gettext; let current_user = null; @@ -64,41 +66,78 @@ const DescriptionRow = ({ onValueChanged, description }) => { ); }; -function getDefaultMemoryPath(vm, snapName) { - // Choosing a default path where memory snapshot should be stored might be tricky. Ideally we want - // to store it in the same directory where the primary disk (the disk which is first booted) is stored - // If however no such disk can be found, we should fallback to libvirt's default /var/lib/libvirt +function getDefaultMemoryLocation(vm) { + // If we find an existing external snapshot, use it's memory path + // as the default. Otherwise, try to find the primary disk and use + // it's location. If that fails as well, use a reasonable hard + // coded value. + + for (const s of vm.snapshots.sort((a, b) => b.creationTime - a.creationTime)) { + if (s.memoryPath) + return dirname(s.memoryPath); + } + const devices = getSortedBootOrderDevices(vm).filter(d => d.bootOrder && d.device.device === "disk" && d.device.type === "file" && d.device.source.file); if (devices.length > 0) { - const primaryDiskPath = devices[0].device.source.file; - const directory = primaryDiskPath.substring(0, primaryDiskPath.lastIndexOf("/") + 1); - return directory + snapName; + return dirname(devices[0].device.source.file); } else { if (vm.connectionName === LIBVIRT_SYSTEM_CONNECTION) - return "/var/lib/libvirt/memory/" + snapName; + return "/var/lib/libvirt/memory"; else if (current_user) - return current_user.home + "/.local/share/libvirt/memory/" + snapName; + return current_user.home + "/.local/share/libvirt/memory"; } return ""; } -const MemoryPathRow = ({ onValueChanged, memoryPath, validationError }) => { +const MemoryLocationRow = ({ onValueChanged, memoryLocation, validationError, available, needed }) => { + let info = ""; + let info_variant = "default"; + + if (needed) { + info = cockpit.format(_("Memory snapshot will use about $0."), + cockpit.format_bytes(needed)); + } + if (available) { + info = info + " " + cockpit.format(_("Total space available: $0."), cockpit.format_bytes(available)); + if (needed && available * 0.9 < needed) + info_variant = "warning"; + } + return ( - + onValueChanged("memoryPath", value)} - superuser="try" + onChange={value => onValueChanged("memoryLocation", value)} + value={memoryLocation} isOptionCreatable - value={memoryPath} /> - + onlyDirectories + placeholder={_("Path to directory")} + superuser="try" /> + ); }; +function get_available_space(path, superuser, callback) { + if (!path) + callback(null); + + cockpit.script(get_available_space_sh, [path], { superuser }) + .then(output => { + const info = JSON.parse(output); + callback(info.free * info.unit); + }) + .catch(exc => { + // channel has already logged the error + callback(null); + }); +} + export class CreateSnapshotModal extends React.Component { static contextType = DialogsContext; @@ -107,11 +146,12 @@ export class CreateSnapshotModal extends React.Component { // cut off seconds, subseconds, and timezone const now = new Date().toISOString() .replace(/:[^:]*$/, ''); - const snapName = props.vm.name + '_' + now; + const snapName = now; this.state = { name: snapName, description: "", - memoryPath: getDefaultMemoryPath(props.vm, snapName), + memoryLocation: getDefaultMemoryLocation(props.vm), + available: null, inProgress: false, }; @@ -121,8 +161,19 @@ export class CreateSnapshotModal extends React.Component { this.onCreate = this.onCreate.bind(this); } + updateAvailableSpace(path) { + get_available_space(path, this.props.vm.connectionName === LIBVIRT_SYSTEM_CONNECTION, + val => this.setState({ available: val })); + } + onValueChanged(key, value) { this.setState({ [key]: value }); + if (key == "memoryLocation") { + // We don't need to debounce this. The "memoryLocation" + // state is not changed on each keypress, but only when + // the input is blurred. + this.updateAvailableSpace(value); + } } dialogErrorSet(text, detail) { @@ -130,7 +181,7 @@ export class CreateSnapshotModal extends React.Component { } onValidate() { - const { name, memoryPath } = this.state; + const { name, memoryLocation } = this.state; const { vm, isExternal } = this.props; const validationError = {}; @@ -139,8 +190,8 @@ export class CreateSnapshotModal extends React.Component { else if (!name) validationError.name = _("Name can not be empty"); - if (isExternal && vm.state === "running" && !memoryPath) - validationError.memory = _("Memory file can not be empty"); + if (isExternal && vm.state === "running" && !memoryLocation) + validationError.memory = _("Memory save location can not be empty"); return validationError; } @@ -148,18 +199,28 @@ export class CreateSnapshotModal extends React.Component { onCreate() { const Dialogs = this.context; const { vm, isExternal } = this.props; - const { name, description, memoryPath } = this.state; + const { name, description, memoryLocation } = this.state; const validationError = this.onValidate(); if (!Object.keys(validationError).length) { this.setState({ inProgress: true }); - snapshotCreate({ - vm, - name, - description, - isExternal, - memoryPath: isExternal && vm.state === "running" && memoryPath, - }) + let mpath = null; + if (isExternal && vm.state === "running" && memoryLocation) { + mpath = memoryLocation; + if (mpath[mpath.length - 1] != "/") + mpath = mpath + "/"; + mpath = mpath + vm.name + "." + name + ".save"; + } + const superuser = (vm.connectionName === LIBVIRT_SYSTEM_CONNECTION); + cockpit.spawn(["mkdir", "-p", memoryLocation], { superuser, err: "message" }) + .then(() => + snapshotCreate({ + vm, + name, + description, + isExternal, + memoryPath: mpath, + })) .then(() => { // VM Snapshots do not trigger any events so we have to refresh them manually snapshotGetAll({ connectionName: vm.connectionName, domainPath: vm.id }); @@ -175,19 +236,38 @@ export class CreateSnapshotModal extends React.Component { } } + componentDidMount() { + this.updateAvailableSpace(this.state.memoryLocation); + } + + estimateMemorySnapshotSize(vm) { + /* According to experiments, the memory snapshot is smaller + than the amount of RAM used by the virtual machine. + + RSS File + ---------------- + 254 MB 145 MB + 636 MB 492 MB + 1.57 GB 1.4 GB + */ + return (vm.rssMemory || vm.currentMemory) * 1024; + } + render() { const Dialogs = this.context; const { idPrefix, isExternal, vm } = this.props; - const { name, description, memoryPath } = this.state; + const { name, description, memoryLocation, available } = this.state; const validationError = this.onValidate(); const body = ( -
e.preventDefault()} isHorizontal> + e.preventDefault()}> {isExternal && vm.state === 'running' && - } + } ); diff --git a/src/helpers.js b/src/helpers.js index 7ab9f20e7..94e189a9b 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -910,3 +910,13 @@ export function vmSupportsExternalSnapshots(config, vm) { export function vmHasVFIOHostDevs(vm) { return !!vm.hostDevices.find(hd => hd.driver === "vfio"); } + +function dirname(path) { + const i = path.lastIndexOf("/"); + if (i < 0) + return "."; + else if (i == 0) + return "/"; + else + return path.substr(0, i); +} diff --git a/src/libvirt-xml-parse.js b/src/libvirt-xml-parse.js index e4d4894f7..1c8c3f33f 100644 --- a/src/libvirt-xml-parse.js +++ b/src/libvirt-xml-parse.js @@ -216,14 +216,16 @@ export function parseDomainSnapshotDumpxml(snapshot) { const nameElem = getSingleOptionalElem(snapElem, 'name'); const descElem = getSingleOptionalElem(snapElem, 'description'); const parentElem = getSingleOptionalElem(snapElem, 'parent'); + const memElem = getSingleOptionalElem(snapElem, 'memory'); const name = nameElem?.childNodes[0].nodeValue; const description = descElem?.childNodes[0].nodeValue; const parentName = parentElem?.getElementsByTagName("name")[0].childNodes[0].nodeValue; const state = snapElem.getElementsByTagName("state")[0].childNodes[0].nodeValue; const creationTime = snapElem.getElementsByTagName("creationTime")[0].childNodes[0].nodeValue; + const memoryPath = memElem?.getAttribute("file"); - return { name, description, state, creationTime, parentName }; + return { name, description, state, creationTime, parentName, memoryPath }; } export function parseDomainDumpxml(connectionName, domXml, objPath) { diff --git a/test/check-machines-snapshots b/test/check-machines-snapshots index 5b7272ab1..39ae4c910 100755 --- a/test/check-machines-snapshots +++ b/test/check-machines-snapshots @@ -20,6 +20,8 @@ import os import subprocess import time +import unittest +import xml.etree.ElementTree as ET import machineslib import testlib @@ -30,6 +32,13 @@ def supportsSnapshot(image): return not image.startswith("rhel-8") +def supportsExternalSnapshots(image): + # external memory snapshots introduced in libvirt 9.9.0 + return not ( + image.startswith("rhel-8") or + image in ["debian-stable", "ubuntu-2204"]) + + @testlib.nondestructive class TestMachinesSnapshots(machineslib.VirtualMachinesCase): @@ -147,7 +156,7 @@ class TestMachinesSnapshots(machineslib.VirtualMachinesCase): if self.description: b.set_input_text("#snapshot-create-dialog-description", self.description) if self.memory_path is not None: - b.set_input_text("#snapshot-create-dialog-memory-path input", self.memory_path) + b.set_input_text("#snapshot-create-dialog-memory-location input", self.memory_path) def assert_pixels(self): if self.name == 'test_snap_1': @@ -172,7 +181,7 @@ class TestMachinesSnapshots(machineslib.VirtualMachinesCase): self.assert_pixels() b.wait_visible("#snapshot-create-dialog-name[aria-invalid=true]") elif self.xfail == 'memory-path': - b.wait_visible("#snapshot-create-dialog-memory-path .pf-v5-c-helper-text__item-text") + b.wait_visible("#snapshot-create-dialog-memory-location .pf-v5-c-helper-text__item-text") else: raise ValueError(f"Unknown xfail: {self.xfail}") @@ -242,8 +251,7 @@ class TestMachinesSnapshots(machineslib.VirtualMachinesCase): # No Snapshots present b.wait_visible("#vm-subVmTest1-add-snapshot-button") - # external memory snapshots introduced in libvirt 9.9.0 - supports_external = not (m.image.startswith("rhel-8") or m.image in ["debian-stable", "ubuntu-2204"]) + supports_external = supportsExternalSnapshots(m.image) # HACK: deleting external snapshots for non-running VMs is broken https://bugs.debian.org/bug=1061725 # Work around that by temporarily disabling libvirtd's AppArmor profile. AppArmor isn't installed by @@ -469,6 +477,117 @@ class TestMachinesSnapshots(machineslib.VirtualMachinesCase): self.assertEqual("no", m.execute("virsh snapshot-info --domain subVmTest1 --snapshotname snapshotB | grep 'Current:' | awk '{print $2}'").strip()) self.assertEqual("yes", m.execute("virsh snapshot-info --domain subVmTest1 --snapshotname snapshotC | grep 'Current:' | awk '{print $2}'").strip()) + def testMemoryLocation(self, connection="system"): + b = self.browser + m = self.machine + + def getSnapshotXML(vm, snap): + xml = self.run_admin(f"virsh -c qemu:///{connection} snapshot-dumpxml --domain {vm} --snapshotname {snap}", + connectionName=connection) + return ET.fromstring(xml) + + if not supportsExternalSnapshots(m.image): + raise unittest.SkipTest("external snapshots not supported") + + self.login_and_go("/machines") + self.waitPageInit() + + self.createVm("subVmTest1", connection=connection) + + self.waitVmRow("subVmTest1", connectionName=connection) + b.wait_in_text(f"#vm-subVmTest1-{connection}-state", "Running") + self.goToVmPage("subVmTest1", connectionName=connection) + + # Open dialog + b.click("#vm-subVmTest1-add-snapshot-button") + b.set_input_text("#snapshot-create-dialog-name", "snap-1") + + snapshot_dir = "/var/lib/libvirt" if connection == "system" else "/home/admin/.local/share/libvirt" + + # Check default + b.wait_val("#snapshot-create-dialog-memory-location input", snapshot_dir + "/images") + + # Check validation + b.set_input_text("#snapshot-create-dialog-memory-location input", "") + b.wait_text("#snapshot-create-dialog-memory-location .pf-v5-c-helper-text__item-text", + "Memory save location can not be empty") + + # Check creation of new directories + b.set_input_text("#snapshot-create-dialog-memory-location input", snapshot_dir + "/new") + b.wait_text("#snapshot-create-dialog-memory-location li button", f'Create "{snapshot_dir}/new"') + b.click("#snapshot-create-dialog-memory-location li button") + + # Check available space detection + b.wait_in_text("#snapshot-create-dialog-memory-location .pf-v5-c-helper-text__item-text", + "Memory snapshot will use about ") + b.wait_in_text("#snapshot-create-dialog-memory-location .pf-v5-c-helper-text__item-text", + "Total space available: ") + self.assertNotIn("pf-m-warning", + b.attr("#snapshot-create-dialog-memory-location .pf-v5-c-helper-text__item", "class")) + + # Create it + b.click(".pf-v5-c-modal-box__footer button:contains(Create)") + b.wait_not_present("#vm-subVmTest1-create-snapshot-modal") + + # Check that it uses the new directory + b.wait_text("#vm-subVmTest1-snapshot-0-name", "snap-1") + self.assertEqual(getSnapshotXML("subVmTest1", "snap-1").find("memory").get("file"), + snapshot_dir + "/new/subVmTest1.snap-1.save") + + # Open dialog for second snapshot + b.click("#vm-subVmTest1-add-snapshot-button") + b.set_input_text("#snapshot-create-dialog-name", "snap-2") + + # Check that default comes from the previous snapshot + b.wait_val("#snapshot-create-dialog-memory-location input", snapshot_dir + "/new") + + # Use a different location so that we have two snapshots and + # can test the sorting function that determines the most + # recent one. + + snapshot_dir2 = "/var/lib/libvirt2" if connection == "system" else "/home/admin/.local/share/libvirt/2" + + b.set_input_text("#snapshot-create-dialog-memory-location input", snapshot_dir2) + b.wait_text("#snapshot-create-dialog-memory-location li button", f'Create "{snapshot_dir2}"') + b.click("#snapshot-create-dialog-memory-location li:first-child button") + + # Wait a couple of seconds to guarantee a different + # "creationTime" from the previous snapshot and create it + time.sleep(3) + b.click(".pf-v5-c-modal-box__footer button:contains(Create)") + b.wait_not_present("#vm-subVmTest1-create-snapshot-modal") + + b.wait_text("#vm-subVmTest1-snapshot-0-name", "snap-2") + self.assertEqual(getSnapshotXML("subVmTest1", "snap-2").find("memory").get("file"), + os.path.join(snapshot_dir2, "subVmTest1.snap-2.save")) + + # Open dialog for third snapshot + b.click("#vm-subVmTest1-add-snapshot-button") + b.set_input_text("#snapshot-create-dialog-name", "snap-3") + + # Check that default comes from the previous snapshot + b.wait_val("#snapshot-create-dialog-memory-location input", snapshot_dir2) + + dev = self.add_ram_disk(50) + m.execute(f"mkfs.ext4 {dev}; mkdir -p /var/small; mount {dev} /var/small") + if connection == "session": + m.execute("chown admin:admin /var/small") + b.set_input_text("#snapshot-create-dialog-memory-location input", "/var/small") + b.wait_text("#snapshot-create-dialog-memory-location li:first-child button", "/var/small/") + b.click("#snapshot-create-dialog-memory-location li:first-child button") + b.wait_in_text("#snapshot-create-dialog-memory-location .pf-v5-c-helper-text__item.pf-m-warning", + "Total space available: ") + + # Go ahead and let it fail + b.click(".pf-v5-c-modal-box__footer button:contains(Create)") + b.wait_in_text("#vm-subVmTest1-create-snapshot-modal .pf-v5-c-alert", + "No space left on device") + b.click(".pf-v5-c-modal-box__footer button:contains(Cancel)") + b.wait_not_present("#vm-subVmTest1-create-snapshot-modal") + + def testMemoryLocationSession(self): + self.testMemoryLocation(connection="session") + if __name__ == '__main__': testlib.test_main() diff --git a/test/machineslib.py b/test/machineslib.py index d0515ce40..425a930b9 100644 --- a/test/machineslib.py +++ b/test/machineslib.py @@ -410,7 +410,7 @@ def stop_all(): ' echo "$out" | grep -q "domain is not running"; ' " fi; done") m.execute("runuser -l admin -c 'for d in $(virsh -c qemu:///session list --all --name); do " - "virsh -c qemu:///session undefine $d; done'") + "virsh -c qemu:///session undefine $d --snapshots-metadata; done'") # pools m.execute("rm -rf /run/libvirt/storage/*") diff --git a/test/reference b/test/reference index 8ee2e589c..3b83d43ee 160000 --- a/test/reference +++ b/test/reference @@ -1 +1 @@ -Subproject commit 8ee2e589cb8448aaf4bd8cf07f22b3cbebc18739 +Subproject commit 3b83d43eecc693250b63df27d573444cbae177c8