Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions crates/integration-tests/src/tests/run_ephemeral.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,7 +406,7 @@ integration_test!(test_run_ephemeral_virtiofs_mmap);
/// Test that ephemeral VMs have the expected mount layout:
/// - / is read-only virtiofs
/// - /etc is overlayfs with tmpfs upper (writable)
/// - /var is tmpfs (not overlayfs, so podman can use overlayfs inside)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an explicit comment here you're reverting. We can't break podman/docker.

We need an alternative path. One is to investigate here is to mount /var as a writable virtiofs pointing to the host container's overlayfs store (or perhaps even better, make it a dedicated volume).

Yeah, thinking about this more: if we do an anonymous volume for /var by default, but we also made it easy to use e.g. --volume=<name>:/var then one could have ephemeral-with-persistent-data which would be a powerful pattern.

If we run into issues with virtiofsd mappings here, what would likely work reliably albeit less efficiently is to attach a temporary file in that volume as a block device to qemu, and then initialize it as /var in the guest (xfs/btrfs) via a systemd mount unit.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done a number of builds with this change and I have not had issues with Podman and the overlay. I did see the comment, I have not been able to come up with a scenario where it's applicable in bcvk itself. I feel like I'm missing something. What kind of scenario would cause an issue with /var not being a tmpfs?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have done a number of builds with this change and I have not had issues with Podman and the overlay.

This is about using podman run inside bcvk ephemeral itself - as distinct from using podman run in a bcvk libvirt run install (a totally different thing).

It turned out that bcvk ephemeral is a really useful general purpose tool, and I'd like to keep it that way.

/// - /var is overlayfs with tmpfs upper (writable, instant setup for large /var trees)
fn test_run_ephemeral_mount_layout() -> TestResult {
let sh = shell()?;
let bck = get_bck_command()?;
Expand Down Expand Up @@ -446,16 +446,17 @@ fn test_run_ephemeral_mount_layout() -> TestResult {
etc_fstype
);

// Check /var mount - should be tmpfs, NOT overlay
// Check /var mount - should be overlay (using overlayfs for instant setup,
// avoiding slow/failing cp -a of large /var trees on virtiofs)
let var_fstype = cmd!(
sh,
"{bck} ephemeral run --rm --label {label} --execute 'findmnt -n -o FSTYPE /var' {image}"
)
.read()?;
assert_eq!(
var_fstype.trim(),
"tmpfs",
"/var should be tmpfs (not overlay), got: {}",
"overlay",
"/var should be overlay, got: {}",
var_fstype
);

Expand Down
2 changes: 1 addition & 1 deletion crates/kit/src/cpio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ pub fn create_initramfs_units_cpio() -> io::Result<Vec<u8>> {
),
File(
"usr/lib/systemd/system/initrd-fs.target.d/bcvk-var-ephemeral.conf",
b"[Unit]\nWants=bcvk-var-ephemeral.service\n",
b"[Unit]\nRequires=bcvk-var-ephemeral.service\n",
),
File(
"usr/lib/systemd/system/initrd-fs.target.d/bcvk-copy-units.conf",
Expand Down
4 changes: 4 additions & 0 deletions crates/kit/src/libvirt/base_disks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ pub fn find_or_create_base_disk(
image_digest: &str,
install_options: &InstallOptions,
connect_uri: Option<&str>,
virtiofsd_binary: Option<String>,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer Option<&str>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 7b34872

) -> Result<Utf8PathBuf> {
let metadata = DiskImageMetadata::from(install_options, image_digest, source_image);
let cache_hash = metadata.compute_cache_hash();
Expand Down Expand Up @@ -67,6 +68,7 @@ pub fn find_or_create_base_disk(
image_digest,
install_options,
connect_uri,
virtiofsd_binary,
)?;

Ok(base_disk_path)
Expand All @@ -79,6 +81,7 @@ fn create_base_disk(
image_digest: &str,
install_options: &InstallOptions,
connect_uri: Option<&str>,
virtiofsd_binary: Option<String>,
) -> Result<()> {
use crate::run_ephemeral::CommonVmOpts;
use crate::to_disk::{Format, ToDiskAdditionalOpts, ToDiskOpts};
Expand Down Expand Up @@ -116,6 +119,7 @@ fn create_base_disk(
memory: crate::common_opts::MemoryOpts {
memory: super::LIBVIRT_DEFAULT_MEMORY.to_string(),
},
virtiofsd_binary,
..Default::default()
},
..Default::default()
Expand Down
1 change: 1 addition & 0 deletions crates/kit/src/libvirt/base_disks_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ pub fn run_create(
&image_digest,
&opts.install_options,
connect_uri,
None,
)?;
println!("Created base disk: {path}");

Expand Down
5 changes: 5 additions & 0 deletions crates/kit/src/libvirt/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,10 @@ pub struct LibvirtRunOpts {
#[clap(long = "ignition")]
pub ignition_config: Option<Utf8PathBuf>,

/// Path to virtiofsd binary (overrides auto-detection for disk creation)
#[clap(long = "virtiofsd", env = "VIRTIOFSD_BIN")]
pub virtiofsd_binary: Option<String>,

/// Log virtio console (OS/journald on hvc0) to this file (created if absent)
#[clap(long = "console-log")]
pub console_log: Option<Utf8PathBuf>,
Expand Down Expand Up @@ -498,6 +502,7 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, mut opts: LibvirtRunOpt
&image_digest,
&opts.install,
connect_uri,
opts.virtiofsd_binary.take(),
)
.with_context(|| "Failed to find or create base disk")?;

Expand Down
10 changes: 7 additions & 3 deletions crates/kit/src/units/bcvk-var-ephemeral.service
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ Requires=sysroot.mount
Type=oneshot
RemainAfterExit=yes
TimeoutStartSec=60
ExecStart=/usr/bin/mkdir -p /run/var-ephemeral
ExecStart=/usr/bin/cp -a /sysroot/var/. /run/var-ephemeral/
ExecStart=/usr/bin/mount --bind /run/var-ephemeral /sysroot/var
# Use overlayfs like /etc to avoid copying large files
# from virtiofs to tmpfs. The cp -a approach hangs and systemd continues to progress
# stages which causes issues if /var never becomes read/write.
# Bind-mount /sysroot/var to a separate location first (same technique as /etc overlay).
ExecStart=/usr/bin/mkdir -p /run/var-lower /run/var-upper /run/var-work
ExecStart=/usr/bin/mount --bind /sysroot/var /run/var-lower
ExecStart=/usr/bin/mount -t overlay overlay -o lowerdir=/run/var-lower,upperdir=/run/var-upper,workdir=/run/var-work,index=off,metacopy=off /sysroot/var
Loading