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

Standalone #31

Merged
merged 18 commits into from
Jun 25, 2019
Merged

Standalone #31

merged 18 commits into from
Jun 25, 2019

Conversation

NathanTP
Copy link
Contributor

@NathanTP NathanTP commented Jun 5, 2019

Previous versions of firemarshal required sudo to operate. This wasn't intrinsic, just convenient. It also had some artificial dependencies on being a submodule of firesim. This PR removes these requirements so that firesim-software can work on its own.

The firesim submodule was a minor change that doesn't affect behavior significantly (just changes some warnings).

Getting rid of sudo was more involved. I had to switch to guestmount (a FUSE tool). Implications/changes:

  1. Guestmount didn't like the way rsync and cpio worked (it handles permissions in the target system in a weird way). The result is that we don't manipulate file owners when constructing images, and guestmount seems to correctly convert from root to the host user.
  2. We'll need to update firesim's machine-launch script to install libguestfs-tools. Anyone updating will need to install that.
  3. People using this on Ubuntu hosts will need to jump through some extra hoops because guestmount doesn't play nice there. I've documented this at guestmount failing on ubuntu #30 for posterity.

@NathanTP NathanTP requested review from davidbiancolin and sagark June 5, 2019 20:10
@NathanTP
Copy link
Contributor Author

NathanTP commented Jun 5, 2019

BTW: This PR addresses #20

Copy link
Contributor

@davidbiancolin davidbiancolin left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -6,9 +6,6 @@ import logging
import wlutil
import contextlib

if 'RISCV' not in os.environ:
Copy link
Contributor

Choose a reason for hiding this comment

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

You don't depend on this variable anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We never depended on it explicitly. I was just using it as a proxy to detect if you'd sourced sourcme. In reality, we just depend on the riscv toolchain. I just added back a check for that instead.

@NathanTP NathanTP merged commit 5c304b1 into dev Jun 25, 2019
@NathanTP NathanTP mentioned this pull request Jun 25, 2019
@jerryz123 jerryz123 deleted the standalone branch February 27, 2023 23:06
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