From bf2acc0ab8a96cc202f1b8ff527fae8fadc12c26 Mon Sep 17 00:00:00 2001
From: Miroslav Vadkerti <mvadkert@redhat.com>
Date: Fri, 22 May 2026 01:57:19 +0200
Subject: [PATCH] Add `--graphics` option to replace `--no-graphic` and `--vnc`
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Replace the non-functional `--no-graphic` and `--vnc` CLI flags with a
single `--graphics {spice,vnc,none}` option that actually wires through
to the libvirt domain XML.

This is useful when one needs graphical access to the VM via a SPICE or
VNC client (e.g. `virt-viewer` or `remote-viewer`), for example to debug
boot issues or interact with a graphical desktop.

The old flags were accepted by argparse but never consumed — they had
zero effect on VM creation. The new option generates proper `<graphics>`
and `<video>` elements in the domain XML. For `qemu:///system`
connections, the graphics display binds to `127.0.0.1` for security.

Also removes the unused `self.vnc` and `self.graphics` attributes from
the `Instance` class.

Assisted-by: Claude Code
Signed-off-by: Miroslav Vadkerti <mvadkert@redhat.com>
---
 README.md                         | 21 +++++--
 conf/testcloud                    |  2 +-
 manpages/testcloud.1              |  7 +--
 test/test_domain_configuration.py | 91 +++++++++++++++++++++++++++++++
 testcloud/domain_configuration.py | 26 +++++++++
 testcloud/instance.py             |  2 -
 6 files changed, 135 insertions(+), 14 deletions(-)
 create mode 100644 test/test_domain_configuration.py

diff --git a/README.md b/README.md
index 5a5c159..1c1168d 100644
--- a/README.md
+++ b/README.md
@@ -216,13 +216,11 @@ new instance using **testcloud**.
 : To set the amount of VCPUS that will be available to the virtual
     machine.
 
-\--no-graphic
+\--graphics {spice,vnc,none}
 
-: This turns off the graphical display of the virtual machine.
-
-\--vnc
-
-: To open a VNC connection at the `:1` display of the instance.
+: Set the graphics display type. `spice` or `vnc` enables graphical
+    output viewable via `virt-viewer`/`remote-viewer`, `none` (default)
+    runs headless.
 
 -n, \--name NAME
 
@@ -252,6 +250,17 @@ new instance using **testcloud**.
 
 : To provide virtual iommu device
 
+### Connecting to graphical display
+
+When an instance is created with `--graphics spice` or `--graphics vnc`,
+you can connect to its display using `virt-viewer` or `remote-viewer`:
+
+    $ virt-viewer --connect qemu:///system <instance-name>
+
+For session mode:
+
+    $ virt-viewer --connect qemu:///session <instance-name>
+
 There are several additional options that can be used to create a
 new Coreos instance using **testcloud**.
 
diff --git a/conf/testcloud b/conf/testcloud
index e9ede30..7f635db 100644
--- a/conf/testcloud
+++ b/conf/testcloud
@@ -9,7 +9,7 @@ __testcloud()
     base_opts="instance image -h --help"
     image_opts="list destroy -h --help"
     instance_opts="list start stop remove create -c -h --help"
-    instance_create_opts="-h --ram --no-graphic --vnc --timeout --disksize"
+    instance_create_opts="-h --ram --graphics --timeout --disksize"
 
     echo ${COMP_WORDS[@]} >> /tmp/compwords
 
diff --git a/manpages/testcloud.1 b/manpages/testcloud.1
index ac3386b..7048eac 100644
--- a/manpages/testcloud.1
+++ b/manpages/testcloud.1
@@ -106,11 +106,8 @@ recognized.
 \fB--disksize DISKSIZE\fR
         Set the disk size of the instance VM (in GiB).
 
-\fB--vnc\fR
-        Open a VNC connection to the \fB:1\fR display of the instance VM.
-
-\fB--no-graphic\fR
-        Turn off the instance VM's graphical display.
+\fB--graphics {spice,vnc,none}\fR
+        Set the graphics display type. \fBspice\fR or \fBvnc\fR enables graphical output viewable via virt-viewer/remote-viewer, \fBnone\fR (default) runs headless.
 
 \fB--timeout TIMEOUT\fR
         Specify the time (in seconds) to wait for the instance boot to complete. To disable waiting time (default behaviour) set to \fB0\fR.
diff --git a/test/test_domain_configuration.py b/test/test_domain_configuration.py
new file mode 100644
index 0000000..2b33528
--- /dev/null
+++ b/test/test_domain_configuration.py
@@ -0,0 +1,91 @@
+# -*- coding: utf-8 -*-
+# Copyright 2025, Red Hat, Inc.
+# License: GPL-2.0+ <http://spdx.org/licenses/GPL-2.0+>
+# See the LICENSE file for more details on Licensing
+
+"""Tests for domain configuration, specifically GraphicsConfiguration."""
+
+from unittest.mock import patch
+
+import peewee as pw
+
+from testcloud import image
+from testcloud.domain_configuration import (
+    GraphicsConfiguration,
+    _get_default_domain_conf,
+)
+from testcloud.sql import DBImage, DB
+
+DB = pw.SqliteDatabase(":memory:")
+
+
+class TestGraphicsConfiguration(object):
+
+    def test_generate_spice(self):
+        gc = GraphicsConfiguration(graphics_type="spice")
+        xml = gc.generate()
+        assert "<graphics type='spice' autoport='yes'/>" in xml
+        assert "<model type='virtio'/>" in xml
+
+    def test_generate_vnc(self):
+        gc = GraphicsConfiguration(graphics_type="vnc")
+        xml = gc.generate()
+        assert "<graphics type='vnc' autoport='yes'/>" in xml
+
+    def test_generate_with_listen_address(self):
+        gc = GraphicsConfiguration(graphics_type="spice", listen_address="127.0.0.1")
+        xml = gc.generate()
+        assert "listen='127.0.0.1'" in xml
+
+    def test_generate_without_listen_address(self):
+        gc = GraphicsConfiguration(graphics_type="spice")
+        xml = gc.generate()
+        assert "listen=" not in xml
+
+
+class TestDefaultDomainConfGraphics(object):
+
+    def setup_method(self, method):
+        DB.bind([DBImage], bind_refs=False, bind_backrefs=False)
+        DB.connect()
+        DB.create_tables([DBImage])
+        self.patcher = patch('testcloud.sql.data_dir_changed', return_value=None)
+        self.mocked_method = self.patcher.start()
+
+    def teardown_method(self, method):
+        self.patcher.stop()
+        DB.drop_tables([DBImage])
+        DB.close()
+
+    def _make_image(self):
+        return image.Image("file:///someimage.qcow2")
+
+    def test_graphics_none(self):
+        conf = _get_default_domain_conf("test-vm", self._make_image(), graphics="none")
+        assert conf.graphics_configuration is None
+
+    def test_graphics_spice(self):
+        conf = _get_default_domain_conf("test-vm", self._make_image(), graphics="spice")
+        assert conf.graphics_configuration is not None
+        assert conf.graphics_configuration.graphics_type == "spice"
+
+    def test_graphics_vnc(self):
+        conf = _get_default_domain_conf("test-vm", self._make_image(), graphics="vnc")
+        assert conf.graphics_configuration is not None
+        assert conf.graphics_configuration.graphics_type == "vnc"
+
+    def test_graphics_spice_system_listen_address(self):
+        conf = _get_default_domain_conf(
+            "test-vm", self._make_image(), graphics="spice", connection="qemu:///system")
+        assert conf.graphics_configuration.listen_address == "127.0.0.1"
+
+    def test_graphics_in_generated_xml(self):
+        conf = _get_default_domain_conf("test-vm", self._make_image(), graphics="vnc")
+        xml = conf.generate()
+        assert 'type="vnc"' in xml
+        assert "<video>" in xml
+
+    def test_no_graphics_in_generated_xml(self):
+        conf = _get_default_domain_conf("test-vm", self._make_image(), graphics="none")
+        xml = conf.generate()
+        assert "<graphics" not in xml
diff --git a/testcloud/domain_configuration.py b/testcloud/domain_configuration.py
index e73b680..aa49b65 100644
--- a/testcloud/domain_configuration.py
+++ b/testcloud/domain_configuration.py
@@ -315,6 +315,22 @@ def generate(self):
             model=self.model
         )
 
+
+class GraphicsConfiguration:
+    def __init__(self, graphics_type="spice", listen_address=None) -> None:
+        self.graphics_type = graphics_type
+        self.listen_address = listen_address
+
+    def generate(self):
+        listen_attr = " listen='{}'".format(self.listen_address) if self.listen_address else ""
+        return """
+        <graphics type='{graphics_type}' autoport='yes'{listen_attr}/>
+        <video>
+            <model type='virtio'/>
+        </video>
+        """.format(graphics_type=self.graphics_type, listen_attr=listen_attr)
+
+
 def get_console_log_real_path(instance_uuid: str, suffix="console.log"):
     return os.path.join(config_data.CONSOLE_LOG_DIR, f"{instance_uuid}-{suffix}")
 
@@ -328,6 +344,7 @@ class DomainConfiguration:
     tpm_configuration: Optional[TPMConfiguration]
     virtiofs_configuration: list[VIRTIOFSConfiguration]
     iommu_configuration: Optional[IOMMUConfiguration]
+    graphics_configuration: Optional[GraphicsConfiguration]
     qemu_args: list[str]
     qemu_envs: dict[str, str]
     coreos: Optional[bool]
@@ -352,6 +369,7 @@ def __init__(self, name) -> None:
         self.console_log_file = None
         self.virtiofs_configuration = []
         self.iommu_configuration = None
+        self.graphics_configuration = None
         self.qemu_args = []
         self.qemu_envs = {}
         self.coreos = False
@@ -460,6 +478,7 @@ def generate(self) -> str:
                 {tpm}
                 {virtiofs_device}
                 {iommu}
+                {graphics}
             </devices>
             <qemu:commandline>
                 {qemu_args}
@@ -481,6 +500,7 @@ def generate(self) -> str:
             virtiofs_head=self.generate_virtiofs_head() if self.virtiofs_configuration else "",
             virtiofs_device=self.generate_virtiofs_mounts() if self.virtiofs_configuration else "",
             iommu=self.iommu_configuration.generate() if self.iommu_configuration else "",
+            graphics=self.graphics_configuration.generate() if self.graphics_configuration else "",
             qemu_args=self.get_qemu_args(),
             qemu_envs=self.get_qemu_envs(),
         )
@@ -505,6 +525,7 @@ def _get_default_domain_conf(
     virtiofs_source: Optional[str] = None,
     virtiofs_target: Optional[str] = None,
     iommu: bool = False,
+    graphics: str = "none",
 ):
 
     desired_arch = desired_arch or platform.machine()
@@ -577,4 +598,9 @@ def _get_default_domain_conf(
     if iommu:
         domain_configuration.iommu_configuration = IOMMUConfiguration()
 
+    if graphics in ("spice", "vnc"):
+        listen_address = "127.0.0.1" if connection == "qemu:///system" else None
+        domain_configuration.graphics_configuration = GraphicsConfiguration(
+            graphics_type=graphics, listen_address=listen_address)
+
     return domain_configuration
diff --git a/testcloud/instance.py b/testcloud/instance.py
index 0b60ed8..051ca14 100644
--- a/testcloud/instance.py
+++ b/testcloud/instance.py
@@ -249,8 +249,6 @@ def __init__(
         self.connection = connection
         self.pci_net = None
 
-        self.vnc = False
-        self.graphics = False
         self.hostname = hostname if hostname else config_data.HOSTNAME
 
         self.image_path = os.path.join(config_data.DATA_DIR, "instances", self.name, self.name + "-local.qcow2")