diff --git a/build.js b/build.js index ce5b8f714..7a373ef56 100755 --- a/build.js +++ b/build.js @@ -85,6 +85,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..bc667ecdf 100644 --- a/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx +++ b/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx @@ -17,7 +17,7 @@ * along with Cockpit; If not, see . */ import cockpit from "cockpit"; -import React from "react"; +import React from 'react'; import { Button } from "@patternfly/react-core/dist/esm/components/Button"; import { Form, FormGroup } from "@patternfly/react-core/dist/esm/components/Form"; @@ -30,9 +30,14 @@ 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, + formatWithBestUnit, 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 +69,80 @@ 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; + const directory = dirname(primaryDiskPath); + return directory; } 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 (available) { + info = cockpit.format(_("$0 available at this location."), formatWithBestUnit(available)); + if (available * 0.9 < needed) + info_variant = "warning"; + } + if (needed) { + info = info + " " + cockpit.format(_("About $0 are needed to save the memory state of the machine."), + formatWithBestUnit(needed)); + } + 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, callback) { + if (!path) + callback(null); + + cockpit.script(get_available_space_sh, [path], { superuser: "try" }) + .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 +151,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 +166,18 @@ export class CreateSnapshotModal extends React.Component { this.onCreate = this.onCreate.bind(this); } + updateAvailableSpace(path) { + get_available_space(path, 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 +185,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 +194,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,20 +203,29 @@ 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"; + } + cockpit.spawn(["mkdir", "-p", memoryLocation], { superuser: "try", 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 + // VM Snapshots do not trigger any events so we have to refresh them manually snapshotGetAll({ connectionName: vm.connectionName, domainPath: vm.id }); // Creating an external snapshot might change // the disk configuration of a VM without event. @@ -175,10 +239,14 @@ export class CreateSnapshotModal extends React.Component { } } + componentDidMount() { + this.updateAvailableSpace(this.state.memoryLocation); + } + 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 = ( @@ -186,8 +254,10 @@ export class CreateSnapshotModal extends React.Component { {isExternal && vm.state === 'running' && - } + } ); diff --git a/src/helpers.js b/src/helpers.js index 0718740ae..9d98eefae 100644 --- a/src/helpers.js +++ b/src/helpers.js @@ -145,6 +145,12 @@ export function convertToUnitVerbose(input, inputUnit, outputUnit) { return result; } +export function formatWithBestUnit(input, inputUnit) { + const best = convertToBestUnit(input, inputUnit || units.B); + const decimals = best.value >= 100 ? 0 : best.value >= 10 ? 1 : 2; + return cockpit.format("$0 $1", best.value.toFixed(decimals), best.unit); +} + export function isEmpty(str) { return (!str || str.length === 0); } @@ -907,3 +913,13 @@ export function vmSupportsExternalSnapshots(config, vm) { return true; } + +export function dirname(path) { + const i = path.lastIndexOf("/"); + if (i < 0) + return null; + 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 74492abc0..4da8bd7a1 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..130757d4f 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,18 @@ 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"]) + + +def getSnapshotXML(machine, vm, snap): + xml = machine.execute(f"virsh -c qemu:///system snapshot-dumpxml --domain {vm} --snapshotname {snap}") + return ET.fromstring(xml) + + @testlib.nondestructive class TestMachinesSnapshots(machineslib.VirtualMachinesCase): @@ -147,7 +161,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 +186,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 +256,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 +482,101 @@ 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): + b = self.browser + m = self.machine + + if not supportsExternalSnapshots(m.image): + raise unittest.SkipTest("external snapshots not supported") + + self.createVm("subVmTest1") + + self.login_and_go("/machines") + self.waitPageInit() + self.waitVmRow("subVmTest1") + b.wait_in_text("#vm-subVmTest1-system-state", "Running") + self.goToVmPage("subVmTest1") + + # Open dialog + b.click("#vm-subVmTest1-add-snapshot-button") + b.set_input_text("#snapshot-create-dialog-name", "snap-1") + + # Check default + b.wait_val("#snapshot-create-dialog-memory-location input", "/var/lib/libvirt/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", "/var/lib/libvirt/new") + b.wait_text("#snapshot-create-dialog-memory-location li button", 'Create "/var/lib/libvirt/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", + "available at this location") + b.wait_in_text("#snapshot-create-dialog-memory-location .pf-v5-c-helper-text__item-text", + "are needed to save the memory state of the machine.") + + # 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(m, "subVmTest1", "snap-1").find("memory").get("file"), + "/var/lib/libvirt/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", "/var/lib/libvirt/new") + + # Use a different location so that we have two snapshots and + # can test the sorting function that determines the most + # recent one. To check another edge case, we use "/" as the + # memory save location. + b.set_input_text("#snapshot-create-dialog-memory-location input", "/") + b.wait_text("#snapshot-create-dialog-memory-location li:first-child button", "/") + 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(m, "subVmTest1", "snap-2").find("memory").get("file"), + "/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", "/") + + # Make a small filesystem and check warning + dev = self.add_ram_disk(50) + m.execute(f"mkfs.ext4 {dev}; mkdir -p /var/small; mount {dev} /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", + "available at this location") + + # 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") + if __name__ == '__main__': testlib.test_main()