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

Few fixes that allow auraed to run as true PID 1 #447

Merged
merged 4 commits into from
Mar 19, 2023

Conversation

JeroenSoeters
Copy link
Contributor

This PR fixes a few things that were preventing auraed to run as true PID 1. This is required for the upcoming VM work.

@@ -87,9 +87,6 @@ impl SystemRuntime for Pid1SystemRuntime {
// TODO We likely to do not need to mount these filesystems.
// TODO Do we want to have a way to "try" these mounts and continue without erroring?

MountSpec { source: None, target: "/dev", fstype: Some("devtmpfs") }
.mount()?;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was throwing a "Resource busy" so I presumed this wasn't actually needed?

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it just a feature of cloud-hypervisor that /dev is already mounted, but we would need to mount it in a non-vm context (i.e., true-true PID 1 for lack of a better identifier)?

Maybe the try and continue approach is right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looking at cloud-hypervisor's config that you linked to in #439 I see the following in the file:

CONFIG_DEVTMPFS=y
CONFIG_DEVTMPFS_MOUNT=y

Would this config be causing the mount to already be mounted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably so. "Resource busy" usually means that the device is already mounted afaik

Copy link
Contributor

Choose a reason for hiding this comment

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

sounds like we should try and mount just in case the config doesn't exist to automatically mount it (but allow the error if it fails... maybe log a warning)

Copy link
Contributor Author

@JeroenSoeters JeroenSoeters Mar 18, 2023

Choose a reason for hiding this comment

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

We should probably do that for all mounts we deem essential. I'm wondering though if we should do this now or if we can postpone this to later when we allow for kernels other than the ones we control - if we ever are planning to let users provide their own kernel this at all, as this introduces a whole set of other challenges. Until that point we know with 100% certainty what is mounted and what is not.

Copy link
Contributor

Choose a reason for hiding this comment

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

Lets postpone until later once we have better control of the kernels we are running.

@krisnova krisnova merged commit 84fc1bf into aurae-runtime:main Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants