Skip to content

Commit

Permalink
snapshots: Improve memory path handling
Browse files Browse the repository at this point in the history
 - We only ask for the directory of the memory snapshot, not for the
   full filename.

 - The default for the memory save location is taken from previous
   snapshots.

 - The default snapshot name does not include the VM name.

 - Available and needed storage space is shown in the dialog.

 - New directories can be created from the dialog.

 - The memory save file gets the extension ".save", to mirror what
   "virsh managedsave" is doing.
  • Loading branch information
mvollmer committed Sep 10, 2024
1 parent 72ac8c2 commit 681addd
Show file tree
Hide file tree
Showing 9 changed files with 253 additions and 42 deletions.
1 change: 1 addition & 0 deletions build.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ const context = await esbuild.context({
loader: {
".js": "jsx",
".py": "text",
".sh": "text",
},
metafile: !!args.metafile,
minify: production,
Expand Down
2 changes: 1 addition & 1 deletion node_modules
Submodule node_modules updated 40 files
+21 −15 .package-lock.json
+3 −2 .package.json
+2 −2 @isaacs/cliui/node_modules/ansi-regex/index.d.ts
+4 −2 @isaacs/cliui/node_modules/ansi-regex/index.js
+6 −3 @isaacs/cliui/node_modules/ansi-regex/package.json
+2 −14 @isaacs/cliui/node_modules/ansi-regex/readme.md
+16 −0 path-browserify/.travis.yml
+20 −0 path-browserify/CHANGELOG.md
+20 −0 path-browserify/LICENSE
+45 −0 path-browserify/README.md
+529 −0 path-browserify/index.js
+30 −0 path-browserify/package.json
+10 −0 path-browserify/security.md
+9 −0 path-browserify/test/index.js
+79 −0 path-browserify/test/test-path-basename.js
+58 −0 path-browserify/test/test-path-dirname.js
+96 −0 path-browserify/test/test-path-extname.js
+33 −0 path-browserify/test/test-path-isabsolute.js
+126 −0 path-browserify/test/test-path-join.js
+235 −0 path-browserify/test/test-path-parse-format.js
+66 −0 path-browserify/test/test-path-relative.js
+45 −0 path-browserify/test/test-path-resolve.js
+53 −0 path-browserify/test/test-path-zero-length-strings.js
+107 −0 path-browserify/test/test-path.js
+2 −2 string-width/node_modules/ansi-regex/index.d.ts
+4 −2 string-width/node_modules/ansi-regex/index.js
+6 −3 string-width/node_modules/ansi-regex/package.json
+2 −14 string-width/node_modules/ansi-regex/readme.md
+2 −2 stylelint-formatter-pretty/node_modules/ansi-regex/index.d.ts
+4 −2 stylelint-formatter-pretty/node_modules/ansi-regex/index.js
+6 −3 stylelint-formatter-pretty/node_modules/ansi-regex/package.json
+2 −14 stylelint-formatter-pretty/node_modules/ansi-regex/readme.md
+2 −2 stylelint/node_modules/strip-ansi/node_modules/ansi-regex/index.d.ts
+4 −2 stylelint/node_modules/strip-ansi/node_modules/ansi-regex/index.js
+6 −3 stylelint/node_modules/strip-ansi/node_modules/ansi-regex/package.json
+2 −14 stylelint/node_modules/strip-ansi/node_modules/ansi-regex/readme.md
+2 −2 wrap-ansi/node_modules/ansi-regex/index.d.ts
+4 −2 wrap-ansi/node_modules/ansi-regex/index.js
+6 −3 wrap-ansi/node_modules/ansi-regex/package.json
+2 −14 wrap-ansi/node_modules/ansi-regex/readme.md
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
"stylelint-use-logical-spec": "5.0.1"
},
"dependencies": {
"@novnc/novnc": "1.4.0",
"@patternfly/patternfly": "5.4.0",
"@patternfly/react-console": "5.1.0",
"@patternfly/react-core": "5.4.0",
Expand All @@ -55,12 +56,12 @@
"@xterm/addon-canvas": "0.7.0",
"@xterm/xterm": "5.5.0",
"dequal": "2.0.3",
"path-browserify": "1.0.1",
"prop-types": "15.8.1",
"react": "18.3.1",
"react-dom": "18.3.1",
"redux": "5.0.1",
"redux-thunk": "3.1.0",
"throttle-debounce": "5.0.2",
"@novnc/novnc": "1.4.0"
"throttle-debounce": "5.0.2"
}
}
7 changes: 7 additions & 0 deletions src/components/vm/snapshots/get-available-space.sh
Original file line number Diff line number Diff line change
@@ -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"
145 changes: 113 additions & 32 deletions src/components/vm/snapshots/vmSnapshotsCreateModal.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
*/
import cockpit from "cockpit";
import React from "react";
import { dirname } from 'path-browserify';

import { Button } from "@patternfly/react-core/dist/esm/components/Button";
import { Form, FormGroup } from "@patternfly/react-core/dist/esm/components/Form";
Expand All @@ -33,6 +34,8 @@ import { snapshotCreate, snapshotGetAll } from "../../../libvirtApi/snapshot.js"
import { getSortedBootOrderDevices, LIBVIRT_SYSTEM_CONNECTION } 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;
Expand Down Expand Up @@ -64,41 +67,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 (
<FormGroup id="snapshot-create-dialog-memory-path" label={_("Memory file")}>
<FormGroup id="snapshot-create-dialog-memory-location" label={_("Memory state file location")}>
<FileAutoComplete
onChange={value => onValueChanged("memoryPath", value)}
superuser="try"
onChange={value => onValueChanged("memoryLocation", value)}
value={memoryLocation}
isOptionCreatable
value={memoryPath} />
<FormHelper helperTextInvalid={validationError} />
onlyDirectories
placeholder={_("Path to directory")}
superuser="try" />
<FormHelper helperTextInvalid={validationError}
helperText={info}
variant={validationError ? "error" : info_variant} />
</FormGroup>
);
};

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;

Expand All @@ -107,11 +147,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,
};

Expand All @@ -121,16 +162,27 @@ 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) {
this.setState({ dialogError: text, dialogErrorDetail: detail });
}

onValidate() {
const { name, memoryPath } = this.state;
const { name, memoryLocation } = this.state;
const { vm, isExternal } = this.props;
const validationError = {};

Expand All @@ -139,27 +191,37 @@ 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;
}

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 });
Expand All @@ -175,19 +237,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 = (
<Form onSubmit={e => e.preventDefault()} isHorizontal>
<Form onSubmit={e => e.preventDefault()}>
<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} />}
<MemoryLocationRow memoryLocation={memoryLocation} onValueChanged={this.onValueChanged}
validationError={validationError.memory}
available={available}
needed={this.estimateMemorySnapshotSize(vm)} />}
</Form>
);

Expand Down
4 changes: 3 additions & 1 deletion src/libvirt-xml-parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 681addd

Please sign in to comment.