[PATCH] base-files: add option to make /var persistent

[Alberto Bursi]

On 07/08/21 10:40, Stijn Tintel wrote:
> 
> On 7/08/2021 10:05, Alberto Bursi wrote:
>>
>>
>> On 07/08/21 02:46, Stijn Tintel wrote:
>>> On 7/08/2021 02:56, Alberto Bursi wrote:
>>>>
>>>>
>>>> On 06/08/21 21:27, Stijn Tintel wrote:
>>>>> In OpenWrt, /var is symlinked to /tmp by default. This is done to 
>>>>> reduce
>>>>> the amount of writes to the flash chip, which often don't have the
>>>>> greatest durability. As a result, things like DHCP or UPnP lease 
>>>>> files,
>>>>> are not persistent across reboots.
>>>>>
>>>>> Since OpenWrt can run on devices with more durable storage, it makes
>>>>> sense to have an option for a persistent /var. Add an option to make
>>>>> /var persistent. When enabled, /var will no longer be symlinked to 
>>>>> /tmp,
>>>>> but /var/run will be symlink to /tmp/run, as it should contain only
>>>>> files that should not be kept during reboot. The option is off by
>>>>> default, to maintain the current behaviour.
>>>>>
>>>>
>>>> Since it does not really need to recompile anything, I think it
>>>> can/should be handled as a package, not as a compile option.
>>>>
>>>> When you install the package these operations are executed, if you
>>>> remove the package they are undone.
>>>
>>> Removing /var at runtime, which basically happens when you remove the
>>> symlink, is very ugly and has a huge potential for breakage. Having it
>>> as a build option also avoids users from accidentally installing it as a
>>> package. If you disagree, please elaborate further, including measures
>>> to avoid random breakage caused by removing /var at runtime.
>>>
>>
>> My focus was more on "not using a compile option", I don't think it's 
>> necessary to do this at runtime.
>>
>> A simple measure to avoid breakage would be to add something that runs 
>> on boot (either initscript or preinitscript) to do these changes 
>> before any other service that needs that folder is started.
>> So you install the package and then reboot.
>>
>> This approach also allows to make this a permanent change (not an 
>> optional package) and control this function with UCI config, just 
>> change the setting and reboot.
>>
>> IMHO this is good enough for most users, and much better than having 
>> to recompile or do the change manually.
>>
>>
>> For the sake of completeness, (also shameless plug about my project):
>> A more complex way to do it at runtime that avoids breakage is bind 
>> mounting the original folder somewhere else so you can sync the 
>> contents with the new/empty folder, then restarting all services that 
>> need it.
>> See
>> https://github.com/bobafetthotmail/folder2ram/blob/master/debian_package/sbin/folder2ram#L658 
>>
>> (the service restart is not included in that script since it has no 
>> way of knowing what services need the folders, this operation is left 
>> to the user or for OpenMediaVault it's handled by the plugin that also 
>> writes the configuration)
>>
> 
> Appreciate the input. It still sounds overly complex compared to my 
> suggestion, especially for something that most users will probably not 
> use. I don't feel comfortable implementing something like that. I'll 
> just keep using my patch locally then, as I have done for almost five 
> years.
> 

I'm not stopping you, I'm just voicing my opinion.

Having this as a compile option is still better than nothing, please 
don't drop this just because of my feedback.

-Alberto