make bind mounting files over existing files on read-only filesystems… #2928

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

bjne commented Mar 31, 2016

systemd-nspawn tries to touch(where) without checking if it already
exists. This causes problems with read-only filesystems, and is in
general not needed. Moving this block under ENOENT solves this
issue, and also prevents mkdir from beeing run when directory exists,
even tho that case was caught by EEXISTS

bjne added some commits Mar 31, 2016

src/nspawn/nspawn-mount.c
+ else
+ r = touch(where);
+
+ if (r < 0 && r != -EEXIST)
@poettering

poettering Apr 1, 2016

Owner

the r != -EEXIST check can be dropped now if this is moved into the errno == ENOENT block...

Let's keep this minimal and not keep around code that excepts certain errors if we don't need actually need it.

@poettering poettering added the nspawn label Apr 1, 2016

Owner

poettering commented Apr 1, 2016

hmm, I do agree that your patch cleans up the code a bit (which is reason enough to apply it), but i do wonder about the rationale: why precisely do you say that the current code has problems on read-only media? I mean, are you saying that if the file already exists and the image is read-only we get a EROFS instead of an EEXIST?

Owner

poettering commented Apr 1, 2016

what's the second patch about? the first hunk looks like a whitespace borkage?

Owner

poettering commented Apr 1, 2016

and the second hunk of the second patch lacks any explanation?

Owner

poettering commented Apr 1, 2016

oh i see now that the second patch is from #2929? please drop it from this PR?

Contributor

bjne commented Apr 1, 2016

Sorry about this.

New pull request #2939, please close this one.

Owner

poettering commented Apr 1, 2016

Obsoleted by #2939

@poettering poettering closed this Apr 1, 2016

Owner

poettering commented Apr 1, 2016

Still wondering about the precise rationale, see above...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment