Skip to content

Commit

Permalink
vm: Support for VM titles and descriptions
Browse files Browse the repository at this point in the history
Based on work by @Britz, thanks a lot!
  • Loading branch information
mvollmer committed May 22, 2024
1 parent e7dd5bb commit d9dc7b9
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 38 deletions.
4 changes: 1 addition & 3 deletions src/components/vm/vmActions.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -445,9 +445,7 @@ const VmActions = ({ vm, vms, onAddErrorNotification, isDetailsPage }) => {
dropdownItems.push(
<DropdownItem key={`${id}-rename`}
id={`${id}-rename`}
onClick={() => Dialogs.show(<RenameDialog vmName={vm.name}
vmId={vm.id}
connectionName={vm.connectionName} />)}>
onClick={() => Dialogs.show(<RenameDialog vm={vm} />)}>
{_("Rename")}
</DropdownItem>
);
Expand Down
3 changes: 2 additions & 1 deletion src/components/vm/vmDetailsPage.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,14 +67,15 @@ export const VmDetailsPage = ({
const vmActionsPageSection = (
<PageSection className="actions-pagesection" variant={PageSectionVariants.light} isWidthLimited>
<div className="vm-top-panel" data-vm-transient={!vm.persistent}>
<h2 className="vm-name">{vm.name}</h2>
<h2 className="vm-name">{vm.title ? vm.title + " (" + vm.name + ")" : vm.name}</h2>
<VmActions vm={vm}
config={config}
onAddErrorNotification={onAddErrorNotification}
isDetailsPage />
<VmNeedsShutdown vm={vm} />
<VmUsesSpice vm={vm} />
</div>
{ vm.description && <p className="vm-description">{vm.description}</p> }
</PageSection>
);

Expand Down
64 changes: 43 additions & 21 deletions src/components/vm/vmRenameDialog.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,47 +23,57 @@ import { Button } from "@patternfly/react-core/dist/esm/components/Button";
import { Form, FormGroup } from "@patternfly/react-core/dist/esm/components/Form";
import { Modal } from "@patternfly/react-core/dist/esm/components/Modal";
import { TextInput } from "@patternfly/react-core/dist/esm/components/TextInput";
import { TextArea } from "@patternfly/react-core/dist/esm/components/TextArea";

import { FormHelper } from 'cockpit-components-form-helper.jsx';
import { ModalError } from 'cockpit-components-inline-notification.jsx';
import { useDialogs } from 'dialogs.jsx';

import { isObjectEmpty } from '../../helpers.js';
import { domainRename } from '../../libvirtApi/domain.js';
import { domainRename, domainSetMetadata } from '../../libvirtApi/domain.js';

const _ = cockpit.gettext;

export const RenameDialog = ({ vmName, vmId, connectionName }) => {
export const RenameDialog = ({ vm }) => {
const Dialogs = useDialogs();
const [newName, setNewName] = useState(vmName);
const [name, setName] = useState(vm.name);
const [fullName, setFullName] = useState(vm.title || "");
const [description, setDescription] = useState(vm.description || "");
const [error, dialogErrorSet] = useState({});
const [submitted, setSubmitted] = useState(false);

function onRename() {
async function onRename() {
setSubmitted(true);

if (!newName)
if (!name)
return;

return domainRename({ connectionName, id: vmId, newName })
.then(() => {
Dialogs.close();
// If we are on the VMs details page change the URL to reflect the new name after the rename operation succeeded
if (cockpit.location.path.length > 0)
cockpit.location.go(["vm"], { ...cockpit.location.options, name: newName, connection: connectionName });
}, exc => {
dialogErrorSet({ dialogError: cockpit.format(_("Failed to rename VM $0"), vmName), dialogErrorDetail: exc.message });
});
try {
await domainSetMetadata(vm, { title: fullName, description });
if (name != vm.name)
await domainRename({ connectionName: vm.connectionName, id: vm.id, newName: name });
Dialogs.close();

// If we are on the VMs details page change the URL to
// reflect the new name after the rename operation
// succeeded
if (name != vm.name && cockpit.location.path.length > 0)
cockpit.location.go(["vm"], { ...cockpit.location.options, name, connection: vm.connectionName });
} catch (exc) {
dialogErrorSet({
dialogError: cockpit.format(_("Failed to rename VM $0"), vm.name),
dialogErrorDetail: exc.message
});
}
}

return (
<Modal position="top" variant="small" isOpen onClose={Dialogs.close}
title={cockpit.format(_("Rename VM $0"), vmName)}
title={cockpit.format(_("Rename VM $0"), vm.name)}
footer={
<>
<Button variant='primary'
id="rename-dialog-confirm"
isDisabled={!newName}
onClick={onRename}>
{_("Rename")}
</Button>
Expand All @@ -78,13 +88,25 @@ export const RenameDialog = ({ vmName, vmId, connectionName }) => {
}}
isHorizontal>
{!isObjectEmpty(error) && <ModalError dialogError={error.dialogError} dialogErrorDetail={error.dialogErrorDetail} />}
<FormGroup label={_("New name")}
<FormGroup label={_("Name")}
fieldId="rename-dialog-new-name">
<TextInput id='rename-dialog-new-name'
validated={submitted && !newName ? "error" : "default"}
value={newName}
onChange={(_, value) => setNewName(value)} />
<FormHelper helperTextInvalid={submitted && !newName && _("New name must not be empty")} />
validated={submitted && !name ? "error" : "default"}
value={name}
onChange={(_, value) => setName(value)} />
<FormHelper helperTextInvalid={submitted && !name && _("Name must not be empty")} />
</FormGroup>
<FormGroup label={_("Full name")}
fieldId="rename-dialog-full-name">
<TextInput id='rename-dialog-full-name'
value={fullName}
onChange={(_, value) => setFullName(value)} />
</FormGroup>
<FormGroup label={_("Description")}
fieldId="rename-dialog-description">
<TextArea id='rename-dialog-description'
value={description}
onChange={(_, value) => setDescription(value)} />
</FormGroup>
</Form>
</Modal>
Expand Down
14 changes: 9 additions & 5 deletions src/components/vms/hostvmslist.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,11 @@ const HostVmsList = ({ vms, config, ui, storagePools, actions, networks, onAddEr
<ListingTable aria-label={_("Virtual machines")}
variant='compact'
columns={[
{ title: _("Name"), header: true, props: { width: 25 } },
{ title: _("Connection"), props: { width: 25 } },
{ title: _("State"), props: { width: 25 } },
{ title: "", props: { width: 25, "aria-label": _("Actions") } },
{ title: _("Name"), header: true, props: { width: 20 } },
{ title: _("Full name"), props: { width: 20 } },
{ title: _("Connection"), props: { width: 20 } },
{ title: _("State"), props: { width: 20 } },
{ title: "", props: { width: 20, "aria-label": _("Actions") } },
]}
emptyCaption={_("No VM is running or defined on this host")}
rows={ combinedVmsFiltered
Expand All @@ -186,8 +187,11 @@ const HostVmsList = ({ vms, config, ui, storagePools, actions, networks, onAddEr
isDisabled={vm.isUi && !vm.createInProgress}
component="a"
href={'#' + cockpit.format("vm?name=$0&connection=$1", encodeURIComponent(vm.name), vm.connectionName)}
className="vm-list-item-name">{vm.name}</Button>
className="vm-list-item-name">
{vm.name}
</Button>
},
{ title: vm.title },
{ title: rephraseUI('connections', vm.connectionName) },
{
title: (
Expand Down
6 changes: 6 additions & 0 deletions src/libvirt-xml-parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,9 @@ export function parseDomainDumpxml(connectionName, domXml, objPath) {
const metadataElem = getSingleOptionalElem(domainElem, "metadata");

const name = domainElem.getElementsByTagName("name")[0].childNodes[0].nodeValue;
const uuid = domainElem.getElementsByTagName("uuid")[0].childNodes[0].nodeValue;
const title = domainElem.getElementsByTagName("title")[0]?.childNodes[0]?.nodeValue;
const description = domainElem.getElementsByTagName("description")[0]?.childNodes[0]?.nodeValue;
const id = objPath;
const osType = osTypeElem.childNodes[0].nodeValue;
const osBoot = parseDumpxmlForOsBoot(osBootElems);
Expand Down Expand Up @@ -292,7 +295,10 @@ export function parseDomainDumpxml(connectionName, domXml, objPath) {

return {
connectionName,
uuid,
name,
title,
description,
id,
osType,
osBoot,
Expand Down
18 changes: 18 additions & 0 deletions src/libvirtApi/domain.js
Original file line number Diff line number Diff line change
Expand Up @@ -890,6 +890,24 @@ export function domainSendNMI({
return call(connectionName, objPath, 'org.libvirt.Domain', 'InjectNMI', [0], { timeout, type: 'u' });
}

async function domainSetXML(vm, option, values) {
const opts = { err: "message" };
if (vm.connectionName === 'system')
opts.superuser = 'try';

const args = [];
for (const key in values)
args.push(key + '="' + values[key].replaceAll('"', '\\"') + '"');

await cockpit.spawn([
'virt-xml', '-c', `qemu:///${vm.connectionName}`, '--' + option, args.join(','), vm.uuid, '--edit'
], opts);
}

export async function domainSetMetadata(vm, values) {
await domainSetXML(vm, "metadata", values);
}

export function domainSetCpuMode({
name,
id: objPath,
Expand Down
57 changes: 50 additions & 7 deletions test/check-machines-lifecycle
Original file line number Diff line number Diff line change
Expand Up @@ -349,22 +349,65 @@ class TestMachinesLifecycle(machineslib.VirtualMachinesCase):
self.waitVmRow("new")
self.waitVmRow("old", "system", False)

# Rename to itself and expect error
self.goToVmPage("new")
self.performAction("new", "rename")

b.set_input_text("#rename-dialog-new-name", "new")
# Empty name should be an error

b.set_input_text("#rename-dialog-new-name", "")
b.click("#rename-dialog-confirm")
b.wait_in_text(".pf-v5-c-modal-box__body .pf-v5-c-alert.pf-m-danger", "Can't rename domain to itself")
b.wait_in_text(".pf-v5-c-helper-text__item.pf-m-error", "Name must not be empty")

self.goToVmPage("new")
self.performAction("new", "rename")
# Test errors from libvirt

b.set_input_text("#rename-dialog-new-name", "test%")
b.set_input_text("#rename-dialog-new-name", "/")
b.click("#rename-dialog-confirm")
b.wait_in_text(".pf-v5-c-alert__description", "name / cannot contain '/'")

# Set full name and description and use non-ASCII characters

b.set_input_text("#rename-dialog-new-name", 'test%')
# This is an attempted injection for the virt-xml invocation
b.set_input_text("#rename-dialog-full-name", '"full name",blah=yo')
b.set_input_text("#rename-dialog-description", 'Döscrüptiän كرة القدم')
b.click("#rename-dialog-confirm")
b.wait(lambda: "vm?name=test%25&connection=system" in b.eval_js("window.location.href"))
b.wait_text("h2.vm-name", "test%")
b.wait_text("h2.vm-name", '"full name",blah=yo (test%)')
b.wait_text(".vm-description", 'Döscrüptiän كرة القدم')
b.wait_not_present("#navbar-oops")

# Check that full name and description are picked up correctly by rename dialog

self.performAction("test\\%", "rename")
b.wait_val("#rename-dialog-new-name", 'test%')
b.wait_val("#rename-dialog-full-name", '"full name",blah=yo')
b.wait_val("#rename-dialog-description", 'Döscrüptiän كرة القدم')

# Rename machine to "--", which is allowed by libvirt and has
# the potential of breaking naive command line interactions.

b.set_input_text("#rename-dialog-new-name", '--')
b.set_input_text("#rename-dialog-full-name", 'Can you handle this?')
b.click("#rename-dialog-confirm")
b.wait(lambda: "vm?name=--&connection=system" in b.eval_js("window.location.href"))

# Rename back to something sane

self.performAction("--", "rename")
b.set_input_text("#rename-dialog-new-name", 'srv-1')
b.set_input_text("#rename-dialog-full-name", 'Server I')
b.set_input_text("#rename-dialog-description", '')
b.click("#rename-dialog-confirm")
b.wait(lambda: "vm?name=srv-1&connection=system" in b.eval_js("window.location.href"))

b.wait_text("h2.vm-name", 'Server I (srv-1)')
b.wait_not_present(".vm-description")

# Check overview
self.goToMainPage()
self.waitVmRow("srv-1")
b.wait_text('td[data-label="Full name"]', "Server I")

def testDelete(self):
b = self.browser
m = self.machine
Expand Down

0 comments on commit d9dc7b9

Please sign in to comment.