feat: support installed guests and distribution display defaults

dev/tools.nix

@@ -8,6 +8,9 @@ let
     url = "https://github.com/NixOS/nixpkgs/archive/7fd4004d0511991fff3a0524856a9be09e28c5f0.tar.gz";
     sha256 = "sha256-i5qk1kLiit98rRbh1AXmWBNT9QXC/y4l5Wk/u5EdpDQ=";
   }) {};
+  qemu = pkgs.qemu.overrideAttrs (old: {
+    patches = (old.patches or []) ++ [ ../patches/qemu-virtio-refresh-rate.patch ];
+  });
   viewer = pkgs.virt-viewer.overrideAttrs (old: {
     src = pkgs.fetchurl {
       url = "https://releases.pagure.org/virt-viewer/virt-viewer-11.0.tar.xz";
@@ -17,7 +20,7 @@ let
     doCheck = true;
   });
 in pkgs.mkShell {
-  packages = [ pkgs.python3 pkgs.qemu viewer graphics.labwc pkgs.foot
+  packages = [ pkgs.python3 qemu viewer graphics.labwc pkgs.foot
                pkgs.brightnessctl pkgs.wayland-utils pkgs.wlr-randr ];
-  passthru = { inherit viewer; labwc = graphics.labwc; };
+  passthru = { inherit viewer qemu; labwc = graphics.labwc; };
 }

hydra/cli.py

@@ -7,7 +7,7 @@ import subprocess
 import sys
 from pathlib import Path
 
-from .profiles import Profile
+from .profiles import Profile, load_defaults
 from .runtime import Runtime, RuntimeError
 from .session import launch_session, session_helper, view
 
@@ -17,16 +17,26 @@ def parser() -> argparse.ArgumentParser:
     cli.add_argument('--config', type=Path, default=Path(os.environ.get('XDG_CONFIG_HOME', Path.home() / '.config')) / 'hydra')
     cli.add_argument('--state', type=Path, default=Path(os.environ.get('XDG_STATE_HOME', Path.home() / '.local/state')) / 'hydra')
     cli.add_argument('--runtime', type=Path, default=Path(os.environ.get('XDG_RUNTIME_DIR', f'/tmp/hydra-{os.getuid()}')) / 'hydra')
+    cli.add_argument('--defaults', type=Path, default=Path('/etc/hydra/defaults.toml'),
+                     help='Distribution preferences for newly created profiles')
     commands = cli.add_subparsers(dest='command', required=True)
     add = commands.add_parser('add', help='Create a VM profile without starting it')
     add.add_argument('name')
-    add.add_argument('--iso', type=Path, required=True)
+    add.add_argument('--iso', type=Path)
     add.add_argument('--memory', type=int, default=4096)
     add.add_argument('--cpus', type=int, default=2)
     add.add_argument('--disk-size', default='40G')
     add.add_argument('--graphics', choices=('auto', 'software'), default='auto')
-    add.add_argument('--network', choices=('user', 'bridge'), default='user')
+    add.add_argument('--network', choices=('user', 'bridge'))
-    add.add_argument('--bridge', default='hydrabr0')
+    add.add_argument('--bridge')
+    add.add_argument('--width', type=int)
+    add.add_argument('--height', type=int)
+    add.add_argument('--refresh-millihz', type=int)
+    media = commands.add_parser('media', help='Attach or detach installation media while stopped')
+    media.add_argument('name')
+    selection = media.add_mutually_exclusive_group(required=True)
+    selection.add_argument('--iso', type=Path)
+    selection.add_argument('--detach', action='store_true')
     commands.add_parser('list', help='List configured VMs')
     status = commands.add_parser('status', help='Query VM state through private QMP')
     status.add_argument('name', nargs='?')
@@ -57,12 +67,18 @@ def main(argv: list[str] | None = None) -> int:
         elif args.command == '_session-helper':
             return session_helper(args.operation, args.manifest)
         elif args.command == 'add':
+            preferences = load_defaults(args.defaults)
+            preferences.update({key: getattr(args, key) for key in
+                                ('network', 'bridge', 'width', 'height', 'refresh_millihz')
+                                if getattr(args, key) is not None})
             profile = Profile(args.name, args.iso, args.memory, args.cpus, args.disk_size,
-                              args.graphics, args.network, args.bridge)
+                              args.graphics, **preferences)
             runtime.add(profile)
             result = {'name': profile.name, 'status': 'added'}
+        elif args.command == 'media':
+            result = runtime.media(args.name, None if args.detach else args.iso)
         elif args.command == 'list':
-            result = [{'name': profile.name, 'iso': str(profile.iso)} for profile in runtime.profiles()]
+            result = [{'name': profile.name, 'iso': str(profile.iso) if profile.iso else None} for profile in runtime.profiles()]
         elif args.command == 'status':
             result = runtime.status(args.name) if args.name else [runtime.status(p.name) for p in runtime.profiles()]
         elif args.command == 'stop':

hydra/profiles.py

@@ -16,24 +16,32 @@ def validate_name(name: str) -> str:
 @dataclass(frozen=True)
 class Profile:
     name: str
-    iso: Path
+    iso: Path | None = None
     memory: int = 4096
     cpus: int = 2
     disk_size: str = '40G'
     graphics: str = 'auto'
     network: str = 'user'
     bridge: str = 'hydrabr0'
+    width: int = 1280
+    height: int = 800
+    refresh_millihz: int = 0
 
     def __post_init__(self):
         validate_name(self.name)
-        iso = Path(self.iso).expanduser().resolve()
+        if self.iso is not None:
-        if not iso.is_file():
+            if not isinstance(self.iso, (str, Path)) or not str(self.iso):
-            raise ValueError(f'ISO does not exist or is not a file: {iso}')
+                raise ValueError('ISO must be a nonempty path or omitted')
-        object.__setattr__(self, 'iso', iso)
+            object.__setattr__(self, 'iso', Path(self.iso).expanduser().resolve())
         for key, upper in (('memory', 262144), ('cpus', 256)):
             value = getattr(self, key)
             if type(value) is not int or not 1 <= value <= upper:
                 raise ValueError(f'{key} must be an integer between 1 and {upper}')
+        for key in ('width', 'height'):
+            if type(getattr(self, key)) is not int or not 1 <= getattr(self, key) <= 16384:
+                raise ValueError(f'{key} must be an integer between 1 and 16384')
+        if type(self.refresh_millihz) is not int or not 0 <= self.refresh_millihz <= 1000000:
+            raise ValueError('refresh_millihz must be an integer between 0 and 1000000')
         if not isinstance(self.disk_size, str) or not re.fullmatch(r'[1-9][0-9]{0,5}[MGT]', self.disk_size):
             raise ValueError('disk_size must be a positive size with M, G or T suffix')
         if self.graphics not in ('auto', 'software'):
@@ -67,6 +75,22 @@ def save_profile(path: Path, profile: Profile) -> None:
     lines = []
     for field in Profile.__dataclass_fields__:
         value = getattr(profile, field)
+        if value is None:
+            continue
         lines.append(f'{field} = {value if type(value) is int else json.dumps(str(value), ensure_ascii=False)}\n')
     with path.open('x', encoding='utf-8') as stream:
         stream.write(''.join(lines))
+
+
+def load_defaults(path: Path) -> dict:
+    """Distribution preferences apply only when creating a new profile."""
+    try:
+        with Path(path).open('rb') as stream:
+            data = tomllib.load(stream)
+    except FileNotFoundError:
+        return {}
+    unknown = data.keys() - {'network', 'bridge', 'width', 'height', 'refresh_millihz'}
+    if unknown:
+        raise ValueError(f'Unknown defaults fields: {", ".join(sorted(unknown))}')
+    Profile('defaults-check', **data)
+    return data

hydra/runtime.py

@@ -11,7 +11,7 @@ import subprocess
 import tempfile
 import time
 import uuid
-from dataclasses import dataclass
+from dataclasses import dataclass, replace
 from pathlib import Path
 
 from .profiles import Profile, load_profile, save_profile, validate_name
@@ -64,15 +64,21 @@ def build_qemu_argv(profile: Profile, paths: VMPaths) -> list[str]:
             '-mon', 'chardev=qmp,mode=control']
     if accel == 'kvm':
         argv += ['-cpu', 'host']
-    for options in (
+    blockdevs = [
         {'driver': 'file', 'filename': str(paths.disk), 'node-name': 'disk-file'},
         {'driver': 'qcow2', 'file': 'disk-file', 'node-name': 'disk'},
+    ]
+    if profile.iso is not None:
+        blockdevs += [
         {'driver': 'file', 'filename': str(profile.iso), 'node-name': 'iso-file', 'read-only': True},
         {'driver': 'raw', 'file': 'iso-file', 'node-name': 'iso', 'read-only': True},
-    ):
+        ]
+    for options in blockdevs:
         argv += ['-blockdev', json.dumps(options)]
-    argv += ['-device', 'virtio-blk-pci,drive=disk', '-device', 'ide-cd,drive=iso',
+    argv += ['-device', 'virtio-blk-pci,drive=disk']
-             '-boot', 'order=dc', '-device', 'virtio-net-pci,netdev=net',
+    if profile.iso is not None:
+        argv += ['-device', 'ide-cd,drive=iso']
+    argv += ['-boot', 'order=dc' if profile.iso is not None else 'order=c', '-device', 'virtio-net-pci,netdev=net',
              '-netdev', 'user,id=net' if profile.network == 'user'
              else f'bridge,id=net,br={profile.bridge}',
              '-device', 'qemu-xhci,id=usb', '-device', 'usb-tablet', '-device', 'usb-kbd']
@@ -83,12 +89,15 @@ def build_qemu_argv(profile: Profile, paths: VMPaths) -> list[str]:
                  '-device', f'usb-redir,chardev=usbredir{channel}']
     spice = (f'unix=on,addr={_qemu_value(paths.spice)},disable-ticketing=on,'
              'disable-copy-paste=on,disable-agent-file-xfer=on')
+    mode = f',xres={profile.width},yres={profile.height}'
+    if profile.refresh_millihz:
+        mode += f',refresh_rate={profile.refresh_millihz}'
     render = _intel_render_node() if profile.graphics == 'auto' else None
     if render:
         spice += f',gl=on,rendernode={_qemu_value(render)}'
-        argv += ['-display', f'egl-headless,rendernode={_qemu_value(render)}', '-device', 'virtio-vga-gl']
+        argv += ['-display', f'egl-headless,rendernode={_qemu_value(render)}', '-device', 'virtio-vga-gl' + mode]
     else:
-        argv += ['-display', 'none', '-device', 'virtio-vga']
+        argv += ['-display', 'none', '-device', 'virtio-vga' + mode]
     return argv + ['-spice', spice]
 
 
@@ -125,9 +134,33 @@ class Runtime:
             _private_dir(directory)
 
     def add(self, profile: Profile) -> None:
+        self._validate_media(profile.iso)
         _private_dir(self.config)
         save_profile(self.config / f'{profile.name}.toml', profile)
 
+    @staticmethod
+    def _validate_media(iso: Path | None) -> None:
+        if iso is not None and not iso.is_file():
+            raise RuntimeError(f'ISO does not exist or is not a file: {iso}; attach valid media or detach it while stopped')
+
+    def media(self, name: str, iso: Path | None) -> dict:
+        paths = self.paths(name)
+        with self._locked(paths):
+            current = self.status(name)
+            if current['status'] != 'stopped':
+                raise RuntimeError(f'Cannot change media for {name}: {current["status"]}')
+            profile = replace(self.profile(name), iso=iso)
+            self._validate_media(profile.iso)
+            destination = self.config / f'{name}.toml'
+            if destination.is_symlink():
+                raise RuntimeError('Refusing to replace a symlink profile')
+            with tempfile.TemporaryDirectory(prefix=f'.{name}-', dir=self.config) as tmp:
+                temp = Path(tmp) / f'{name}.toml'
+                save_profile(temp, profile)
+                temp.chmod(0o600)
+                os.replace(temp, destination)
+        return {'name': name, 'iso': str(profile.iso) if profile.iso else None, 'status': 'media-updated'}
+
     def profiles(self) -> list[Profile]:
         if not self.config.exists():
             return []
@@ -234,12 +267,13 @@ class Runtime:
             temp.unlink(missing_ok=True)
 
     def start(self, name: str) -> dict:
-        profile = self.profile(name)
         paths = self.paths(name)
         with self._locked(paths):
+            profile = self.profile(name)
             current = self.status(name)
             if current['status'] != 'stopped':
                 raise RuntimeError(f'Cannot start {name}: {current["status"]}')
+            self._validate_media(profile.iso)
             for path in (paths.qmp, paths.spice):
                 if self._listening(path):
                     raise RuntimeError(f'Runtime socket is already in use: {path}')

patches/qemu-virtio-refresh-rate.patch

@@ -0,0 +1,71 @@
+--- a/include/hw/virtio/virtio-gpu.h
++++ b/include/hw/virtio/virtio-gpu.h
+@@ -128,6 +128,7 @@
+     uint32_t flags;
+     uint32_t xres;
+     uint32_t yres;
++    uint32_t refresh_rate;
+     uint64_t hostmem;
+     VirtIOGPUOutputList *outputs;
+ };
+@@ -173,7 +174,8 @@
+     DEFINE_PROP_BIT("edid", _state, _conf.flags, \
+                     VIRTIO_GPU_FLAG_EDID_ENABLED, true), \
+     DEFINE_PROP_UINT32("xres", _state, _conf.xres, 1280), \
+-    DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800)
++    DEFINE_PROP_UINT32("yres", _state, _conf.yres, 800), \
++    DEFINE_PROP_UINT32("refresh_rate", _state, _conf.refresh_rate, 0)
+ 
+ typedef struct VGPUDMABuf {
+     QemuDmaBuf *buf;
+--- a/hw/display/virtio-gpu-base.c
++++ b/hw/display/virtio-gpu-base.c
+@@ -64,7 +64,9 @@
+         .height_mm = g->req_state[scanout].height_mm,
+         .prefx = g->req_state[scanout].width,
+         .prefy = g->req_state[scanout].height,
+-        .refresh_rate = g->req_state[scanout].refresh_rate,
++        /* An explicit monitor refresh rate, in mHz, survives UI resizes. */
++        .refresh_rate = g->conf.refresh_rate ? g->conf.refresh_rate :
++                        g->req_state[scanout].refresh_rate,
+     };
+ 
+     for (output_idx = 0, node = g->conf.outputs;
+--- a/hw/display/edid-generate.c
++++ b/hw/display/edid-generate.c
+@@ -220,17 +220,17 @@
+     desc[5 + len] = '\n';
+ }
+ 
+-static void edid_desc_ranges(uint8_t *desc)
++static void edid_desc_ranges(uint8_t *desc, uint32_t refresh_rate,
++                             uint32_t horizontal_rate)
+ {
+     edid_desc_type(desc, 0xfd);
+ 
+-    /* vertical (50 -> 125 Hz) */
+-    desc[5] =  50;
+-    desc[6] = 125;
+-
+-    /* horizontal (30 -> 160 kHz) */
+-    desc[7] =  30;
+-    desc[8] = 160;
++    /* Preserve the legacy ranges and include the preferred mode. */
++    desc[5] = 50;
++    desc[6] = MIN(255, MAX(125, DIV_ROUND_UP(refresh_rate, 1000)));
++
++    desc[7] = 30;
++    desc[8] = MIN(255, MAX(160, DIV_ROUND_UP(horizontal_rate, 1000)));
+ 
+     /* max dot clock (2550 MHz) */
+     desc[9] = 2550 / 10;
+@@ -513,7 +513,8 @@
+      * so dta descriptor offsets don't move any more.
+      */
+ 
+-    edid_desc_ranges(desc);
++    edid_desc_ranges(desc, refresh_rate,
++                     timings.clock * 10000 / (info->prefx + timings.xblank));
+     desc = edid_desc_next(edid, dta, desc);
+ 
+     if (desc && info->name) {

tests/test_cli.py

@@ -0,0 +1,66 @@
+import json
+import subprocess
+import sys
+import tempfile
+import unittest
+from pathlib import Path
+
+from hydra.profiles import load_profile
+
+
+class CLITests(unittest.TestCase):
+    def setUp(self):
+        self.tmp = tempfile.TemporaryDirectory(prefix='hc-')
+        self.addCleanup(self.tmp.cleanup)
+        self.root = Path(self.tmp.name)
+        self.defaults = self.root / 'defaults.toml'
+        self.base = [sys.executable, 'bin/hydra', '--config', str(self.root / 'config'),
+                     '--state', str(self.root / 'state'), '--runtime', str(self.root / 'run'),
+                     '--defaults', str(self.defaults)]
+
+    def run_cli(self, *args):
+        return subprocess.run(self.base + list(args), capture_output=True, text=True, timeout=5)
+
+    def test_defaults_affect_new_profiles_only_and_explicit_options_win(self):
+        self.defaults.write_text('network="bridge"\nbridge="hydrabr0"\nwidth=1920\nheight=1080\nrefresh_millihz=144000\n')
+        added = self.run_cli('add', 'first', '--width', '1600', '--network', 'user', '--refresh-millihz', '0')
+        self.assertEqual(added.returncode, 0, added.stderr)
+        first = load_profile(self.root / 'config/first.toml')
+        self.assertEqual((first.network, first.width, first.height, first.refresh_millihz), ('user', 1600, 1080, 0))
+        added = self.run_cli('add', 'second')
+        self.assertEqual(added.returncode, 0, added.stderr)
+        second = load_profile(self.root / 'config/second.toml')
+        self.assertEqual((second.network, second.width, second.height, second.refresh_millihz), ('bridge', 1920, 1080, 144000))
+        self.defaults.write_text('width=1024\nnetwork="user"\n')
+        self.assertEqual(load_profile(self.root / 'config/second.toml'), second)
+        self.defaults.write_text('width="bad"\n')
+        listed = self.run_cli('list')
+        self.assertEqual(listed.returncode, 0, listed.stderr)
+        self.assertEqual(len(json.loads(listed.stdout)), 2)
+        self.assertEqual(self.run_cli('add', 'third').returncode, 1)
+        self.assertFalse((self.root / 'config/third.toml').exists())
+
+    def test_missing_defaults_use_developer_preferences_and_media_cli(self):
+        added = self.run_cli('add', 'guest')
+        self.assertEqual(added.returncode, 0, added.stderr)
+        profile = load_profile(self.root / 'config/guest.toml')
+        self.assertEqual((profile.network, profile.width, profile.height, profile.refresh_millihz), ('user', 1280, 800, 0))
+        iso = self.root / 'installer.iso'
+        iso.touch()
+        attached = self.run_cli('media', 'guest', '--iso', str(iso))
+        self.assertEqual(attached.returncode, 0, attached.stderr)
+        iso.unlink()
+        listed = self.run_cli('list')
+        self.assertEqual(listed.returncode, 0, listed.stderr)
+        self.assertEqual(self.run_cli('status', 'guest').returncode, 0)
+        detached = self.run_cli('media', 'guest', '--detach')
+        self.assertEqual(detached.returncode, 0, detached.stderr)
+        self.assertIsNone(load_profile(self.root / 'config/guest.toml').iso)
+
+    def test_unknown_or_invalid_distribution_defaults_are_rejected(self):
+        for content in ('iso="/private.iso"\n', 'network="public"\n', 'refresh_millihz=true\n', '[display]\nwidth=1920\n'):
+            self.defaults.write_text(content)
+            result = self.run_cli('add', 'guest')
+            self.assertEqual(result.returncode, 1, result.stderr)
+            self.assertNotIn('Traceback', result.stderr)
+            self.assertFalse((self.root / 'config/guest.toml').exists())

tests/test_profiles.py

@@ -33,9 +33,7 @@ class ProfilesTests(unittest.TestCase):
                 with self.subTest(field=field, value=value), self.assertRaises(ValueError):
                     Profile('guest', self.iso, **{field: value})
 
-    def test_missing_iso_and_duplicate_profile_preserve_existing(self):
+    def test_duplicate_profile_preserves_existing(self):
-        with self.assertRaises(ValueError):
-            Profile('guest', self.root / 'missing.iso')
         dest = self.root / 'guest.toml'
         save_profile(dest, Profile('guest', self.iso))
         original = dest.read_bytes()
@@ -43,6 +41,25 @@ class ProfilesTests(unittest.TestCase):
             save_profile(dest, Profile('guest', self.iso, memory=8192))
         self.assertEqual(dest.read_bytes(), original)
 
+    def test_disk_only_profile_roundtrips_without_media(self):
+        dest = self.root / 'guest.toml'
+        save_profile(dest, Profile('guest'))
+        self.assertIsNone(load_profile(dest).iso)
+        self.assertNotIn('iso =', dest.read_text())
+
+    def test_missing_media_does_not_block_loading_profile(self):
+        dest = self.root / 'guest.toml'
+        save_profile(dest, Profile('guest', self.iso))
+        self.iso.unlink()
+        self.assertEqual(load_profile(dest).iso, self.iso)
+
+    def test_display_bounds_and_types(self):
+        for field, values in {'width': (0, True, 16385), 'height': (0, '1080', 16385),
+                              'refresh_millihz': (-1, True, 1000001)}.items():
+            for value in values:
+                with self.subTest(field=field, value=value), self.assertRaises(ValueError):
+                    Profile('guest', self.iso, **{field: value})
+
     def test_profile_name_must_match_filename(self):
         path = self.root / 'guest.toml'
         path.write_text(f'name = "other"\niso = "{self.iso}"\n')

tests/test_runtime.py

@@ -88,6 +88,50 @@ class RuntimeTests(unittest.TestCase):
         except VMError:
             pass
 
+    def test_disk_boot_omits_media_and_requests_display(self):
+        profile = Profile('guest', width=1920, height=1080, refresh_millihz=144000,
+                          graphics='software')
+        argv = build_qemu_argv(profile, self.runtime.paths('guest'))
+        self.assertIn('virtio-vga,xres=1920,yres=1080,refresh_rate=144000', argv)
+        self.assertNotIn('ide-cd,drive=iso', argv)
+        self.assertIn('order=c', argv)
+        devices = [json.loads(argv[i+1]) for i, item in enumerate(argv) if item == '-blockdev']
+        self.assertEqual(len(devices), 2)
+        stock = build_qemu_argv(Profile('guest', graphics='software'), self.runtime.paths('guest'))
+        self.assertFalse(any('refresh_rate=' in arg for arg in stock))
+
+    def test_missing_iso_is_error_at_start_without_creating_disk(self):
+        self.iso.unlink()
+        with self.assertRaisesRegex(VMError, 'ISO'):
+            self.runtime.start('guest')
+        self.assertFalse(self.runtime.paths('guest').disk.exists())
+        self.assertEqual(self.runtime.status('guest')['status'], 'stopped')
+
+    def test_media_changes_only_while_stopped_and_preserves_disk(self):
+        paths = self.runtime.paths('guest')
+        self.runtime.start('guest')
+        disk = paths.disk.read_bytes()
+        config = (self.runtime.config / 'guest.toml').read_bytes()
+        for iso in (None, self.iso):
+            with self.assertRaises(VMError):
+                self.runtime.media('guest', iso)
+        self.assertEqual((self.runtime.config / 'guest.toml').read_bytes(), config)
+        self.iso.unlink()
+        self.assertEqual(self.runtime.profile('guest').iso, self.iso)
+        self.runtime.stop('guest', force=True)
+        self.runtime.media('guest', None)
+        self.assertIsNone(self.runtime.profile('guest').iso)
+        self.runtime.start('guest')
+        self.runtime.stop('guest', force=True)
+        self.assertEqual(paths.disk.read_bytes(), disk)
+        replacement = self.root / 'replacement.iso'
+        replacement.touch()
+        self.runtime.media('guest', replacement)
+        self.assertEqual(self.runtime.profile('guest').iso, replacement)
+        with self.assertRaisesRegex(VMError, 'ISO'):
+            self.runtime.media('guest', self.iso)
+        self.assertEqual(self.runtime.profile('guest').iso, replacement)
+
     def test_argv_is_private_and_comma_safe(self):
         paths = self.runtime.paths('guest')
         argv = build_qemu_argv(self.profile, paths)