Create external snapshots when supported

External VM snapshots are more reliable and flexible than the internal format. They also work for raw disk images, not just for qcow2. In addition, the internal format was declared deprecated in RHEL 8 already. The external format is supported with libvirt 9.9.0 or later. Detect it through the capability and use it when available. If the VM is running, add a a memory snapshot file path input to the snapshot creation dialog. Co-Authored-By: Martin Pitt <[email protected]>
skobyda · Jan 30, 2024 · b556d2b · b556d2b
1 parent a1d8ab8
commit b556d2b
Show file tree

Hide file tree

Showing 9 changed files with 233 additions and 29 deletions.
diff --git a/src/components/vm/snapshots/vmSnapshotsCard.jsx b/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} />);
     }
 

diff --git a/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx b/src/components/vm/snapshots/vmSnapshotsCreateModal.jsx
@@ -28,20 +28,25 @@ 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
             label={_("Name")}
             fieldId="snapshot-create-dialog-name">
             <TextInput value={name}
-                validated={validationError.name ? "error" : "default"}
+                validated={validationError ? "error" : "default"}
                 id="snapshot-create-dialog-name"
                 onChange={(_, value) => onValueChanged("name", value)} />
-            <FormHelper helperTextInvalid={validationError.name} />
+            <FormHelper helperTextInvalid={validationError} />
         </FormGroup>
     );
 };
@@ -58,6 +63,40 @@ 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, validationError }) => {
+    return (
+        <FormGroup id="snapshot-create-dialog-memory-path" label={_("Memory file")}>
+            <FileAutoComplete
+                onChange={value => onValueChanged("memoryPath", value)}
+                superuser="try"
+                value={memoryPath} />
+            <FormHelper helperTextInvalid={validationError} />
+        </FormGroup>
+    );
+};
+
 export class CreateSnapshotModal extends React.Component {
     static contextType = DialogsContext;
 
@@ -66,9 +105,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 +128,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.memory = _("Memory file can 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 +178,17 @@ 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} />
+                <NameRow name={name} validationError={validationError.name} onValueChanged={this.onValueChanged} />
                 <DescriptionRow description={description} onValueChanged={this.onValueChanged} />
+                {isExternal && vm.state === 'running' &&
+                    <MemoryPathRow memoryPath={memoryPath} onValueChanged={this.onValueChanged}
+                                   validationError={validationError.memory} />}
             </Form>
         );
 

diff --git a/src/components/vm/vmDetailsPage.jsx b/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} />
         });
     }

diff --git a/src/helpers.js b/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;
+}
diff --git a/src/libvirt-xml-create.js b/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);

diff --git a/src/libvirt-xml-parse.js b/src/libvirt-xml-parse.js
@@ -414,7 +414,7 @@ export function parseDumpxmlForCapabilities(capabilitiesXML) {
             const osTypeElem = getSingleOptionalElem(guestElem, 'os_type');
             const archElem = getSingleOptionalElem(guestElem, 'arch');
 
-            const guestCapabilities = { // see https://libvirt.org/formatdomain.html#elementsDisks
+            const guestCapabilities = { // see https://libvirt.org/formatcaps.html#guest-capabilities
                 osType: osTypeElem?.childNodes[0].nodeValue,
                 arch: archElem.getAttribute('name'),
             };
@@ -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

diff --git a/src/libvirtApi/helpers.js b/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,

diff --git a/src/libvirtApi/snapshot.js b/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 = (!isExternal || 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 }) {