libvirt: Retry virsh when the session daemon is restarting - #394
Conversation
| //! For `qemu:///session`, the libvirt client auto-spawns a per-user daemon | ||
| //! (`libvirtd` or `virtqemud`) with `--timeout=120`, which exits after two | ||
| //! idle minutes. On exit the daemon removes its socket first and releases | ||
| //! its pidfile only once its drivers have shut down. A client connecting in |
There was a problem hiding this comment.
This is clearly just a workaround for a libvirt bug. Don't make workarounds without actually looking for e.g. bugs filed upstream or digging into the upstream code - if a bug doesn't exist upstream, a human or agentic process should queue one.
We need to propose a review checklist item for this.
Do that triage and update the comment here
There was a problem hiding this comment.
Triage: no upstream report or fix exists (checked the libvirt GitLab tracker, devel/users list archives and commits through v12.8.0); the shutdown order (socket unlinked in virNetDaemonRun(), pidfile released at the end of main()) and the client spawn-once-then-poll-5s loop are unchanged on master, and a pidfile-lock reproducer fails the same way on 11.10.0. Draft upstream report with reproducer and suggested fixes for you to file: https://gist.github.com/cgwalters-bot/47081fd1fdf2333b3fe83c79f4a11887
Updated the virsh.rs comment to name the upstream code paths, link the draft, and say it goes once bcvk's supported libvirt has a fix; squashed into 95d7582 (your sign-off kept). Checklist item landed in cgwalters-bot/homegit#70.
There was a problem hiding this comment.
OK yeah this would need some design work to fix upstream, probably cleanest would be libvirt to use the same socket activation it uses on the system...
There was a problem hiding this comment.
Agreed; I updated the draft upstream report to propose shipping virtqemud.socket/libvirtd.socket as systemd user units (the daemon already handles activation in session mode and never unlinks activated sockets; libvirt just installs no user units, cf. libvirt#267): https://gist.github.com/cgwalters-bot/47081fd1fdf2333b3fe83c79f4a11887
CI intermittently fails bcvk libvirt commands with "Failed to connect socket to '/run/user/1001/libvirt/libvirt-sock': No such file or directory" (bootc-dev/bootc#1843), typically right after a step that used no libvirt for two minutes, such as creating a base disk. The libvirt client auto-spawns the qemu:///session daemon with --timeout=120. When that daemon exits on idle it removes its socket first and releases its pidfile only after its drivers shut down. A client connecting in that window spawns a new daemon, which exits at once because the pidfile is still held, then waits five seconds for a socket nobody will create. A connection failure means virsh never ran its command, so it is safe to run it again; the retry spawns the daemon anew once the old one is gone. Route every virsh invocation through one VirshCommand type that does that for session connections, with a bounded backoff. It doesn't deref to Command, so a call chain can't bypass the retry. Generated-by: AI Signed-off-by: Colin Walters <walters@verbum.org>
A workaround for another project's bug that doesn't say which bug it works around never gets removed, and nobody tells upstream. cgwalters flagged this on bcvk's virsh retry for the libvirt session daemon race (bootc-dev/bcvk#394), where no upstream report existed. Generated-by: AI
df4ffed to
95d7582
Compare
bcvk and bootc CI intermittently fail with "Failed to connect socket to '/run/user/1001/libvirt/libvirt-sock': No such file or directory" (bootc-dev/bootc#1843). The libvirt client auto-spawns the session daemon with --timeout=120; when it exits on idle it removes its socket before it releases its pidfile, and a client connecting in between spawns a daemon that exits at once and then gives up after five seconds. It shows up right after test steps that went two minutes without touching libvirt. bcvk now retries in that case (bootc-dev/bcvk#394), but a daemon that never idles out avoids the window entirely, including for the pinned bcvk release. Rather than starting a daemon by hand, connect once (which spawns it, or reuses one that is already running) and turn off its idle timeout with virt-admin, so this works whichever daemon the client picked and is safe to run twice. The test workflow now checks that the daemon is still the same process well past 120 idle seconds and that bcvk can talk to it. Generated-by: AI
bcvk and bootc CI intermittently fail with "Failed to connect socket to '/run/user/1001/libvirt/libvirt-sock': No such file or directory" (bootc-dev/bootc#1843). The libvirt client auto-spawns the session daemon with --timeout=120; when it exits on idle it removes its socket before it releases its pidfile, and a client connecting in between spawns a daemon that exits at once and then gives up after five seconds. It shows up right after test steps that went two minutes without touching libvirt. bcvk now retries in that case (bootc-dev/bcvk#394), but a daemon that never idles out avoids the window entirely, including for the pinned bcvk release. Rather than starting a daemon by hand, connect once (which spawns it, or reuses one that is already running) and turn off its idle timeout with virt-admin, so this works whichever daemon the client picked and is safe to run twice. The test workflow now checks that the daemon is still the same process well past 120 idle seconds and that bcvk can talk to it. Generated-by: AI
The libvirt client auto-spawns the
qemu:///sessiondaemon with--timeout=120. When it exits on idle it removes its socket before releasing its pidfile, so a client connecting in that window spawns a daemon that exits at once and then fails after 5s withFailed to connect socket to '.../libvirt-sock': No such file or directory. In both recent CI failures the first libvirt call came right after two minutes without one (base disk install in bootc's plan-01, ephemeral tests in bcvk#393's partition 1), and several consecutive virsh calls each failed after exactly 5s.All virsh invocations now go through one
VirshCommandtype, which reruns virsh with a bounded backoff (60s) when it failed to connect to a session daemon. A connect failure means virsh never ran the command, so this is safe.Testing, on a 16-core RHEL 10 devspace (libvirt 11.10, virtqemud):
libvirt listfails after 5.0s with the CI error; this branch succeeds after 16.9s. With 6 concurrentbcvk libvirt listand a 20s hold: 0.19.0 6/6 failed, this branch 6/6 passed.cargo test -p bcvk(88 + 22 passed, including new data-driven unit tests for the retry and error matching),cargo clippy --release,cargo fmt --check.test_libvirt_list_functionality,test_libvirt_error_handling,test_to_base_disk_with_filesystem: passed.Fixes: bootc-dev/bootc#1843
The
Signed-off-by: Colin Walters <walters@verbum.org>on these commits was added on cgwalters's approval of the review draft: cgwalters-forge#11 (review)Generated-by: https://github.com/cgwalters/#llms