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

Add an "all_writable" option to allow overwriting non-writable files in backing directories. #138

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

mindhog
Copy link

@mindhog mindhog commented Dec 11, 2023

It's often useful to create overlays based on read-only filesystems. For example, one might mount a CDROM with the intention of performing local modifications to it, thus avoiding the need to copy the entire contents of the volume to the local filesystem.

The "all_writable" option augments copy-on-write to treat all files and directories on the backing, read-only fiilesystems that are readable by the user as writable. A read-only file's permissions will include the "w" bits in all cases where there is a corresponding "r" bit (for example, "r-xr-x---" will get promoted to "rwxrwx---") and the file/directory will be created with the corresponding permissions during a copy-on-write.

@rpodgorny
Copy link
Owner

thanks for your contribution! sorry it took me so long to look at it. :-(

anyway, do all the tests pass for you? for me 52 (out of 73) fail. :-(

also, is there any specific reason the all_writable flag is set for the entire mount and not for individual branches?

all_writable is copy on write with writable enabled for even read-only files
on the backing filesystems, providing a mutable view of readonly files.
When copying a directory from a read-only filesystem, make the directory
writable for all users that it is readable for in the underlying filesystem.

Also, introduce tests for "all_writable".
@mindhog
Copy link
Author

mindhog commented Apr 3, 2024

[replying again since my e-mail reply didn't seem to go through]

Radek Podgorny wrote:

thanks for your contribution! sorry it took me so long to look at it. :-(

No worries! Thanks for coming back to it :-)

anyway, do all the tests pass for you? for me 52 (out of 73) fail. :-(

They do. I just now rebased the branch on the off chance that something
incompatible was introduced, but still no problems.

What errors are you seeing?

also, is there any specific reason the all_writable flag is set for the entire mount and not for individual branches?

Honestly, it hadn't occurred to me to set it for individual branches.
Offhand, a case where some branches are "all-writable" and others preserve
read-only seems strange and confusing to me -- this feature was basically
targeted at cases where you always want to be able to mutate in the rw
branch.

Do you think there's a use-case for all-writable at the branch level? I could
try to implement that, though frankly I'd be more inclined to leave it at the
mount level with the assumption that it could also be added at the branch
level if/when necessary.

@rpodgorny
Copy link
Owner

so, the reason for failing tests is the hard-coded path for debug file. please remove it completely.

also, can you please post the exact mount command you use for your specific use case? ...i might have some follow-up questions... ;-)

@mindhog
Copy link
Author

mindhog commented Apr 15, 2024

so, the reason for failing tests is the hard-coded path for debug file. please remove it completely.

:-% ... sorry about that. Done.

also, can you please post the exact mount command you use for your specific use case? ...i might have some follow-up questions... ;-)

I typically run a lot of them, but in general the form is something like:

$ unionfs -o all_writable /home/mmuller/changes/sometask=rw:/repo/12345 /home/mmuller/workspace/sometask

Where "/repo/12345" is a mounted, read-only filesystem corresponding to a specific version in a source tree..

@rpodgorny
Copy link
Owner

so, the reason for failing tests is the hard-coded path for debug file. please remove it completely.

:-% ... sorry about that. Done.

also, can you please post the exact mount command you use for your specific use case? ...i might have some follow-up questions... ;-)

I typically run a lot of them, but in general the form is something like:

$ unionfs -o all_writable /home/mmuller/changes/sometask=rw:/repo/12345 /home/mmuller/workspace/sometask

Where "/repo/12345" is a mounted, read-only filesystem corresponding to a specific version in a source tree..

how would swapping the branches work for you? -> "unionfs -o cow /repo/123=ro:/home/user/task=rw /home/user/task"

@rpodgorny
Copy link
Owner

so, the reason for failing tests is the hard-coded path for debug file. please remove it completely.

:-% ... sorry about that. Done.

also, can you please post the exact mount command you use for your specific use case? ...i might have some follow-up questions... ;-)

I typically run a lot of them, but in general the form is something like:

$ unionfs -o all_writable /home/mmuller/changes/sometask=rw:/repo/12345 /home/mmuller/workspace/sometask

Where "/repo/12345" is a mounted, read-only filesystem corresponding to a specific version in a source tree..

how would swapping the branches work for you? -> "unionfs -o cow /repo/123=ro:/home/user/task=rw /home/user/task"

also, i don't see you specifying =ro for /repo

@mindhog
Copy link
Author

mindhog commented Apr 16, 2024

how would swapping the branches work for you? -> "unionfs -o cow /repo/123=ro:/home/user/task=rw /home/user/task"

Seems to me, pretty poorly :-) After that I essentially get:

$ touch /home/user/task/existingfile
touch: cannot touch '/home/user/task/existingfile': Permission denied

So, my goal here is to be able to modify files and directories in the rw branch even if I don't have permission in the ro directory.

also, i don't see you specifying =ro for /repo

Yes, I was wondering about that also. I felt like it might have been the default, but regardless of whether it is or not: those files are not writable.

@mindhog
Copy link
Author

mindhog commented Apr 16, 2024

Yes, "ro" is effectively the default. It would print an error but configure readonly:

                if (strcasecmp(res, "rw") == 0) {
                        uopt.branches[uopt.nbranches].rw = 1;
                } else if (strcasecmp(res, "ro") == 0) {
                        // no action needed here
                } else {
                        fprintf(stderr, "Failed to parse RO/RW flag, setting RO.\n");
                        // no action needed here either
                }

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