Skip to content

Commit 1bb845b

Browse files
committed
virtiofs: Fix writable virtiofs mounts
Regression from previous commit; we unconditionally added `--readonly` if virtiofsd supported it (which it doesn't yet in the Ubuntu 24.04 but does in Fedora 42+). We want to support writable mounts. It turns out the integration tests didn't catch this because they only looked at the permission bits. Signed-off-by: Colin Walters <walters@verbum.org>
1 parent de51faf commit 1bb845b

3 files changed

Lines changed: 39 additions & 37 deletions

File tree

crates/integration-tests/src/tests/mount_feature.rs

Lines changed: 32 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -20,57 +20,48 @@ use tempfile::TempDir;
2020

2121
use crate::{get_test_image, run_bcvk, INTEGRATION_TEST_LABEL};
2222

23-
/// Create a systemd unit that verifies a mount exists and contains expected content
23+
/// Create a systemd unit that verifies a mount exists and tests writability
2424
fn create_mount_verify_unit(
2525
unit_dir: &Utf8Path,
2626
mount_name: &str,
2727
expected_file: &str,
28-
expected_content: &str,
28+
expected_content: Option<&str>,
29+
readonly: bool,
2930
) -> std::io::Result<()> {
30-
let unit_content = format!(
31-
r#"[Unit]
32-
Description=Verify mount {mount_name} and poweroff
33-
RequiresMountsFor=/run/virtiofs-mnt-{mount_name}
31+
let (description, content_check, write_check, unit_prefix) = if readonly {
32+
(
33+
format!("Verify read-only mount {mount_name} and poweroff"),
34+
format!("ExecStart=test -f /run/virtiofs-mnt-{mount_name}/{expected_file}"),
35+
format!("ExecStart=/bin/sh -c '! echo test-write > /run/virtiofs-mnt-{mount_name}/write-test.txt 2>/dev/null'"),
36+
"verify-ro-mount",
37+
)
38+
} else {
39+
let content = expected_content.expect("expected_content required for writable mounts");
40+
(
41+
format!("Verify mount {mount_name} and poweroff"),
42+
format!("ExecStart=grep -qF \"{content}\" /run/virtiofs-mnt-{mount_name}/{expected_file}"),
43+
format!("ExecStart=/bin/sh -c 'echo test-write > /run/virtiofs-mnt-{mount_name}/write-test.txt'"),
44+
"verify-mount",
45+
)
46+
};
3447

35-
[Service]
36-
Type=oneshot
37-
ExecStart=grep -qF "{expected_content}" /run/virtiofs-mnt-{mount_name}/{expected_file}
38-
ExecStart=test -w /run/virtiofs-mnt-{mount_name}/{expected_file}
39-
ExecStart=echo ok mount verify {mount_name}
40-
ExecStart=systemctl poweroff
41-
StandardOutput=journal+console
42-
StandardError=journal+console
43-
"#
44-
);
45-
46-
let unit_path = unit_dir.join(format!("verify-mount-{}.service", mount_name));
47-
fs::write(&unit_path, unit_content)?;
48-
Ok(())
49-
}
50-
51-
/// Create a systemd unit that tries to write to a mount to verify read-only status
52-
fn create_ro_mount_verify_unit(
53-
unit_dir: &Utf8Path,
54-
mount_name: &str,
55-
expected_file: &str,
56-
) -> std::io::Result<()> {
5748
let unit_content = format!(
5849
r#"[Unit]
59-
Description=Verify read-only mount {mount_name} and poweroff
50+
Description={description}
6051
RequiresMountsFor=/run/virtiofs-mnt-{mount_name}
6152
6253
[Service]
6354
Type=oneshot
64-
ExecStart=test -f /run/virtiofs-mnt-{mount_name}/{expected_file}
65-
ExecStart=test '!' -w /run/virtiofs-mnt-{mount_name}/{expected_file}
55+
{content_check}
56+
{write_check}
6657
ExecStart=echo ok mount verify {mount_name}
6758
ExecStart=systemctl poweroff
6859
StandardOutput=journal+console
6960
StandardError=journal+console
7061
"#
7162
);
7263

73-
let unit_path = unit_dir.join(format!("verify-ro-mount-{}.service", mount_name));
64+
let unit_path = unit_dir.join(format!("{unit_prefix}-{mount_name}.service"));
7465
fs::write(&unit_path, unit_content)?;
7566
Ok(())
7667
}
@@ -90,8 +81,14 @@ pub fn test_mount_feature_bind() {
9081
fs::create_dir(&system_dir).expect("Failed to create system directory");
9182

9283
// Create verification unit
93-
create_mount_verify_unit(&system_dir, "testmount", "test.txt", test_content)
94-
.expect("Failed to create verify unit");
84+
create_mount_verify_unit(
85+
&system_dir,
86+
"testmount",
87+
"test.txt",
88+
Some(test_content),
89+
false,
90+
)
91+
.expect("Failed to create verify unit");
9592

9693
println!("Testing bind mount with temp directory: {}", temp_dir_path);
9794

@@ -135,7 +132,7 @@ pub fn test_mount_feature_ro_bind() {
135132
fs::create_dir(&system_dir).expect("Failed to create system directory");
136133

137134
// Create verification unit for read-only mount
138-
create_ro_mount_verify_unit(&system_dir, "romount", "readonly.txt")
135+
create_mount_verify_unit(&system_dir, "romount", "readonly.txt", None, true)
139136
.expect("Failed to create verify unit");
140137

141138
println!(

crates/kit/src/qemu.rs

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,8 @@ pub struct VirtiofsConfig {
898898
/// Host directory to share
899899
pub shared_dir: Utf8PathBuf,
900900
pub debug: bool,
901+
/// Mount as read-only
902+
pub readonly: bool,
901903
}
902904

903905
impl Default for VirtiofsConfig {
@@ -906,6 +908,8 @@ impl Default for VirtiofsConfig {
906908
socket_path: "/run/inner-shared/virtiofs.sock".into(),
907909
shared_dir: "/run/source-image".into(),
908910
debug: false,
911+
// We don't need to write to this, there's a transient overlay
912+
readonly: true,
909913
}
910914
}
911915
}
@@ -977,8 +981,8 @@ pub async fn spawn_virtiofsd_async(config: &VirtiofsConfig) -> Result<tokio::pro
977981
"--sandbox=none",
978982
]);
979983

980-
// Only add --readonly if supported
981-
if supports_readonly {
984+
// Only add --readonly if requested and supported
985+
if config.readonly && supports_readonly {
982986
cmd.arg("--readonly");
983987
}
984988

crates/kit/src/run_ephemeral.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,7 @@ pub(crate) async fn run_impl(opts: RunEphemeralOpts) -> Result<()> {
816816
socket_path: socket_path.clone().into(),
817817
shared_dir: source_path,
818818
debug: false,
819+
readonly: is_readonly,
819820
};
820821
additional_mounts.push((virtiofsd_config, tag.clone()));
821822

0 commit comments

Comments
 (0)