Skip to content

feat(quickemu): enable SPICE display reconnection for running VMs#1847

Merged
flexiondotorg merged 1 commit intomasterfrom
spice
Jan 26, 2026
Merged

feat(quickemu): enable SPICE display reconnection for running VMs#1847
flexiondotorg merged 1 commit intomasterfrom
spice

Conversation

@flexiondotorg
Copy link
Member

Fixes #1370

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have performed a self-review of my code

Fixes #1370

Signed-off-by: Martin Wimpress <martin@wimpress.org>
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Confidence score: 3/5

  • configure_ports in quickemu unconditionally deletes .spice, which can break the auto-reconnect path by removing the port detection source
  • Severity is moderate (6/10) with a concrete reconnection failure risk, so there’s some merge risk despite limited scope
  • Pay close attention to quickemu - .spice deletion can cause auto-reconnect to fail.
Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="quickemu">

<violation number="1" location="quickemu:168">
P2: `configure_ports` deletes the `.spice` file unconditionally, but the reconnection logic later depends on that file to detect the running VM’s SPICE port. This causes the auto-reconnect path to fail when you run quickemu against an already-running VM. Only remove the file when starting a new VM (or leave it intact for running VMs).</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@flexiondotorg
Copy link
Member Author

Thanks for the review, but I believe the implementation is correct. Let me explain the execution flow:

Scenario 1: Starting a NEW VM

  1. VM_PID is empty (no running process)
  2. Code takes the if [ -z "${VM_PID}" ] branch (line 2789)
  3. vm_boot is called → configure_ports runs
  4. configure_ports deletes any stale .spice file (line 1414) — correct behaviour
  5. If SPICE is enabled, a new .spice file is created with the new port (line 1478)

Scenario 2: Reconnecting to an ALREADY RUNNING VM

  1. Lines 2734-2742 check the .pid file and validate the process is running via kill -0
  2. VM_PID is set to the running process ID
  3. Code takes the else branch (line 2800) — vm_boot is never called
  4. Since configure_ports is only called from within vm_boot, it never runs for reconnection
  5. The .spice file remains intact
  6. Lines 2805-2809 read the .spice file and set display="spice"
  7. start_viewer launches the SPICE viewer

The key insight: configure_ports is only invoked when booting a new VM. The reconnection path bypasses vm_boot entirely, so the .spice file is preserved and correctly used for reconnection.

@flexiondotorg flexiondotorg merged commit fbb2960 into master Jan 26, 2026
5 checks passed
@flexiondotorg flexiondotorg deleted the spice branch January 26, 2026 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Connect to a detached running vm

1 participant