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

Create a subdirectory under XDG_RUNTIME_DIR #488

Merged
merged 1 commit into from
Nov 23, 2021

Conversation

dgibson
Copy link
Contributor

@dgibson dgibson commented Nov 23, 2021

When using a root state directory from $XDG_RUNTIME_DIR, or from it's
default value of /run/user/, we don't add a tag specific to youki
to the path. That means the directories for individual containers
will be placed directly in the general use runtime dir.

That's against normal conventions, and could mean that "youki list"
will see files or directories from other software as if they were
youki managed containers. Therefore, add "youki" to the base runtime
path from XDG.

fixes #487

Signed-off-by: David Gibson david@gibson.dropbear.id.au

When using a root state directory from $XDG_RUNTIME_DIR, or from it's
default value of /run/user/<uid>, we don't add a tag specific to youki
to the path.  That means the directories for individual containers
will be placed directly in the general use runtime dir.

That's against normal conventions, and could mean that "youki list"
will see files or directories from other software as if they were
youki managed containers.  Therefore, add "youki" to the base runtime
path from XDG.

fixes youki-dev#487

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
@dgibson
Copy link
Contributor Author

dgibson commented Nov 23, 2021

/cc @c3d

Comment on lines +151 to +153
if create_dir_all_with_mode(&path, uid, Mode::S_IRWXU).is_ok() {
return Ok(path);
}
Copy link
Member

Choose a reason for hiding this comment

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

If it fails, it must return with an error or the process will continue, which is what you are aiming for, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, not immediately. If this create fails we continue onto the remaining options ($HOME/.youki/run then /tmp/youki/<uid>). AFAICT that's what happened before, and I'm not intending to change that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That is correct. If we cannot get the preferred location (which is XDG_RUNTIME_DIR), we try a bunch of other locations that may be suitable before we give up.

@codecov-commenter
Copy link

Codecov Report

Merging #488 (6a49688) into main (e766310) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #488      +/-   ##
==========================================
- Coverage   61.08%   61.07%   -0.01%     
==========================================
  Files          85       85              
  Lines       12416    12417       +1     
==========================================
  Hits         7584     7584              
- Misses       4832     4833       +1     

@Furisto
Copy link
Collaborator

Furisto commented Nov 23, 2021

Looks good to me.

@Furisto Furisto merged commit 580f878 into youki-dev:main Nov 23, 2021
@Furisto
Copy link
Collaborator

Furisto commented Nov 23, 2021

@dgibson Thanks for your contribution!

@dgibson dgibson deleted the runtime-subdir branch November 24, 2021 03:13
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.

determine_root_path() isn't quite right for XDG_RUNTIME_DIR and /run/user/{} cases
4 participants