Skip to content

Commit f1dad30

Browse files
mkutsevolmeta-codesync[bot]
authored andcommitted
[kernel][zcrx] vmtest: add extra_mounts support for sharing host directories into VM
Summary: Add first-class support for mounting arbitrary host directories into vmtest VMs. This enables Use Case #3 from the Zero Copy Receive testing plan (https://fburl.com/gdoc/msf9i6y0): local rapid iteration on a devserver. At the very least we'd need the kernel source mounted in, but having the ability to specify any directory makes it fit any use case. Before this change, only a single directory was shared - fbcode. Which is not enough for the kernel use case. Now, adding `extra_mounts = [("/path/to/dir", True)]` to a vm.host() target does everything automatically: - **Container level**: bind-mounts the path into the isolation container using the existing antlir2_isolate inputs/outputs mechanism (read-only or read-write based on the boolean flag). - **VM level**: feeds the path into the existing virtiofs share machinery, which starts virtiofsd, generates QEMU device args, and creates systemd .mount unit files so the directory appears at the same path inside the VM on boot. For the ZCRX test, local development is now: buck run -c zcrx.extra_share=/path/to/linux \ -c zcrx.prebuilt_vmlinux=/path/to/bzImage \ ':fbnic-zcrx-vm[container]' Design decisions: - Each mount carries a read_only flag (Buck attr is a tuple of (path, bool)), routed to inputs() for RO or outputs() for RW at the container level, and to ShareOpts.read_only at the VM level. - No canonicalize() on mount paths. Symlink resolution is left to the nspawn --bind-ro mechanism, avoiding path mismatches when /home is a symlink to /data/users (common on devservers). Test Plan: 1. Built machine_json sub-target with extra_share config, verified JSON serialization: ``` buck2 build fbcode//kernel/vmtest/iouring_zcrx:fbnic-zcrx-vm[machine_json] -c zcrx.extra_share=/tmp/test_extra_mount ``` Produces correct `{"path": ["/tmp/test_extra_mount"], "read_only": true}`. 2. Ran the VM with extra_share and verified RO enforcement: ``` buck2 run fbcode//kernel/vmtest/iouring_zcrx:fbnic-zcrx-vm \ -c zcrx.extra_share=/tmp/test_extra_mount \ -c zcrx.prebuilt_vmlinux=.../bzImage \ -- --timeout-secs 30 -- sh -c \ 'if touch /tmp/test_extra_mount/write_test 2>/dev/null; then echo FAIL; else echo PASS; fi' ``` Output: `PASS: write denied on RO mount` 3. Verified symlinked paths work correctly (no canonicalize mismatch). 4. Confirmed nspawn backend routes inputs->--bind-ro and outputs->--bind (isolate_nspawn/src/lib.rs).; 5. Confirmed working RW in `buck run -c zcrx.extra_share=/home/mkutsevol/linux ':iouring-zcrx-test[shell]'` Reviewed By: wujj123456 Differential Revision: D95386284 fbshipit-source-id: 6031019e294a8f4150fab2a3496250c8fd00c027
1 parent 9ba751d commit f1dad30

File tree

5 files changed

+115
-33
lines changed

5 files changed

+115
-33
lines changed

antlir/antlir2/antlir2_vm/bzl/defs.bzl

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ def _machine_json(ctx: AnalysisContext) -> (Artifact, typing.Any):
5151
"cpus": ctx.attrs.cpus,
5252
"disks": [d[DiskInfo] for d in disks],
5353
"extra_qemu_args": ctx.attrs.extra_qemu_args,
54+
"input_dirs": ctx.attrs.input_dirs,
5455
"machine_type": ctx.attrs.machine_type,
5556
"max_combined_channels": ctx.attrs.max_combined_channels,
5657
"mem_mib": ctx.attrs.mem_mib,
@@ -61,6 +62,7 @@ def _machine_json(ctx: AnalysisContext) -> (Artifact, typing.Any):
6162
"kernel": ctx.attrs.kernel,
6263
} if ctx.attrs.initrd else None,
6364
"num_nics": ctx.attrs.num_nics,
65+
"output_dirs": ctx.attrs.output_dirs,
6466
"qemu_binary": cmd_args(ctx.attrs.qemu_binary, delimiter = ""),
6567
"serial_index": ctx.attrs.serial_index,
6668
"sidecar_services": ctx.attrs.sidecar_services,
@@ -154,6 +156,12 @@ _vm_host = rule(
154156
default = None,
155157
doc = "initrd to boot from when not booting from disk",
156158
),
159+
"input_dirs": attrs.list(
160+
attrs.arg(),
161+
default = [],
162+
doc = "additional read-only host directories to bind-mount into the VM container. " +
163+
"Useful when extra_qemu_args references paths outside the repo.",
164+
),
157165
"kernel": attrs.option(
158166
attrs.source(),
159167
default = None,
@@ -168,6 +176,12 @@ _vm_host = rule(
168176
default = True,
169177
doc = "Mount runtime platform (aka /usr/local/fbcode) from the host",
170178
),
179+
"output_dirs": attrs.list(
180+
attrs.arg(),
181+
default = [],
182+
doc = "additional read-write host directories to bind-mount into the VM container. " +
183+
"Useful when extra_qemu_args references paths outside the repo.",
184+
),
171185
# Must be an arg() because it needs to accept locations.
172186
"qemu_binary": attrs.arg(
173187
default = arch_select(

antlir/antlir2/antlir2_vm/src/isolation.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,10 +87,12 @@ impl Platform {
8787
/// * `image` - container image that would be used to run the VM
8888
/// * `envs` - env vars to set inside container.
8989
/// * `outputs` - Additional writable directories
90+
/// * `inputs` - Additional read-only host directories to bind-mount
9091
pub(crate) fn isolated(
9192
image: &PathBuf,
9293
envs: Vec<KvPair>,
9394
outputs: HashSet<PathBuf>,
95+
inputs: HashSet<PathBuf>,
9496
) -> Result<IsolatedContext> {
9597
let repo = Platform::repo_root()?;
9698
let mut builder = IsolationContext::builder(image);
@@ -103,6 +105,7 @@ pub(crate) fn isolated(
103105
.outputs(outputs);
104106
#[cfg(facebook)]
105107
builder.inputs(Path::new("/var/facebook/x509_identities/server.pem"));
108+
builder.inputs(inputs);
106109
builder.setenv(
107110
envs.into_iter()
108111
.map(|p| (p.key, p.value))

antlir/antlir2/antlir2_vm/src/main.rs

Lines changed: 49 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -96,9 +96,6 @@ struct IsolateCmdArgs {
9696
/// Pass through these environment variables into the container and VM, if they exist.
9797
#[arg(long)]
9898
passenv: Vec<String>,
99-
/// Extra RW bind-mount into the VM for debugging purpose
100-
#[arg(long)]
101-
scratch_dir: Option<PathBuf>,
10299
/// Whether or not to dump the VM's eth0 traffic to a file. When running the test command, this will set eth0_output_file to a file that will be uploaded to tpx.
103100
#[arg(long, default_value_t = false)]
104101
dump_eth0_traffic: bool,
@@ -190,9 +187,6 @@ fn respawn(args: &IsolateCmdArgs) -> Result<()> {
190187
let mut vm_args = args.run_cmd_args.vm_args.clone();
191188
let envs = env_names_to_kvpairs(args.passenv.clone());
192189
vm_args.command_envs = envs.clone();
193-
if let Some(scratch_dir) = args.scratch_dir.as_ref() {
194-
vm_args.output_dirs.push(scratch_dir.clone());
195-
}
196190

197191
// Let's always capture console output unless it's console mode
198192
let _console_dir;
@@ -205,15 +199,22 @@ fn respawn(args: &IsolateCmdArgs) -> Result<()> {
205199
antlir2_rootless::unshare_new_userns()?;
206200
antlir2_isolate::unshare_and_privatize_mount_ns().context("while isolating mount ns")?;
207201

208-
let isolated = isolated(
209-
&args.image,
210-
envs,
211-
vm_args
212-
.get_container_output_dirs()
213-
.into_iter()
214-
.chain(writable_devices())
215-
.collect(),
216-
)?;
202+
let machine_spec = &args.run_cmd_args.machine_spec;
203+
let inputs: HashSet<PathBuf> = machine_spec
204+
.input_dirs
205+
.iter()
206+
.map(|d| PathBuf::from(d.concat()))
207+
.collect();
208+
let mut outputs: HashSet<PathBuf> = vm_args
209+
.get_container_output_dirs()
210+
.into_iter()
211+
.chain(writable_devices())
212+
.collect();
213+
for dir in &machine_spec.output_dirs {
214+
outputs.insert(PathBuf::from(dir.concat()));
215+
}
216+
217+
let isolated = isolated(&args.image, envs, outputs, inputs)?;
217218
let exe = env::current_exe().context("while getting argv[0]")?;
218219
let mut command = isolated.command(exe)?;
219220
command
@@ -357,11 +358,21 @@ fn writable_devices() -> HashSet<PathBuf> {
357358
/// to discover the tests to run. This command is not our intended test to
358359
/// execute, so it's unnecessarily wasteful to execute it inside the VM. We
359360
/// directly run it inside the container without booting VM.
360-
fn list_test_command(args: &IsolateCmdArgs, validated_args: &ValidatedVMArgs) -> Result<Command> {
361+
fn list_test_command(
362+
args: &IsolateCmdArgs,
363+
validated_args: &ValidatedVMArgs,
364+
inputs: HashSet<PathBuf>,
365+
outputs: &[Vec<String>],
366+
) -> Result<Command> {
367+
let mut collected_outputs = writable_outputs(validated_args);
368+
for dir in outputs {
369+
collected_outputs.insert(PathBuf::from(dir.concat()));
370+
}
361371
let isolated = isolated(
362372
&args.image,
363373
validated_args.inner.command_envs.clone(),
364-
writable_outputs(validated_args),
374+
collected_outputs,
375+
inputs,
365376
)?;
366377
let mut inner_cmd = validated_args
367378
.inner
@@ -376,11 +387,21 @@ fn list_test_command(args: &IsolateCmdArgs, validated_args: &ValidatedVMArgs) ->
376387
}
377388

378389
/// For actual test command, we spawn the VM and run it.
379-
fn vm_test_command(args: &IsolateCmdArgs, validated_args: &ValidatedVMArgs) -> Result<Command> {
390+
fn vm_test_command(
391+
args: &IsolateCmdArgs,
392+
validated_args: &ValidatedVMArgs,
393+
inputs: HashSet<PathBuf>,
394+
outputs: &[Vec<String>],
395+
) -> Result<Command> {
396+
let mut collected_outputs = writable_outputs(validated_args);
397+
for dir in outputs {
398+
collected_outputs.insert(PathBuf::from(dir.concat()));
399+
}
380400
let isolated = isolated(
381401
&args.image,
382402
validated_args.inner.command_envs.clone(),
383-
writable_outputs(validated_args),
403+
collected_outputs,
404+
inputs,
384405
)?;
385406
let exe = env::current_exe().context("while getting argv[0]")?;
386407
let mut command = isolated.command(exe)?;
@@ -405,6 +426,13 @@ fn test(args: &IsolateCmdArgs) -> Result<()> {
405426
// The inner antlir2_vm process will need fbcode runtime to start.
406427
// It may then decide whether to use host's platform for the actual test.
407428
Platform::set(&MountPlatformDecision(true))?;
429+
let machine_spec = &args.run_cmd_args.machine_spec;
430+
let inputs: HashSet<PathBuf> = machine_spec
431+
.input_dirs
432+
.iter()
433+
.map(|d| PathBuf::from(d.concat()))
434+
.collect();
435+
let outputs = &machine_spec.output_dirs;
408436

409437
let validated_args = get_test_vm_args(
410438
&args.run_cmd_args.vm_args,
@@ -415,9 +443,9 @@ fn test(args: &IsolateCmdArgs) -> Result<()> {
415443
antlir2_isolate::unshare_and_privatize_mount_ns().context("while isolating mount ns")?;
416444

417445
let mut command = if validated_args.is_list {
418-
list_test_command(args, &validated_args)
446+
list_test_command(args, &validated_args, inputs, outputs)
419447
} else {
420-
vm_test_command(args, &validated_args)
448+
vm_test_command(args, &validated_args, inputs, outputs)
421449
}?;
422450
let status = log_command(&mut command).status()?;
423451
if !status.success() {

antlir/antlir2/antlir2_vm/src/types.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,16 @@ pub(crate) struct MachineOpts {
297297
/// Each arg is serialized as a single-element list by Buck2's attrs.arg().
298298
#[serde(default)]
299299
pub(crate) extra_qemu_args: Vec<Vec<String>>,
300+
/// Additional read-only host directories to bind-mount into the VM
301+
/// container. Each path is serialized as a single-element list by
302+
/// Buck2's attrs.arg().
303+
#[serde(default)]
304+
pub(crate) input_dirs: Vec<Vec<String>>,
305+
/// Additional read-write host directories to bind-mount into the VM
306+
/// container. Each path is serialized as a single-element list by
307+
/// Buck2's attrs.arg().
308+
#[serde(default)]
309+
pub(crate) output_dirs: Vec<Vec<String>>,
300310
}
301311

302312
#[cfg(test)]

antlir/antlir2/antlir2_vm/src/vm.rs

Lines changed: 39 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,17 @@ impl<S: Share> VM<S> {
130130
let state_dir = Self::create_state_dir()?;
131131
let pci_bridges = PCIBridges::new(machine.disks.len())?;
132132
let disks = QCow2Disks::new(&machine.disks, &pci_bridges, &state_dir)?;
133+
let mut all_output_dirs = args.get_vm_output_dirs();
134+
for dir in &machine.output_dirs {
135+
all_output_dirs.insert(PathBuf::from(dir.concat()));
136+
}
137+
let all_input_dirs: HashSet<PathBuf> = machine
138+
.input_dirs
139+
.iter()
140+
.map(|d| PathBuf::from(d.concat()))
141+
.collect();
133142
let shares = Self::create_shares(
134-
Self::get_all_shares_opts(&args.get_vm_output_dirs()),
143+
Self::get_all_shares_opts(&all_input_dirs, &all_output_dirs),
135144
&state_dir,
136145
machine.mem_mib,
137146
)?;
@@ -197,25 +206,28 @@ impl<S: Share> VM<S> {
197206

198207
/// All platform paths needs to be mounted inside the VM as read-only shares.
199208
/// Collect them into ShareOpts along with others.
200-
fn get_all_shares_opts(output_dirs: &HashSet<PathBuf>) -> Vec<ShareOpts> {
201-
let mut shares: Vec<_> = Platform::get()
209+
fn get_all_shares_opts(
210+
inputs: &HashSet<PathBuf>,
211+
outputs: &HashSet<PathBuf>,
212+
) -> Vec<ShareOpts> {
213+
Platform::get()
202214
.iter()
203215
.map(|path| ShareOpts {
204216
path: path.to_path_buf(),
205217
read_only: true,
206218
mount_tag: None,
207219
})
208-
.collect();
209-
let mut outputs: Vec<_> = output_dirs
210-
.iter()
211-
.map(|p| ShareOpts {
220+
.chain(inputs.iter().map(|p| ShareOpts {
221+
path: p.to_path_buf(),
222+
read_only: true,
223+
mount_tag: None,
224+
}))
225+
.chain(outputs.iter().map(|p| ShareOpts {
212226
path: p.to_path_buf(),
213227
read_only: false,
214228
mount_tag: None,
215-
})
216-
.collect();
217-
shares.append(&mut outputs);
218-
shares
229+
}))
230+
.collect()
219231
}
220232

221233
/// Create all shares, start virtiofsd daemon and generate necessary unit files
@@ -1151,8 +1163,23 @@ mod test {
11511163
read_only: false,
11521164
mount_tag: None,
11531165
};
1154-
let all_opts = VM::<VirtiofsShare>::get_all_shares_opts(&outputs);
1166+
let all_opts = VM::<VirtiofsShare>::get_all_shares_opts(&HashSet::new(), &outputs);
11551167
assert!(all_opts.contains(&opt));
1168+
1169+
let inputs = HashSet::from([PathBuf::from("/extra/input")]);
1170+
let mut extended_outputs = outputs.clone();
1171+
extended_outputs.insert(PathBuf::from("/extra/output"));
1172+
let all_opts = VM::<VirtiofsShare>::get_all_shares_opts(&inputs, &extended_outputs);
1173+
assert!(all_opts.contains(&ShareOpts {
1174+
path: PathBuf::from("/extra/input"),
1175+
read_only: true,
1176+
mount_tag: None,
1177+
}));
1178+
assert!(all_opts.contains(&ShareOpts {
1179+
path: PathBuf::from("/extra/output"),
1180+
read_only: false,
1181+
mount_tag: None,
1182+
}));
11561183
}
11571184

11581185
#[test]

0 commit comments

Comments
 (0)