ref:2398a0ac21095243883b3898b017e518b4bbf3e7

docs(journal): record the Phase 1 review findings, fixes and results

Refs #3, #4, #5, #6 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SHA: 2398a0ac21095243883b3898b017e518b4bbf3e7
Author: Cole Christensen <cole.christensen@gmail.com>
Date: 2026-09-13 02:21
Parents: e3c2386
1 files changed +91 -0
Type
JOURNAL.md +91 −0
@@ -519,3 +519,94 @@
rule's action names, SELinux/AppArmor, units running under a real systemd instance, and a GUI Moonlight client
seeing both hosts through real mDNS.
## 2026-09-12 — Phase 1 review findings (#3, #4, #5, #6)
Twenty review findings on `phase-1`, each first checked against the code (and QEMU 11.1.1's source where the finding
was about QEMU's behaviour), then fixed test-first where real. None was rejected outright; two were fixed differently
from the suggestion (below).
### Confirmed and fixed
- **Vulkan zero-copy crop/flip and cursor (#3).** Confirmed: `rgb2yuv.comp` flipped the whole backing texture after
adding the crop offset, while QEMU (`egl_fb_blit`), `copy_flipped` and the readback crop first, then flip inside the
rectangle; the shader now computes `uv.y = (src_offset.y + src_size.y - (d.y + 0.5) * scale.y) / tex_h` for
`y_invert` (identical to `1 - uv.y` for wlgrab's full-size buffers). `vulkan_encode.cpp` subtracts the crop offset
from the cursor (kmsgrab passes CRTC coordinates) while `sws_t::load_vram` doesn't, so the qemu display adds the
offset back for Vulkan only. Tests: descriptor-level cursor positions for Vulkan (buffer coordinates) and VAAPI
(display coordinates); `QemuDmabufGpuTest.VulkanCropsAndFlipsBottomUpScanoutLikeQemu` imports a udmabuf through
`vk_vram_t::convert` and checks the NV12 Y plane against the CPU conversion. **It skips here** (no `/dev/udmabuf`,
no Vulkan DMABUF import on WSL2), so the shader change is compiled (glslangValidator) but not pixel-verified.
- **Per-scanout frame allocation (#3, REQ-NFR-001).** Confirmed: `pixels.clear()` kept size 0, so every
ScanoutDMABUF re-ran `assign(w*h*4, 0)`. `resize_locked(..., allocate=false)` now frees the buffer
(`frame_memory()` stays 0 across scanouts in the test).
- **copy_if_newer on an empty frame (#3).** Confirmed by reading: kind() and copy_if_newer take the lock separately.
`copy_if_newer` now returns `size_changed` when the frame has no pixels.
- **Unreadable DMABUF mid-stream with a software encoder (#3).** Reproduced (capture test streamed on for 10 s);
`display_ram_t::snapshot` now returns `reinit`, so `qemu_display()` reports the error.
- **ScanoutMap always acknowledged (#3).** Confirmed in QEMU's `dbus_scanout_map` (fallback only on a failed call).
`display_listener_t::scanout_map` returns bool, the session replies `NotSupported`, and the store drops the
previous map so `Update` messages apply again.
- **ScanoutDMABUF2 with fewer fds than planes (#3).** Confirmed in `dbus_scanout_dmabuf_v2`. The session accepts
`1 <= fds <= num_planes` and gives planes without a handle a dup of the previous descriptor. (QEMU's own
`egl_dmabuf_export_texture` rejects -1 fds, so this only matters for other DMABUF sources.)
- **Relative mouse on an absolute guest (#5).** Reproduced with the fake (lagging MouseSet pulled the pointer back:
`abs 120 100` twice). The absolute branch of `move_to_locked` now records `last_own_motion` too.
- **Idle host devices with capture = qemu (#5).** Confirmed. `input_context_t` takes `host_pointer_devices`; with
`capture = qemu` libvirtualhid creates no keyboard, mouse, touchscreen or pen (also not on `refresh_*`), only
gamepads. Not observable on this host (no uinput), so the test checks the flag and null devices.
- **REQ-CMP-001 input coverage (#5).** Added: with `capture = x11` every platform input call and
`streaming_will_stop()` sends nothing to the fake QEMU, and pen/touch capability equals the libvirtualhid runtime's.
- **Guest volume lost per session (#4).** Reproduced (full-scale RMS after a new control). Volumes are cached
process-wide per (address, VM UUID, QEMU pid) and stream id, applied on `Init`, dropped on `Fini`. A QEMU whose pid
isn't known (p2p without it) relies on UUID+address; a stream id reused by a restarted QEMU with the same UUID and
address could inherit a volume, which needs the same heap address and is accepted.
- **libvirt default URI (#6).** Confirmed from libvirt's rules (non-root default is `qemu:///session`); not run
against real libvirt. `libvirt:<domain>` now means `qemu:///system` in Sunshine and in `sunshine-qemu-wait-vm`;
`?uri=` (empty) keeps libvirt's default.
- **XDG_CONFIG_HOME inherited by virsh/libvirt (#6).** Reasoned, not reproducible here (no libvirt). Both units use
`CONFIGURATION_DIRECTORY`. Deviation: the system unit keeps `HOME=<state dir>`, because `ProtectSystem=strict`
leaves only the state directory writable for driver caches, and a system-URI connection doesn't autospawn a daemon.
- **Packaging test negatives (#6).** Confirmed (`&& fail` returned 0 at the end). Fixed, and the runner now fails a
test that set a failure message; `SUNSHINE_QEMU_PKG_REQUIRE_TOOLS=1` turns skips into failures; the script is
CTest `sunshine_qemu_packaging`. No CI config exists yet (#9).
- **Stale QMP socket (#6).** Verified with QEMU 11.1.1: after `kill -9` the socket file stays; the helper now waits
for the `{"QMP"` greeting (python3; falls back to the file check without python3).
- **mDNS names (#6).** Confirmed (`vm.1` → `vm`). `mdns_service_name()` keeps UTF-8 up to 63 bytes, replaces
control characters, and keeps the old host-name rules when `sunshine_name` is the default (so FQDN host names don't
change for everyone). `--check` reports duplicate names (case-insensitive). Avahi acceptance of names with spaces
and dots was not exercised in the sandbox E2E (it uses `sq-vm-N`).
- **Docs (#6).** Install steps create `/var/lib/sunshine-qemu/$(systemd-escape <vm>)` and set credentials before the
first start, use `systemd-escape --template`, state that QMP access is code execution as QEMU's user, show socket
and bus access for the unit user (bus policy and setgid socket directory: written, not tested), and say the web UI
can't save settings for system instances. `--write` handles a missing final newline and `example.conf` leaves
`port` commented. Stale audio note in the guest README removed.
- **multi_instance relative paths (#6).** `E2E_STATE_LAYOUT=unit`: example.conf's relative paths with a
`CONFIGURATION_DIRECTORY` per instance.
### Fixed differently from the suggestion
- **native_pen_touch for capture = qemu (#5).** Real, but there is no D-Bus way to see whether the VM has
`virtio-multitouch-pci` (QEMU exports MultiTouch with MaxSlots 10 on every console), and a new qemu-specific key
would add config/web-UI surface. Instead `example.conf` sets `native_pen_touch = disabled` with an explanation, and
docs/qemu.md says when to enable it. A plain `capture = qemu` config with the default still advertises pen/touch:
this remains a deviation from #5's "fall back to absolute mouse emulation if the console lacks MultiTouch".
### Results
- `Qemu*` + `Mdns*`: 150 passed, 3 skipped (VAAPI render node, EGL and Vulkan DMABUF GPU tests) before the #6
changes; full Debug `test_sunshine`: 715 tests, 698 passed, 17 skipped, 0 failed tests; `cmake-build-noqemu`: 570
tests, 556 passed, 0 failed tests. Both exit 1 only for the known AudioTest, MouseHIDTest and EncoderTest setup.
- Red first: frame store (capacity 48 kept, `new_frame` from an empty frame), unreadable DMABUF (`ok` after 10 s),
input (`abs 120 100` repeated), audio (RMS 0.354 instead of 0.071), wait-vm/port/unit tests (6 failed against the
previous scripts). The session and Vulkan cursor tests were written together with the interface change.
- `tests/packaging/linux/qemu/test_sunshine_qemu.sh`: 16 ok (incl. systemd-analyze verify and shellcheck); CTest
`sunshine_qemu_packaging` passes.
- `multi_instance.sh`: PASS with `E2E_STATE_LAYOUT=unit` (buses) and with the default layout and VM 2 over QMP.
- Doxygen 1.18.0 as upstream: exit 0, no warnings.
- `e2e_stream.sh`, QEMU 11.1.1, Debug, software: `E2E_INPUT=absolute` PASS (keys, absolute position and the relative
move converted to an absolute position, click, wheel); `E2E_AUDIO=1 E2E_SESSIONS=2` PASS (999.997 Hz both
sessions, 0 silent blocks). `cmake-build-release-cuda` builds. Wait helper against a real QEMU 11.1.1 `-qmp`
socket: ready while QEMU runs, times out after `kill -9` left the socket file.
- Not verified here: the Vulkan and EGL DMABUF paths on a GPU, libvirt (URI default, XDG autospawn), units under a
real systemd, Avahi with non-ASCII or dotted names, the raw-QEMU bus policy and socket permissions.