consoles: Allow configuring VNC #1973

mvollmer · 2025-01-14T09:35:38Z

Via the "Consoles" card, when the machine is off.

This is the minimum to get #1795 finished, IMO, and we should not try to do more. Anything on top happens in #1972 and elsewhere, at its own pace.

tests, coverage

mvollmer · 2025-01-14T09:41:01Z

There is more work to be done here, of course:

Optional password reveal in the dialogs.
Explicit option to select "auto" for the port instead of having to input -1.
Input validation in the dialogs, e.g. check if address is valid, check if port is in the required range
Wording change, I think it should be "Listening address" instead of "VNC address", etc, since we are configuring the server, not the client.
tests, coverage

garrett · 2025-01-15T12:33:37Z

Can you add a screenshot of the current status please? Thanks!

mvollmer · 2025-01-15T13:33:27Z

Can you add a screenshot of the current status please? Thanks!

Yes! Thanks for having a look @garrett!

A runing virtual machine looks and behaves exactly like before. The console card of a not running machine has details about VNC.

Machine without VNC:

Machine with default VNC:

Machine with custom VNC:

Dialog for editing the VNC config:

Dialog for adding VNC:

(None of the above is meant to be final, of course.)

garrett · 2025-01-15T16:09:49Z

Thanks for adding the screenshots! I like what I'm seeing so far.

(None of the above is meant to be final, of course.)

It looks like a great start though, and I think it can be banged into resembling the mockups for this. Thanks again for picking up the work on Machines!

mvollmer · 2025-01-16T13:26:33Z

I think it can be banged into resembling the mockups for this.

I think I didn't see any mockups of a stopped machine, hmm. Is this mockup also meant for stopped machines?

mvollmer · 2025-01-17T09:17:42Z

A runing virtual machine looks and behaves exactly like before.

Sorry, that isn't true. A running machine without VNC now allows the selection of the VNC tab, which will look like this:

mvollmer · 2025-01-17T09:57:44Z

I have changed the information display a bit. It deemphasizes the "VNC" term and the listening address.

No VNC:

VNC with auto port:

VNC with custom port:

In the selector, "VNC console" is changed to "Graphical console (VNC)".

The idea is that Cockpit talks about support for a graphical console inside Cockpit. That happens to be implemented via VNC, but that's a detail. And the per-VM listening address is not that interesting. People will very likely leave it at its default, and are better off changing the global default in qemu.conf in any case. The port on the other hand is interesting. VMs might fail to start if they use the same port by accident, and it is a genuine per-VM config setting.

mvollmer · 2025-01-17T11:56:44Z

I have polished the dialogs some. They look like this now when everything is at default:

With a bogus port:

With a number but out of range:

So that people can edit them or add VNC if it is missing.

- The dialogs talk about "server" and "listening" to make that clearer. - There are now placeholder texts to explain the defaults - We use the empty string instead "-1" to signify automatic port assignment in the UI. - The port does validation of its value. - The password field takes the whole row and has can reveal its value.

If the inactive XML config contains a port but not a address, we would try to connect too early, before the address was actually filled in.

cockpituous · 2025-01-17T14:07:12Z

src/components/vm/consoles/consoles.jsx

+    function add_vnc() {
+        Dialogs.show(<AddVNC
+                         idPrefix="add-vnc"
+                         vm={vm} />);


These 4 added lines are not executed by any test.

cockpituous · 2025-01-17T14:07:12Z

src/components/vm/consoles/consoles.jsx

+    function edit_vnc() {
+        Dialogs.show(<EditVNCModal
+                         idPrefix="edit-vnc"
+                         consoleDetail={vnc}
+                         vmName={vm.name}
+                         vmId={vm.id}
+                         connectionName={vm.connectionName} />);


These 7 added lines are not executed by any test.

cockpituous · 2025-01-17T14:07:12Z

src/components/vm/consoles/vnc.jsx

+        if (!consoleDetail) {
+            return (
+                <div className="pf-v5-c-console__vnc">
+                    <EmptyState>
+                        <EmptyStateBody>
+                            {_("Graphical console not supported. Shut down the virtual machine to add support.")}
+                        </EmptyStateBody>
+                    </EmptyState>
+                </div>


These 9 added lines are not executed by any test.

cockpituous · 2025-01-17T14:07:12Z

src/components/vm/consoles/vncAdd.jsx

+    constructor(props) {
+        super(props);


These 2 added lines are not executed by any test.

cockpituous · 2025-01-17T14:07:12Z

src/components/vm/consoles/vncAdd.jsx

+        this.state = {
+            dialogError: undefined,
+            vncAddress: "",
+            vncPort: "",
+            vncPassword: "",
+            addVncInProgress: false,
+            validationErrors: { },


These 7 added lines are not executed by any test.

cockpituous · 2025-01-17T14:07:15Z

src/libvirtApi/domain.js

+    if (connectionName === "system")
+        options.superuser = "try";
+
+    return cockpit.spawn(args, options);


This added line is not executed by any test.

cockpituous · 2025-01-17T14:07:15Z

src/libvirtApi/domain.js

+export function domainChangeVncSettings({
+    connectionName,
+    vmName,
+    vncAddress,
+    vncPort,
+    vncPassword,


These 6 added lines are not executed by any test.

cockpituous · 2025-01-17T14:07:15Z

src/libvirtApi/domain.js

+    const options = { err: "message" };
+    if (connectionName === "system")
+        options.superuser = "try";


These 3 added lines are not executed by any test.

cockpituous · 2025-01-17T14:07:15Z

src/libvirtApi/domain.js

+    if (connectionName === "system")
+        options.superuser = "try";
+
+    const args = ["virt-xml", "-c", `qemu:///${connectionName}`, vmName, "--edit", "--graphics", `vnc,listen=${vncAddress},port=${vncPort},passwd=${vncPassword}`];


This added line is not executed by any test.

cockpituous · 2025-01-17T14:07:15Z

src/libvirtApi/domain.js

+
+    const args = ["virt-xml", "-c", `qemu:///${connectionName}`, vmName, "--edit", "--graphics", `vnc,listen=${vncAddress},port=${vncPort},passwd=${vncPassword}`];
+
+    return cockpit.spawn(args, options);


This added line is not executed by any test.

mvollmer mentioned this pull request Jan 14, 2025

Add "view, edit, and add Video devices" to detailspage #1795

Draft

mvollmer added the no-test label Jan 15, 2025

mvollmer force-pushed the console-vnc-edit branch from 2842ed0 to cd06f95 Compare January 15, 2025 13:46

mvollmer force-pushed the console-vnc-edit branch from cd06f95 to 28c5c41 Compare January 16, 2025 14:01

consoles: Add VNC dialogs

1a72730

mvollmer force-pushed the console-vnc-edit branch 2 times, most recently from f5583f6 to 78c6280 Compare January 16, 2025 15:07

mvollmer force-pushed the console-vnc-edit branch from 78c6280 to 5faf9fd Compare January 17, 2025 10:06

mvollmer added 2 commits January 17, 2025 14:56

consoles: Show VNC details also when VM is off

61d10d0

So that people can edit them or add VNC if it is missing.

mvollmer force-pushed the console-vnc-edit branch from 0ceb441 to 59ed7bf Compare January 17, 2025 12:57

mvollmer removed the no-test label Jan 17, 2025

consoles: Also wait for address when connecting to VNC

5a4dc36

If the inactive XML config contains a port but not a address, we would try to connect too early, before the address was actually filled in.

mvollmer force-pushed the console-vnc-edit branch from 59ed7bf to 5a4dc36 Compare January 17, 2025 13:46

cockpituous reviewed Jan 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

consoles: Allow configuring VNC #1973

consoles: Allow configuring VNC #1973

mvollmer commented Jan 14, 2025 •

edited

Loading

mvollmer commented Jan 14, 2025 •

edited

Loading

garrett commented Jan 15, 2025

mvollmer commented Jan 15, 2025 •

edited

Loading

garrett commented Jan 15, 2025

mvollmer commented Jan 16, 2025

mvollmer commented Jan 17, 2025

mvollmer commented Jan 17, 2025

mvollmer commented Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025

cockpituous Jan 17, 2025


		const args = ["virt-xml", "-c", `qemu:///${connectionName}`, vmName, "--edit", "--graphics", `vnc,listen=${vncAddress},port=${vncPort},passwd=${vncPassword}`];

		return cockpit.spawn(args, options);

consoles: Allow configuring VNC #1973

Are you sure you want to change the base?

consoles: Allow configuring VNC #1973

Conversation

mvollmer commented Jan 14, 2025 • edited Loading

mvollmer commented Jan 14, 2025 • edited Loading

garrett commented Jan 15, 2025

mvollmer commented Jan 15, 2025 • edited Loading

garrett commented Jan 15, 2025

mvollmer commented Jan 16, 2025

mvollmer commented Jan 17, 2025

mvollmer commented Jan 17, 2025

mvollmer commented Jan 17, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mvollmer commented Jan 14, 2025 •

edited

Loading

mvollmer commented Jan 14, 2025 •

edited

Loading

mvollmer commented Jan 15, 2025 •

edited

Loading