Skip to content
New issue

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

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

Already on GitHub? # to your account

Support configuring mount locations within guests #867

Merged
merged 1 commit into from
May 27, 2022

Conversation

chancez
Copy link
Contributor

@chancez chancez commented May 23, 2022

Closes #659

Tested with both sshfs and 9p and confirmed that the mounts were configured in the location specified by mounts[*].mountPoint.

eg:

mounts:
- location: "~/Movies"
  mountPoint: "/movies"

@jandubois
Copy link
Member

Needs documentation in examples/default.yaml.

Replacing the empty Guest with the Location value should happen in limayaml.FillDefaults. This includes properly merging the value from defaults.yaml and override.yaml there as well.

@chancez chancez force-pushed the configure_mount_locations branch 2 times, most recently from b1786d9 to b47cb8f Compare May 24, 2022 15:24
@chancez
Copy link
Contributor Author

chancez commented May 24, 2022

Updated the field name to mountPoint, updated logic to handle merging in FillDefault and updated the examples/default.yaml. Also added DCO.

@chancez chancez force-pushed the configure_mount_locations branch from b47cb8f to 310f954 Compare May 24, 2022 15:37
@chancez chancez force-pushed the configure_mount_locations branch 2 times, most recently from b8fbd83 to 72dbe98 Compare May 24, 2022 16:54
jandubois
jandubois previously approved these changes May 25, 2022
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM!

I have some nit-picky comments though. Feel free to address, or maybe @AkihiroSuda is willing to merge as-is; I don't feel too strongly about them.

@jandubois
Copy link
Member

I also just noticed that there is another field Mount.Target:

Target string // abs path, accessible by the User

For consistency it would be nice to rename that as well to Mount.MountPoint I think (only if you still make other changes anyways).

@jandubois jandubois added this to the v0.11.1 milestone May 25, 2022
Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

I don't know why this comment is still pending; Github is getting confused.

Closes lima-vm#659

Signed-off-by: Chance Zibolski <chance.zibolski@gmail.com>
@chancez
Copy link
Contributor Author

chancez commented May 25, 2022

Updated the logs and Target -> MountPoint.

Copy link
Member

@jandubois jandubois left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM.

Ok to merge @AkihiroSuda?

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda merged commit 44454dd into lima-vm:master May 27, 2022
@chancez chancez deleted the configure_mount_locations branch May 27, 2022 14:53
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow setting of destination for mounts
3 participants