From 7b3d62bd63150d26fd0c70a1734bfd9a63897dfa Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 23 Sep 2026 21:26:54 -0400 Subject: [PATCH 1/2] libvirt ssh: Start the domain if it is shut off A persistent VM that was stopped (by `bcvk libvirt stop`, a host reboot, or shutting down from inside the guest) previously made `bcvk libvirt ssh` fail with a hint to run `virsh start`. The obvious next step is always to start it and retry, so just do that: start a shut off domain, say so on stderr (so that the output of a remote command stays clean), and wait for SSH with the same longer timeout `libvirt run --ssh-wait` uses, since the guest has to boot first. Other states (paused, crashed, ...) still produce an error, since it's less clear what the right action is there. This is only the auto-start half of the issue; stopping idle VMs automatically would need something inside the guest and is left for later. The new integration test boots a full VM, so it joins the tests that nextest runs one at a time. Related: https://github.com/bootc-dev/bcvk/issues/158 Generated-by: AI Signed-off-by: Colin Walters --- .config/nextest.toml | 2 +- .../src/tests/libvirt_verb.rs | 45 ++++++++++++++++++ crates/kit/src/libvirt/run.rs | 4 +- crates/kit/src/libvirt/ssh.rs | 47 +++++++++++++++++-- docs/src/man/bcvk-libvirt-ssh.md | 9 ++++ 5 files changed, 101 insertions(+), 6 deletions(-) diff --git a/.config/nextest.toml b/.config/nextest.toml index b5cf04fde..a8136be1e 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -46,7 +46,7 @@ filter = ''' test(/to_?disk/) | test(/^test_to_base_disk_/) | test(/^test_libvirt_run/) - | test(/^test_libvirt_(comprehensive|ssh_integration|port_forward_(xml|connectivity))/) + | test(/^test_libvirt_(comprehensive|ssh_integration|ssh_autostart|port_forward_(xml|connectivity))/) | test(/^test_base_disk_creation/) | test(=test_varlink_ephemeral_run_ps_and_ssh) ''' diff --git a/crates/integration-tests/src/tests/libvirt_verb.rs b/crates/integration-tests/src/tests/libvirt_verb.rs index ed74e9139..7d1a3eeb6 100644 --- a/crates/integration-tests/src/tests/libvirt_verb.rs +++ b/crates/integration-tests/src/tests/libvirt_verb.rs @@ -146,6 +146,51 @@ fn test_libvirt_ssh_integration() -> TestResult { } integration_test!(test_libvirt_ssh_integration); +/// Test that `bcvk libvirt ssh` starts a shut off domain +fn test_libvirt_ssh_autostart() -> TestResult { + let sh = shell()?; + let bck = get_bck_command()?; + let test_image = get_test_image(); + let label = LIBVIRT_INTEGRATION_TEST_LABEL; + let domain_name = format!("test-ssh-autostart-{}", random_suffix()); + + cleanup_domain(&domain_name); + defer! { + cleanup_domain(&domain_name); + } + + cmd!( + sh, + "{bck} libvirt run --name {domain_name} --label {label} --filesystem ext4 {test_image}" + ) + .run()?; + + cmd!(sh, "{bck} libvirt stop --force {domain_name}").run()?; + let state = cmd!(sh, "env LC_ALL=C virsh domstate {domain_name}").read()?; + assert_eq!(state.trim(), "shut off"); + + let output = cmd!(sh, "{bck} libvirt ssh {domain_name} -- echo hello") + .ignore_status() + .output()?; + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + output.status.success(), + "ssh to a shut off domain should start it and succeed: {stderr}" + ); + // The start notice must not end up in the remote command's output + assert_eq!(stdout.trim(), "hello"); + assert!( + stderr.contains("starting it"), + "should say that the domain is being started: {stderr}" + ); + + let state = cmd!(sh, "env LC_ALL=C virsh domstate {domain_name}").read()?; + assert_eq!(state.trim(), "running"); + Ok(()) +} +integration_test!(test_libvirt_ssh_autostart); + /// Comprehensive workflow test: creates a VM and tests multiple features /// This consolidates several smaller tests to reduce expensive disk image creation fn test_libvirt_comprehensive_workflow() -> TestResult { diff --git a/crates/kit/src/libvirt/run.rs b/crates/kit/src/libvirt/run.rs index 3ad7d4af1..29b9b33d4 100644 --- a/crates/kit/src/libvirt/run.rs +++ b/crates/kit/src/libvirt/run.rs @@ -20,8 +20,8 @@ use crate::libvirt::domain::VirtiofsFilesystem; use crate::utils::parse_memory_to_mb; use crate::xml_utils; -/// SSH wait timeout in seconds -const SSH_WAIT_TIMEOUT_SECONDS: u64 = 180; +/// SSH wait timeout in seconds, for a domain that is booting +pub(crate) const SSH_WAIT_TIMEOUT_SECONDS: u64 = 180; /// Transport type for updating from host container storage const UPDATE_FROM_HOST_TRANSPORT: &str = "containers-storage"; diff --git a/crates/kit/src/libvirt/ssh.rs b/crates/kit/src/libvirt/ssh.rs index 884e708f1..96477314c 100644 --- a/crates/kit/src/libvirt/ssh.rs +++ b/crates/kit/src/libvirt/ssh.rs @@ -24,6 +24,11 @@ const SSH_RETRY_TIMEOUT_SECS: u64 = 60; // Total time to retry SSH connections const SSH_POLL_DELAY_SECS: u64 = 1; // Delay between SSH attempts const SSH_SERVER_ALIVE_INTERVAL: u32 = 60; // Server alive interval in seconds +/// `virsh domstate` output for a running domain +const DOMAIN_STATE_RUNNING: &str = "running"; +/// `virsh domstate` output for a domain that is defined but not running +const DOMAIN_STATE_SHUT_OFF: &str = "shut off"; + /// Configuration options for SSH connection to libvirt domain #[derive(Debug, Parser)] pub struct LibvirtSshOpts { @@ -278,7 +283,7 @@ impl LibvirtSshOpts { return Err(eyre!("Domain '{}' not found", self.domain_name)); } let state = self.get_domain_state(global_opts)?; - if state != "running" { + if state != DOMAIN_STATE_RUNNING { return Err(eyre!( "Domain '{}' is not running (current state: {}). Start it first with: virsh start {}", self.domain_name, @@ -289,6 +294,36 @@ impl LibvirtSshOpts { Ok(()) } + /// Verify the domain exists, starting it if it is shut off. + /// + /// Returns `true` if the domain was started, in which case it + /// will still be booting. + fn ensure_domain_running(&self, global_opts: &crate::libvirt::LibvirtOptions) -> Result { + if !self.check_domain_exists(global_opts)? { + return Err(eyre!("Domain '{}' not found", self.domain_name)); + } + match self.get_domain_state(global_opts)?.as_str() { + DOMAIN_STATE_RUNNING => Ok(false), + DOMAIN_STATE_SHUT_OFF => { + // Use stderr so that the output of a remote command stays clean + if !self.suppress_output { + eprintln!("Domain '{}' is shut off; starting it", self.domain_name); + } + super::run::run_virsh_cmd( + global_opts.connect.as_deref(), + &["start", &self.domain_name], + &format!("Failed to start domain '{}'", self.domain_name), + )?; + Ok(true) + } + state => Err(eyre!( + "Domain '{}' is not running (current state: {}); only shut off domains are started automatically", + self.domain_name, + state + )), + } + } + /// Create temp key file and parse extra SSH options — shared setup for /// both the retry path and single-attempt tests. pub(crate) fn prepare_ssh_session( @@ -374,7 +409,13 @@ pub fn run_ssh_impl( ) -> Result<()> { debug!("Connecting to libvirt domain: {}", opts.domain_name); - opts.verify_domain_running(global_opts)?; + let started = opts.ensure_domain_running(global_opts)?; + // A domain we just started has to boot first + let ssh_timeout = if started { + super::run::SSH_WAIT_TIMEOUT_SECONDS + } else { + SSH_RETRY_TIMEOUT_SECS + }; let ssh_config = opts.extract_ssh_config(global_opts)?; @@ -405,7 +446,7 @@ pub fn run_ssh_impl( Err(_) => Ok(false), } }, - Duration::from_secs(SSH_RETRY_TIMEOUT_SECS), + Duration::from_secs(ssh_timeout), Duration::from_secs(SSH_POLL_DELAY_SECS), ) .map_err(|_| { diff --git a/docs/src/man/bcvk-libvirt-ssh.md b/docs/src/man/bcvk-libvirt-ssh.md index f5209d4db..af5aa0503 100644 --- a/docs/src/man/bcvk-libvirt-ssh.md +++ b/docs/src/man/bcvk-libvirt-ssh.md @@ -10,6 +10,10 @@ bcvk-libvirt-ssh - SSH to libvirt domain with embedded SSH key SSH to libvirt domain with embedded SSH key +If the domain is shut off, it is started first, and the connection is +made once SSH is reachable. Domains in other states (e.g. paused) are +not changed and result in an error. + # OPTIONS @@ -57,6 +61,11 @@ SSH into a running libvirt VM: bcvk libvirt ssh my-server +SSH into a stopped VM, starting it first: + + bcvk libvirt stop my-server + bcvk libvirt ssh my-server + Execute a command on the VM: bcvk libvirt ssh my-server 'systemctl status' From f8c9d44d473b76f81712758378cb8fe66e8b508a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 23 Sep 2026 22:59:55 -0400 Subject: [PATCH 2/2] libvirt start: Wait for the guest to boot before --ssh `libvirt start --ssh` connected with the 60s retry of `libvirt ssh`, which is meant for a guest that is already up. Right after `virsh start` the guest still has to boot, which can take longer, so wait for SSH with the same timeout as `libvirt run --ssh` and a `libvirt ssh` that just started the domain. Generated-by: AI Signed-off-by: Colin Walters --- crates/kit/src/libvirt/run.rs | 2 +- crates/kit/src/libvirt/start.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/crates/kit/src/libvirt/run.rs b/crates/kit/src/libvirt/run.rs index 29b9b33d4..a5775a114 100644 --- a/crates/kit/src/libvirt/run.rs +++ b/crates/kit/src/libvirt/run.rs @@ -374,7 +374,7 @@ impl LibvirtRunOpts { /// /// Uses the same `wait_for_readiness` polling loop as the ephemeral path /// and `run_ssh_impl`, just with a longer timeout for initial VM boot. -fn wait_for_ssh_ready( +pub(crate) fn wait_for_ssh_ready( global_opts: &crate::libvirt::LibvirtOptions, domain_name: &str, timeout_secs: u64, diff --git a/crates/kit/src/libvirt/start.rs b/crates/kit/src/libvirt/start.rs index 729188dd7..7082ee434 100644 --- a/crates/kit/src/libvirt/start.rs +++ b/crates/kit/src/libvirt/start.rs @@ -73,6 +73,14 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtStartOpts) println!("VM '{}' started successfully", opts.name); if opts.ssh { + // The guest has to boot first, so wait for SSH as long as + // `libvirt run --ssh` does rather than the shorter retry of `libvirt ssh` + crate::libvirt::run::wait_for_ssh_ready( + global_opts, + &opts.name, + crate::libvirt::run::SSH_WAIT_TIMEOUT_SECONDS, + )?; + // Use the libvirt SSH functionality directly let ssh_opts = crate::libvirt::ssh::LibvirtSshOpts { domain_name: opts.name,