From a465f186772843f54698324931dcd095659397b0 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 20 Oct 2025 11:38:13 -0400 Subject: [PATCH] to-disk: Add support for --karg For the same reason we propagate other options. Note that we had incorrectly included karg in CommonVmOptions but that can't work without direct kernel boot. Signed-off-by: Colin Walters --- .../src/tests/libvirt_verb.rs | 20 +++++---- crates/kit/src/cache_metadata.rs | 43 +++++-------------- crates/kit/src/install_options.rs | 8 ++++ crates/kit/src/libvirt/base_disks.rs | 7 +-- crates/kit/src/libvirt/run.rs | 1 - crates/kit/src/libvirt/upload.rs | 5 --- crates/kit/src/libvirt_upload_disk.rs | 5 --- crates/kit/src/run_ephemeral.rs | 8 ++-- crates/kit/src/to_disk.rs | 6 +-- docs/src/man/bcvk-ephemeral-run-ssh.md | 8 ++-- docs/src/man/bcvk-ephemeral-run.md | 8 ++-- docs/src/man/bcvk-libvirt-list-volumes.md | 2 +- docs/src/man/bcvk-libvirt-run.md | 4 ++ docs/src/man/bcvk-libvirt-upload.md | 8 ++-- docs/src/man/bcvk-to-disk.md | 8 ++-- 15 files changed, 59 insertions(+), 82 deletions(-) diff --git a/crates/integration-tests/src/tests/libvirt_verb.rs b/crates/integration-tests/src/tests/libvirt_verb.rs index 84d55d0..6abc2fa 100644 --- a/crates/integration-tests/src/tests/libvirt_verb.rs +++ b/crates/integration-tests/src/tests/libvirt_verb.rs @@ -356,6 +356,8 @@ pub fn test_libvirt_run_ssh_full_workflow() { LIBVIRT_INTEGRATION_TEST_LABEL, "--filesystem", "ext4", + "--karg", + "bcvk.test-install-karg=1", &test_image, ]) .expect("Failed to run libvirt run with SSH"); @@ -375,9 +377,9 @@ pub fn test_libvirt_run_ssh_full_workflow() { println!("Waiting for VM to boot and SSH to become available..."); std::thread::sleep(std::time::Duration::from_secs(30)); - // Test SSH connection with simple command - println!("Testing SSH connection: echo 'hello world'"); - let ssh_output = run_bcvk(&["libvirt", "ssh", &domain_name, "--", "echo", "hello world"]) + // Test SSH connection and read kernel command line + println!("Testing SSH connection and validating karg"); + let ssh_output = run_bcvk(&["libvirt", "ssh", &domain_name, "--", "cat", "/proc/cmdline"]) .expect("Failed to run libvirt ssh command"); println!("SSH stdout: {}", ssh_output.stdout); @@ -388,17 +390,19 @@ pub fn test_libvirt_run_ssh_full_workflow() { // Check SSH results if !ssh_output.success() { - panic!("SSH connection failed: {}", ssh_output.stderr); + panic!( + "SSH connection failed: {}\nkernel cmdline: {}", + ssh_output.stderr, ssh_output.stdout + ); } - // Verify we got the expected output + // Verify we got the expected karg in /proc/cmdline assert!( - ssh_output.stdout.contains("hello world"), - "Expected 'hello world' in SSH output. Got: {}", + ssh_output.stdout.contains("bcvk.test-install-karg=1"), + "Expected bcvk.test-install-karg=1 in kernel cmdline.\nActual cmdline: {}", ssh_output.stdout ); - println!("✓ Successfully executed 'echo hello world' via SSH"); println!("✓ Full libvirt run + SSH workflow test passed"); } diff --git a/crates/kit/src/cache_metadata.rs b/crates/kit/src/cache_metadata.rs index 9288022..db76070 100644 --- a/crates/kit/src/cache_metadata.rs +++ b/crates/kit/src/cache_metadata.rs @@ -153,13 +153,13 @@ impl DiskImageMetadata { impl DiskImageMetadata { /// Create new metadata from InstallOptions and image digest - pub fn from(options: &InstallOptions, image: &str, kernel_args: &[String]) -> Self { + pub fn from(options: &InstallOptions, image: &str) -> Self { Self { version: 1, digest: image.to_owned(), filesystem: options.filesystem.clone(), root_size: options.root_size.clone(), - kernel_args: kernel_args.to_vec(), + kernel_args: options.karg.clone(), composefs_native: options.composefs_native, } } @@ -180,7 +180,6 @@ pub fn check_cached_disk( path: &Path, image_digest: &str, install_options: &InstallOptions, - kernel_args: &[String], ) -> Result> { if !path.exists() { tracing::debug!("Disk image {:?} does not exist", path); @@ -188,7 +187,7 @@ pub fn check_cached_disk( } // Create metadata for the current request to compute expected hash - let expected_meta = DiskImageMetadata::from(install_options, image_digest, kernel_args); + let expected_meta = DiskImageMetadata::from(install_options, image_digest); let expected_hash = expected_meta.compute_cache_hash(); // Read the cache hash from the disk image @@ -241,26 +240,16 @@ mod tests { let install_options1 = InstallOptions { filesystem: Some("ext4".to_string()), root_size: Some("20G".to_string()), - storage_path: None, - composefs_native: false, + ..Default::default() }; - let metadata1 = DiskImageMetadata::from( - &install_options1, - "sha256:abc123", - &["console=ttyS0".to_string()], - ); + let metadata1 = DiskImageMetadata::from(&install_options1, "sha256:abc123"); let install_options2 = InstallOptions { filesystem: Some("ext4".to_string()), root_size: Some("20G".to_string()), - storage_path: None, - composefs_native: false, + ..Default::default() }; - let metadata2 = DiskImageMetadata::from( - &install_options2, - "sha256:abc123", - &["console=ttyS0".to_string()], - ); + let metadata2 = DiskImageMetadata::from(&install_options2, "sha256:abc123"); // Same inputs should generate same hash assert_eq!( @@ -272,14 +261,9 @@ mod tests { let install_options3 = InstallOptions { filesystem: Some("ext4".to_string()), root_size: Some("20G".to_string()), - storage_path: None, - composefs_native: false, + ..Default::default() }; - let metadata3 = DiskImageMetadata::from( - &install_options3, - "sha256:xyz789", - &["console=ttyS0".to_string()], - ); + let metadata3 = DiskImageMetadata::from(&install_options3, "sha256:xyz789"); assert_ne!( metadata1.compute_cache_hash(), @@ -290,14 +274,9 @@ mod tests { let install_options4 = InstallOptions { filesystem: Some("xfs".to_string()), root_size: Some("20G".to_string()), - storage_path: None, - composefs_native: false, + ..Default::default() }; - let metadata4 = DiskImageMetadata::from( - &install_options4, - "sha256:abc123", - &["console=ttyS0".to_string()], - ); + let metadata4 = DiskImageMetadata::from(&install_options4, "sha256:abc123"); assert_ne!( metadata1.compute_cache_hash(), diff --git a/crates/kit/src/install_options.rs b/crates/kit/src/install_options.rs index 8db38a4..961d83e 100644 --- a/crates/kit/src/install_options.rs +++ b/crates/kit/src/install_options.rs @@ -29,6 +29,10 @@ pub struct InstallOptions { )] pub storage_path: Option, + #[clap(long)] + /// Set a kernel argument + pub karg: Vec, + /// Default to composefs-native storage #[clap(long)] pub composefs_native: bool, @@ -49,6 +53,10 @@ impl InstallOptions { args.push(root_size.clone()); } + for k in self.karg.iter() { + args.push(format!("--karg={k}")); + } + if self.composefs_native { args.push("--composefs-native".to_owned()); } diff --git a/crates/kit/src/libvirt/base_disks.rs b/crates/kit/src/libvirt/base_disks.rs index e17561e..ab9c9f6 100644 --- a/crates/kit/src/libvirt/base_disks.rs +++ b/crates/kit/src/libvirt/base_disks.rs @@ -17,10 +17,9 @@ pub fn find_or_create_base_disk( source_image: &str, image_digest: &str, install_options: &InstallOptions, - kernel_args: &[String], connect_uri: Option<&str>, ) -> Result { - let metadata = DiskImageMetadata::from(install_options, image_digest, kernel_args); + let metadata = DiskImageMetadata::from(install_options, image_digest); let cache_hash = metadata.compute_cache_hash(); // Extract short hash for filename (first 16 chars after "sha256:") @@ -45,7 +44,6 @@ pub fn find_or_create_base_disk( base_disk_path.as_std_path(), image_digest, install_options, - kernel_args, )? .is_ok() { @@ -68,7 +66,6 @@ pub fn find_or_create_base_disk( source_image, image_digest, install_options, - kernel_args, connect_uri, )?; @@ -81,7 +78,6 @@ fn create_base_disk( source_image: &str, image_digest: &str, install_options: &InstallOptions, - kernel_args: &[String], connect_uri: Option<&str>, ) -> Result<()> { use crate::run_ephemeral::CommonVmOpts; @@ -136,7 +132,6 @@ fn create_base_disk( temp_disk_path.as_std_path(), image_digest, install_options, - kernel_args, ) .context("Querying cached disk")?; diff --git a/crates/kit/src/libvirt/run.rs b/crates/kit/src/libvirt/run.rs index 8606d78..4746cef 100644 --- a/crates/kit/src/libvirt/run.rs +++ b/crates/kit/src/libvirt/run.rs @@ -179,7 +179,6 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtRunOpts) - &opts.image, &image_digest, &opts.install, - &[], // kernel_args connect_uri, ) .with_context(|| "Failed to find or create base disk")?; diff --git a/crates/kit/src/libvirt/upload.rs b/crates/kit/src/libvirt/upload.rs index b40adfc..e2960c6 100644 --- a/crates/kit/src/libvirt/upload.rs +++ b/crates/kit/src/libvirt/upload.rs @@ -42,10 +42,6 @@ pub struct LibvirtUploadOpts { /// Number of vCPUs for installation VM #[clap(long)] pub vcpus: Option, - - /// Additional kernel arguments for installation - #[clap(long)] - pub karg: Vec, } impl LibvirtUploadOpts { @@ -219,7 +215,6 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtUploadOpts common: crate::run_ephemeral::CommonVmOpts { memory: opts.memory.clone(), vcpus: opts.vcpus, - kernel_args: opts.karg.clone(), ..Default::default() }, ..Default::default() diff --git a/crates/kit/src/libvirt_upload_disk.rs b/crates/kit/src/libvirt_upload_disk.rs index 8c82cde..b2fda00 100644 --- a/crates/kit/src/libvirt_upload_disk.rs +++ b/crates/kit/src/libvirt_upload_disk.rs @@ -44,10 +44,6 @@ pub struct LibvirtUploadDiskOpts { #[clap(long)] pub vcpus: Option, - /// Additional kernel arguments for installation - #[clap(long)] - pub karg: Vec, - /// Skip uploading to libvirt (useful for testing) #[clap(long)] pub skip_upload: bool, @@ -290,7 +286,6 @@ pub fn run(opts: LibvirtUploadDiskOpts) -> Result<()> { common: crate::run_ephemeral::CommonVmOpts { memory: opts.memory.clone(), vcpus: opts.vcpus, - kernel_args: opts.karg.clone(), ..Default::default() }, ..Default::default() diff --git a/crates/kit/src/run_ephemeral.rs b/crates/kit/src/run_ephemeral.rs index 072b095..bc9ad8e 100644 --- a/crates/kit/src/run_ephemeral.rs +++ b/crates/kit/src/run_ephemeral.rs @@ -165,9 +165,6 @@ pub struct CommonVmOpts { #[clap(long, help = "Number of vCPUs")] pub vcpus: Option, - #[clap(long = "karg", help = "Additional kernel command line arguments")] - pub kernel_args: Vec, - #[clap(long, help = "Enable console output to terminal for debugging")] pub console: bool, @@ -257,6 +254,9 @@ pub struct RunEphemeralOpts { help = "Mount disk file as virtio-blk device at /dev/disk/by-id/virtio-" )] pub mount_disk_files: Vec, + + #[clap(long = "karg", help = "Additional kernel command line arguments")] + pub kernel_args: Vec, } /// Launch privileged container with QEMU+KVM for ephemeral VM, spawning as subprocess. @@ -982,7 +982,7 @@ StandardOutput=file:/dev/virtio-ports/executestatus kernel_cmdline.push("ds=iid-datasource-none".to_string()); } - kernel_cmdline.extend(opts.common.kernel_args.clone()); + kernel_cmdline.extend(opts.kernel_args.clone()); // TODO allocate unlinked unnamed file and pass via fd let mut tmp_swapfile = None; diff --git a/crates/kit/src/to_disk.rs b/crates/kit/src/to_disk.rs index 0d11c2c..434aff9 100644 --- a/crates/kit/src/to_disk.rs +++ b/crates/kit/src/to_disk.rs @@ -310,7 +310,6 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { opts.target_disk.as_std_path(), &image_digest, &opts.install, - &opts.additional.common.kernel_args, )? { Ok(()) => { println!( @@ -421,6 +420,7 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { opts.target_disk, opts.additional.format.as_str() )], // Attach target disk + kernel_args: Default::default(), }; // Phase 5: SSH-based VM configuration and execution @@ -475,7 +475,6 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { &opts.source_image, &opts.target_disk, &opts.install, - &opts.additional.common.kernel_args, &opts.additional.format, ); if let Err(e) = write_result { @@ -496,7 +495,6 @@ fn write_disk_metadata( source_image: &str, target_disk: &Utf8PathBuf, install_options: &InstallOptions, - kernel_args: &[String], format: &Format, ) -> Result<()> { // Note: xattrs work on regular files including raw and qcow2 images @@ -507,7 +505,7 @@ fn write_disk_metadata( let digest = inspect.digest.to_string(); // Prepare metadata using the new helper method - let metadata = DiskImageMetadata::from(install_options, &digest, kernel_args); + let metadata = DiskImageMetadata::from(install_options, &digest); // Write metadata using rustix fsetxattr let file = std::fs::OpenOptions::new() diff --git a/docs/src/man/bcvk-ephemeral-run-ssh.md b/docs/src/man/bcvk-ephemeral-run-ssh.md index 5bdabc4..a234706 100644 --- a/docs/src/man/bcvk-ephemeral-run-ssh.md +++ b/docs/src/man/bcvk-ephemeral-run-ssh.md @@ -33,10 +33,6 @@ Run ephemeral VM and SSH into it Number of vCPUs -**--karg**=*KERNEL_ARGS* - - Additional kernel command line arguments - **--console** Enable console output to terminal for debugging @@ -109,6 +105,10 @@ Run ephemeral VM and SSH into it Mount disk file as virtio-blk device at /dev/disk/by-id/virtio- +**--karg**=*KERNEL_ARGS* + + Additional kernel command line arguments + # EXAMPLES diff --git a/docs/src/man/bcvk-ephemeral-run.md b/docs/src/man/bcvk-ephemeral-run.md index 9fc8861..a92b033 100644 --- a/docs/src/man/bcvk-ephemeral-run.md +++ b/docs/src/man/bcvk-ephemeral-run.md @@ -59,10 +59,6 @@ This design allows bcvk to provide VM-like isolation and boot behavior while lev Number of vCPUs -**--karg**=*KERNEL_ARGS* - - Additional kernel command line arguments - **--console** Enable console output to terminal for debugging @@ -135,6 +131,10 @@ This design allows bcvk to provide VM-like isolation and boot behavior while lev Mount disk file as virtio-blk device at /dev/disk/by-id/virtio- +**--karg**=*KERNEL_ARGS* + + Additional kernel command line arguments + # EXAMPLES diff --git a/docs/src/man/bcvk-libvirt-list-volumes.md b/docs/src/man/bcvk-libvirt-list-volumes.md index 4cdcee3..097593a 100644 --- a/docs/src/man/bcvk-libvirt-list-volumes.md +++ b/docs/src/man/bcvk-libvirt-list-volumes.md @@ -21,7 +21,7 @@ List available bootc volumes with metadata **--json** - Output as structured JSON instead of table format + Output format (human-readable or JSON) **--detailed** diff --git a/docs/src/man/bcvk-libvirt-run.md b/docs/src/man/bcvk-libvirt-run.md index 798cbdc..fb495a4 100644 --- a/docs/src/man/bcvk-libvirt-run.md +++ b/docs/src/man/bcvk-libvirt-run.md @@ -53,6 +53,10 @@ Run a bootable container as a persistent VM Path to host container storage (auto-detected if not specified) +**--karg**=*KARG* + + Set a kernel argument + **--composefs-native** Default to composefs-native storage diff --git a/docs/src/man/bcvk-libvirt-upload.md b/docs/src/man/bcvk-libvirt-upload.md index cef67ba..610a1e9 100644 --- a/docs/src/man/bcvk-libvirt-upload.md +++ b/docs/src/man/bcvk-libvirt-upload.md @@ -45,6 +45,10 @@ Upload bootc disk images to libvirt with metadata annotations Path to host container storage (auto-detected if not specified) +**--karg**=*KARG* + + Set a kernel argument + **--composefs-native** Default to composefs-native storage @@ -59,10 +63,6 @@ Upload bootc disk images to libvirt with metadata annotations Number of vCPUs for installation VM -**--karg**=*KARG* - - Additional kernel arguments for installation - # EXAMPLES diff --git a/docs/src/man/bcvk-to-disk.md b/docs/src/man/bcvk-to-disk.md index bf90a43..c6209f5 100644 --- a/docs/src/man/bcvk-to-disk.md +++ b/docs/src/man/bcvk-to-disk.md @@ -47,6 +47,10 @@ The installation process: Path to host container storage (auto-detected if not specified) +**--karg**=*KARG* + + Set a kernel argument + **--composefs-native** Default to composefs-native storage @@ -75,10 +79,6 @@ The installation process: Number of vCPUs -**--karg**=*KERNEL_ARGS* - - Additional kernel command line arguments - **--console** Enable console output to terminal for debugging