rkt: calculate real dataDir path #2399
Conversation
If dataDir or some of its parents are symlinks all seems to work correctly except the gc. That's because stage0.MountGC determines which mountpoints should be unmounted filtering the mountpoints returned from /proc/self/mountinfo with the dataDir value. These mountpoints are the real path so, if the dataDir or its parents are symlinks, no mountpoint will match. This patch evaluates the dataDir real path.
eade3a2 to
b02fd2f
Compare
| } else { | ||
| if config.Paths.DataDir != "" { | ||
| dataDir = config.Paths.DataDir | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we can simplify this if-else cascade a bit:
if dataDir == "" {
config, err := getConfig()
if err != nil {
stderr.PrintE("cannot get configuration", err)
os.Exit(1)
}
dataDir = config.Paths.DataDir
}
There was a problem hiding this comment.
@s-urbaniak Agree. I just kept the previous logic to bring less pollution to the patch. But I'll change it if you're ok.
There was a problem hiding this comment.
Let's address this on a follow-up: #2421
|
stylistic nit, else LGTM; I'd also suggest to introduce a functional test for symlink resolving, but unsure where to put this either ;-) @alban thoughts re: test? |
|
I am not sure that tests are needed for that... If you want to add tests, you can look at ctx.go: This function decides which temporary directory to use for the tests. It could create new symlinks, and all tests would run in a directory with symlink components in its path. |
|
LGTM Issue for tests: #2422 |
If dataDir or some of its parents are symlinks all seems to work correctly
except the gc.
That's because stage0.MountGC determines which mountpoints should be
unmounted filtering the mountpoints returned from /proc/self/mountinfo
with the dataDir value. These mountpoints are the real path so, if the
dataDir or its parents are symlinks, no mountpoint will match.
This patch evaluates the dataDir real path.
If some functional tests are needed I'm not sure under what file under
./tests/I should put them.