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

saveConfig fails if config file location and /tmp/ are on different filesystems #6

Closed
LongHairedHacker opened this issue Jul 18, 2019 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@LongHairedHacker
Copy link
Contributor

This is another problem I've noticed, while debugging #4.

On many OpenWRT device /etc/config is a read-only filesystem overlaid with a writable filesystem, while/tmp/ is usually an actual tmpfs/ramdisk.

Consider saveConfig() being called for network.
The call to ioutil.TempFile in saveConfig creates a temporary file in\tmp\.
A subsequent call to os.Rename() tries to hardlink the file to /etc/network.
While this a considered best practice to ensure atomic file changes it will fail for this case as the hardlink crosses filesystem boundaries.
A typical error looks like this:
rename /tmp/network012772557 /etc/config/network: invalid cross link

As the user can't choose a different location for the temporary files, this bug renders this library useless on many OpenWRT installations.

@dmke
Copy link
Member

dmke commented Jul 18, 2019

Huh. I didn't think of that (or misremembered how os.Rename operates).

@dmke dmke self-assigned this Jul 18, 2019
@dmke dmke closed this as completed in 5e135c9 Jul 18, 2019
@dmke
Copy link
Member

dmke commented Jul 18, 2019

saveConfig now places a temporary file in /etc/config (or whatever the config root is), named /etc/config/.012772557.network, before writing the config to it and renaming it to /etc/config/network. OverlayFS does support hardlinks, doesn't it?

While this may leave dot files behind if (for whatever reason) the cleanup mechanism fails, I'd prefer those files being present over half-written config files if we directly write to /etc/config/network.

As far as I can tell, UCI itself ignores dot files in /etc/config. I didn't find the C source responsible for this behaviour, but I've empirically tested this on one of our OpenWRT devices.

@LongHairedHacker
Copy link
Contributor Author

Works on our system and looks okay to me.

@dmke dmke added the bug Something isn't working label Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants