Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve mounts host_path relatively to vmtest.toml #94

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

akerouanton
Copy link
Contributor

When a non-default rootfs is used, and vmtest is run through go test -exec, the binary executed within the VM can't be found, because that rootfs doesn't contain the test binary compiled on the fly. So a custom mount has to be specified.

But, before that commit, if a relative host_path was used, it'd be passed as is to qemu, and thus would be resolved as relative to the cwd where qemu was running.

In the case of Go, if a directory was passed to go test (eg. ./integration/...), and GOTMPDIR=. was set, Go would compile the test binary within its original cwd (eg. project root dir), and then launch vmtest in the 'directory under test' (eg. ./integration), so the relative mount would resolve to the directory where vmtest was started (eg. ./integration) -- a directory that doesn't contain the test binary.

To fix this mismatch, resolve host_path relative to the vmtest.toml file.

FWIW this doesn't fully solve the original issue, unless the VM path put in the config file is hardcoded to the cwd of go test -exec. To overcome this, one can prepend the inline command with a call to mkdir + ln:

GOTMPDIR=. go test -exec "vmtest -c $(pwd)/vmtest.toml -- mkdir $(realpath ..); ln -s /skbdump/ $(pwd); " ./integration/...

@akerouanton akerouanton force-pushed the relative-mounts branch 2 times, most recently from 3e7a558 to 8466295 Compare November 18, 2024 09:34
@akerouanton akerouanton marked this pull request as ready for review November 18, 2024 10:24
Copy link
Owner

@danobi danobi left a comment

Choose a reason for hiding this comment

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

keeping path resolution relative to vmtest.toml sounds good to me as it's consistent with many of the other configs.

src/vmtest.rs Outdated
.map(|(k, v)| {
let mut m = v.clone();
m.host_path = self.resolve_path(v.host_path.as_path());
(k.clone(), m)
Copy link
Owner

Choose a reason for hiding this comment

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

should be possible to modify in-place? Given we already have a mut target cloned above. don't worry about it if it's complicated - just a nit

@danobi
Copy link
Owner

danobi commented Nov 27, 2024

needs a rustfmt fix

When a non-default `rootfs` is used, and vmtest is run through
`go test -exec`, the binary executed within the VM can't be found,
because that rootfs doesn't contain the test binary compiled on the fly.
So a custom mount has to be specified.

But, before that commit, if a relative `host_path` was used, it'd be
passed as is to qemu, and thus would be resolved as relative to the cwd
where qemu was running.

In the case of Go, if a directory was passed to `go test` (eg.
`./integration/...`), and `GOTMPDIR=.` was set, Go would compile the
test binary within its original cwd (eg. project root dir), and then
launch vmtest in the 'directory under test' (eg. `./integration`), so
the relative mount would resolve to the directory where vmtest was
started (eg. `./integration`) -- a directory that doesn't contain the
test binary.

To fix this mismatch, resolve `host_path` relative to the `vmtest.toml`
file.

FWIW this doesn't fully solve the original issue, unless the VM path
put in the config file is hardcoded to the cwd of `go test -exec`. To
overcome this, one can prepend the inline command with a call to
`mkdir` + `ln`:

```sh
GOTMPDIR=. go test -exec "vmtest -c $(pwd)/vmtest.toml -- mkdir $(realpath ..); ln -s /skbdump/ $(pwd); " ./integration/...
```

Signed-off-by: Albin Kerouanton <albinker@gmail.com>
@danobi danobi merged commit edd5141 into danobi:master Dec 3, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants