Create external snapshots when supported

Co-Authored-By: Martin Pitt <[email protected]>

src/components/vm/snapshots/vmSnapshotsCard.jsx

@@ -20,7 +20,7 @@ import React from 'react';
 
 import cockpit from 'cockpit';
 import { useDialogs, DialogsContext } from 'dialogs.jsx';
-import { vmId, localize_datetime } from "../../../helpers.js";
+import { vmId, localize_datetime, vmSupportsExternalSnapshots } from "../../../helpers.js";
 import { CreateSnapshotModal } from "./vmSnapshotsCreateModal.jsx";
 import { ListingTable } from "cockpit-components-table.jsx";
 import { Button } from "@patternfly/react-core/dist/esm/components/Button";
@@ -35,12 +35,16 @@ import './vmSnapshotsCard.scss';
 
 const _ = cockpit.gettext;
 
-export const VmSnapshotsActions = ({ vm }) => {
+export const VmSnapshotsActions = ({ vm, config, storagePools }) => {
     const Dialogs = useDialogs();
     const id = vmId(vm.name);
 
+    const isExternal = vmSupportsExternalSnapshots(config, vm, storagePools);
+
     function open() {
         Dialogs.show(<CreateSnapshotModal idPrefix={`${id}-create-snapshot`}
+                                          isExternal={isExternal}
+                                          storagePools={storagePools}
                                           vm={vm} />);
     }
 

src/components/vm/snapshots/vmSnapshotsCreateModal.jsx

@@ -28,10 +28,15 @@ import { TextInput } from "@patternfly/react-core/dist/esm/components/TextInput"
 import { FormHelper } from 'cockpit-components-form-helper.jsx';
 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";
 
 const _ = cockpit.gettext;
 
+let current_user = null;
+cockpit.user().then(user => { current_user = user });
+
 const NameRow = ({ onValueChanged, name, validationError }) => {
     return (
         <FormGroup
@@ -58,6 +63,39 @@ 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 howver no such disk can be found, we should fallback to libvirt's default /var/lib/libvirt
+    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;
+    } else {
+        if (vm.connectionName === LIBVIRT_SYSTEM_CONNECTION)
+            return "/var/lib/libvirt/memory/" + snapName;
+        else if (current_user)
+            return current_user.home + "/.local/share/libvirt/memory/" + snapName;
+    }
+
+    return "";
+}
+
+const MemoryPathRow = ({ onValueChanged, memoryPath }) => {
+    return (
+        <FormGroup fieldId="snapshot-create-dialog-memory-path" label={_("Memory file")}>
+            <FileAutoComplete id="snapshot-create-dialog-memory-path"
+                onChange={value => onValueChanged("memoryPath", value)}
+                superuser="try"
+                value={memoryPath} />
+        </FormGroup>
+    );
+};
+
 export class CreateSnapshotModal extends React.Component {
     static contextType = DialogsContext;
 
@@ -66,9 +104,11 @@ export class CreateSnapshotModal extends React.Component {
         // cut off seconds, subseconds, and timezone
         const now = new Date().toISOString()
                 .replace(/:[^:]*$/, '');
+        const snapName = props.vm.name + '_' + now;
         this.state = {
-            name: props.vm.name + '_' + now,
+            name: snapName,
             description: "",
+            memoryPath: getDefaultMemoryPath(props.vm, snapName),
             inProgress: false,
         };
 
@@ -87,29 +127,42 @@ export class CreateSnapshotModal extends React.Component {
     }
 
     onValidate(submitted = false) {
-        const { name } = this.state;
-        const { vm } = this.props;
+        const { name, memoryPath } = this.state;
+        const { vm, isExternal } = this.props;
         const validationError = {};
 
         if (vm.snapshots.findIndex(snap => snap.name === name) > -1)
             validationError.name = _("Name already exists");
         else if (!name && submitted)
             validationError.name = _("Name should not be empty");
 
+        if (isExternal && vm.state === "running" && !memoryPath)
+            validationError.name = _("Memory file should not be empty");
+
         return validationError;
     }
 
     onCreate() {
         const Dialogs = this.context;
-        const { vm } = this.props;
-        const { name, description } = this.state;
+        const { vm, isExternal, storagePools } = this.props;
+        const { name, description, memoryPath } = this.state;
+        const disks = Object.values(vm.disks);
         const validationError = this.onValidate(true);
 
         this.setState({ submitted: true });
 
         if (!Object.keys(validationError).length) {
             this.setState({ inProgress: true });
-            snapshotCreate({ connectionName: vm.connectionName, vmId: vm.id, name, description })
+            snapshotCreate({
+                connectionName: vm.connectionName,
+                vmId: vm.id,
+                name,
+                description,
+                memoryPath: vm.state === "running" && memoryPath,
+                disks,
+                isExternal,
+                storagePools
+            })
                     .then(() => {
                         // VM Snapshots do not trigger any events so we have to refresh them manually
                         snapshotGetAll({ connectionName: vm.connectionName, domainPath: vm.id });
@@ -124,14 +177,15 @@ export class CreateSnapshotModal extends React.Component {
 
     render() {
         const Dialogs = this.context;
-        const { idPrefix } = this.props;
-        const { name, description, submitted } = this.state;
+        const { idPrefix, isExternal, vm } = this.props;
+        const { name, description, memoryPath, submitted } = this.state;
         const validationError = this.onValidate(submitted);
 
         const body = (
             <Form onSubmit={e => e.preventDefault()} isHorizontal>
                 <NameRow name={name} validationError={validationError} onValueChanged={this.onValueChanged} />
                 <DescriptionRow description={description} onValueChanged={this.onValueChanged} />
+                {isExternal && vm.state === 'running' && <MemoryPathRow memoryPath={memoryPath} onValueChanged={this.onValueChanged} />}
             </Form>
         );
 

src/components/vm/vmDetailsPage.jsx

@@ -175,7 +175,7 @@ export const VmDetailsPage = ({
             id: cockpit.format("$0-snapshots", vmId(vm.name)),
             className: "snapshots-card",
             title: _("Snapshots"),
-            actions: <VmSnapshotsActions vm={vm} />,
+            actions: <VmSnapshotsActions vm={vm} config={config} storagePools={storagePools} />,
             body: <VmSnapshotsCard vm={vm} config={config} />
         });
     }

src/helpers.js

@@ -873,3 +873,41 @@ export function getIfaceSourceName(iface) {
 export function canDeleteDiskFile(disk) {
     return disk.type === "volume" || (disk.type === "file" && disk.source.file);
 }
+
+export function getStoragePoolPath(storagePools, poolName, connectionName) {
+    const pool = storagePools.find(pool => pool.name === poolName && pool.connectionName === connectionName);
+
+    return pool?.target?.path;
+}
+
+export function vmSupportsExternalSnapshots(config, vm, storagePools) {
+    const disks = Object.values(vm.disks);
+
+    // If at leat one disk has internal snapshot preference specified, use internal snapshot for all disk,
+    // as mixing internal and external is not allowed
+    if (disks.some(disk => disk.snapshot === "internal")) {
+        logDebug(`vmSupportsExternalSnapshots: vm ${vm.name} has internal snapshot preference specified, false`);
+        return false;
+    }
+
+    // Currently external snapshots works only for disks where we can specify a local path of a newly created disk snapshot file
+    // This rules out all disks which are not file-based, or pool-based where pool is of type "dir"
+    if (!disks.every(disk => {
+        const supportedPools = storagePools.filter(pool => pool.type === "dir" && pool.connectionName === vm.connectionName);
+
+        return disk.type === "file" ||
+            (disk.type === "volume" && supportedPools.some(pool => pool.name === disk.source.pool));
+    })) {
+        logDebug(`vmSupportsExternalSnapshots: vm ${vm.name} has unsupported disk type, false`);
+        return false;
+    }
+
+    // External snapshot should only be used if the VM's os types/architecture allow it
+    // and if snapshot features are present among guest capabilities:
+    // https://libvirt.org/formatcaps.html#guest-capabilities
+    const support = config.capabilities?.guests.some(guest => guest.osType === vm.osType &&
+                                                     guest.arch === vm.arch &&
+                                                     guest.features.externalSnapshot);
+    logDebug(`vmSupportsExternalSnapshots: vm ${vm.name} has external snapshot support: ${support}`);
+    return support;
+}

src/libvirt-xml-create.js

@@ -1,3 +1,5 @@
+import { getStoragePoolPath } from "./helpers.js";
+
 export function getDiskXML(type, file, device, poolName, volumeName, format, target, cacheMode, shareable, busType, serial) {
     const doc = document.implementation.createDocument('', '', null);
 
@@ -216,7 +218,8 @@ export function getPoolXML({ name, type, source, target }) {
     return new XMLSerializer().serializeToString(doc.documentElement);
 }
 
-export function getSnapshotXML(name, description) {
+// see https://libvirt.org/formatsnapshot.html
+export function getSnapshotXML(name, description, disks, memoryPath, isExternal, storagePools, connectionName) {
     const doc = document.implementation.createDocument('', '', null);
 
     const snapElem = doc.createElement('domainsnapshot');
@@ -233,6 +236,39 @@ export function getSnapshotXML(name, description) {
         snapElem.appendChild(descriptionElem);
     }
 
+    if (isExternal) {
+        if (memoryPath) {
+            const memoryElem = doc.createElement('memory');
+            memoryElem.setAttribute('snapshot', 'external');
+            memoryElem.setAttribute('file', memoryPath);
+            snapElem.appendChild(memoryElem);
+        }
+
+        const disksElem = doc.createElement('disks');
+        disks.forEach(disk => {
+            // Disk can have attribute "snapshot" set to "no", which means no snapshot should be created of the said disk
+            // This cannot be configured through cockpit, but we should uphold it nevertheless
+            // see "snapshot" attribute of <disk> element at https://libvirt.org/formatdomain.html#hard-drives-floppy-disks-cdroms
+            if (disk.snapshot !== "no") {
+                const diskElem = doc.createElement('disk');
+                diskElem.setAttribute('name', disk.target);
+                diskElem.setAttribute('snapshot', 'external');
+
+                if (disk.type === "volume") {
+                    const poolPath = getStoragePoolPath(storagePools, disk.source.pool, connectionName);
+                    if (poolPath) {
+                        const sourceElem = doc.createElement('source');
+                        sourceElem.setAttribute('file', `${poolPath}/${disk.source.volume}.snap`);
+                        diskElem.appendChild(sourceElem);
+                    }
+                }
+
+                disksElem.appendChild(diskElem);
+            }
+        });
+        snapElem.appendChild(disksElem);
+    }
+
     doc.appendChild(snapElem);
 
     return new XMLSerializer().serializeToString(doc.documentElement);

src/libvirt-xml-parse.js

@@ -468,6 +468,7 @@ export function parseDumpxmlForDisks(devicesElem) {
                 },
                 bootOrder: bootElem?.getAttribute('order'),
                 type: diskElem.getAttribute('type'), // i.e.: file
+                snapshot: diskElem.getAttribute('snapshot'), // i.e.: internal, external
                 device: diskElem.getAttribute('device'), // i.e. cdrom, disk
                 source: {
                     file: sourceElem?.getAttribute('file'), // optional file name of the disk

src/libvirtApi/helpers.js

@@ -41,11 +41,13 @@ export const Enum = {
     VIR_DOMAIN_UNDEFINE_MANAGED_SAVE: 1,
     VIR_DOMAIN_UNDEFINE_SNAPSHOTS_METADATA: 2,
     VIR_DOMAIN_UNDEFINE_NVRAM: 4,
-    VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL: 256,
+    // https://libvirt.org/html/libvirt-libvirt-domain-snapshot.html
     VIR_DOMAIN_SNAPSHOT_REVERT_RUNNING: 1,
     VIR_DOMAIN_SNAPSHOT_REVERT_PAUSED: 2,
     VIR_DOMAIN_SNAPSHOT_REVERT_FORCE: 4,
     VIR_DOMAIN_SNAPSHOT_REVERT_RESET_NVRAM: 8,
+    VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY: 16,
+    VIR_DOMAIN_SNAPSHOT_LIST_INTERNAL: 256,
     VIR_DOMAIN_STATS_BALLOON: 4,
     VIR_DOMAIN_SHUTOFF: 5,
     VIR_DOMAIN_STATS_VCPU: 8,

src/libvirtApi/snapshot.js

@@ -29,10 +29,11 @@ import { parseDomainSnapshotDumpxml } from '../libvirt-xml-parse.js';
 import { call, Enum, timeout } from './helpers.js';
 import { logDebug } from '../helpers.js';
 
-export function snapshotCreate({ connectionName, vmId, name, description }) {
-    const xmlDesc = getSnapshotXML(name, description);
-
-    return call(connectionName, vmId, 'org.libvirt.Domain', 'SnapshotCreateXML', [xmlDesc, 0], { timeout, type: 'su' });
+export function snapshotCreate({ connectionName, vmId, name, description, memoryPath, disks, isExternal, storagePools }) {
+    // that flag ought to be implicit for non-running VMs, see https://issues.redhat.com/browse/RHEL-22797
+    const flags = memoryPath ? 0 : Enum.VIR_DOMAIN_SNAPSHOT_CREATE_DISK_ONLY;
+    const xmlDesc = getSnapshotXML(name, description, disks, memoryPath, isExternal, storagePools, connectionName);
+    return call(connectionName, vmId, 'org.libvirt.Domain', 'SnapshotCreateXML', [xmlDesc, flags], { timeout, type: 'su' });
 }
 
 export function snapshotCurrent({ connectionName, objPath }) {

test/check-machines-snapshots

@@ -29,6 +29,13 @@ def supportsSnapshot(image):
     return image != "centos-8-stream" and not image.startswith("rhel-8")
 
 
+# introduced in libvirt 9.9.0
+def supportsExternalSnapshot(image):
+    old = (image.startswith("rhel-8") or image.startswith("centos-8") or
+           image in ["ubuntu-2204", "ubuntu-stable", "debian-stable", "fedora-38", "fedora-39"])
+    return not old
+
+
 @testlib.nondestructive
 class TestMachinesSnapshots(machineslib.VirtualMachinesCase):
 
@@ -134,6 +141,7 @@ class TestMachinesSnapshots(machineslib.VirtualMachinesCase):
                 self.vm_name = vm_name
                 self.remove = remove
                 self.xfail = xfail
+                self.supports_external = supportsExternalSnapshot(self.test_obj.machine.image)
 
             def execute(self):
                 self.open()
@@ -204,6 +212,26 @@ class TestMachinesSnapshots(machineslib.VirtualMachinesCase):
                 if (self.state):
                     self.test_obj.assertEqual(self.state, m.execute(xmllint_element.format(prop='state')).strip())
 
+                try:
+                    memory_snapshot = m.execute(xmllint_element.format(prop='memory/@snapshot')).strip()
+                    ext_int = "external" if self.supports_external else "internal"
+
+                    if self.state == "running":
+                        self.test_obj.assertEqual(memory_snapshot, ext_int)
+                    else:
+                        self.test_obj.assertEqual(memory_snapshot, 'no')
+
+                    disk_snapshot = m.execute(xmllint_element.format(prop='disks/disk/@snapshot')).strip()
+                    self.test_obj.assertEqual(disk_snapshot, ext_int)
+                    if self.name and self.supports_external:
+                        disk_source = m.execute(xmllint_element.format(prop='disks/disk/source/@file')).strip()
+                        self.test_obj.assertIn(self.name, disk_source)
+                except AssertionError:
+                    print("------ snapshot XML -------")
+                    print(m.execute(snap_xml))
+                    print("------ end snapshot XML -------")
+                    raise
+
             def cleanup(self):
                 b.click(f"#delete-vm-subVmTest1-snapshot-{self.snap_num}")
                 b.wait_in_text(".pf-v5-c-modal-box .pf-v5-c-modal-box__header .pf-v5-c-modal-box__title", "Delete snapshot?")