Re: [PATCH v3] expand-on-first-boot: Ensure that /tmp is writable

["'Jan Kiszka' via isar-users" <isar-users@googlegroups.com>]

On 03.09.24 20:05, Florian Bezdeka wrote:
> On Tue, 2024-09-03 at 11:05 +0200, Jan Kiszka wrote:
>> On 03.09.24 09:20, 'MOESSBAUER, Felix' via isar-users wrote:
>>> On Thu, 2024-08-15 at 07:07 +0300, Uladzimir Bely wrote:
>>>> On Tue, 2024-08-13 at 13:32 +0300, Uladzimir Bely wrote:
>>>>> On Tue, 2024-08-13 at 09:24 +0000, MOESSBAUER, Felix wrote:
>>>>>> On Tue, 2024-08-13 at 12:17 +0300, Uladzimir Bely wrote:
>>>>>>> On Thu, 2024-07-25 at 16:17 +0200, 'Clara Kowalsky' via isar-
>>>>>>> users
>>>>>>> wrote:
>>>>>>>> By setting PrivateTmp, a new file system namespace is created
>>>>>>>> for
>>>>>>>> this
>>>>>>>> service and private /tmp/<service>/tmp and
>>>>>>>> /var/tmp/<service>/tmp
>>>>>>>> subdirectories are mounted, which are only used for processes
>>>>>>>> of
>>>>>>>> this
>>>>>>>> namespace. The service unit receives a mount unit dependency
>>>>>>>> for
>>>>>>>> all
>>>>>>>> mounts required to access /tmp and /var/tmp.
>>>>>>>> This ensures that the /tmp directory is writable for the
>>>>>>>> service,
>>>>>>>> as
>>>>>>>> mktemp is used in expand-last-partition.sh and creates a
>>>>>>>> temporary
>>>>>>>> file.
>>>>>>>>
>>>>>>>> Signed-off-by: Clara Kowalsky <clara.kowalsky@siemens.com>
>>>>>>>> ---
>>>>>>>>  .../expand-on-first-boot/files/expand-on-first-
>>>>>>>> boot.service     
>>>>>>>>>
>>>>>>>> 1
>>>>>>>> +
>>>>>>>>  1 file changed, 1 insertion(+)
>>>>>>>>
>>>>>>>> diff --git a/meta/recipes-support/expand-on-first-
>>>>>>>> boot/files/expand-
>>>>>>>> on-first-boot.service b/meta/recipes-support/expand-on-first-
>>>>>>>> boot/files/expand-on-first-boot.service
>>>>>>>> index 90c92a39..8e76998b 100644
>>>>>>>> --- a/meta/recipes-support/expand-on-first-boot/files/expand-
>>>>>>>> on-
>>>>>>>> first-boot.service
>>>>>>>> +++ b/meta/recipes-support/expand-on-first-boot/files/expand-
>>>>>>>> on-
>>>>>>>> first-boot.service
>>>>>>>> @@ -16,6 +16,7 @@ Type=oneshot
>>>>>>>>  ExecStart=/usr/share/expand-on-first-boot/expand-last-
>>>>>>>> partition.sh
>>>>>>>>  ExecStartPost=-/bin/systemctl disable expand-on-first-
>>>>>>>> boot.service
>>>>>>>>  ExecStopPost=-/bin/systemctl disable expand-on-first-
>>>>>>>> boot.service
>>>>>>>> +PrivateTmp=true
>>>>>>>>  
>>>>>>>>  [Install]
>>>>>>>>  WantedBy=sysinit.target
>>>>>>>> -- 
>>>>>>>> 2.45.2
>>>>>>>>
>>>>>>>
>>>>>>> Hello all.
>>>>>>>
>>>>>>> After few days having this patch merged we at least twice faced
>>>>>>> the
>>>>>>> issue in CI with running qemuamd64 machine, probably related to
>>>>>>> the
>>>>>>> applied patch.
>>>>>>>
>>>>>>> Error message is "ERROR| No resize output while expected".
>>>>>>> E.g.,
>>>>>>> there
>>>>>>> is no btrfs resize output in VM boot log.
>>>>>>>
>>>>>>> The reason of non-running expand-on-first-boot serivce is:
>>>>>>>
>>>>>>> ```
>>>>>>> [    5.578636] systemd[1]: local-fs-pre.target: Job expand-on-
>>>>>>> first-
>>>>>>> boot.service/start deleted to break ordering cycle starting
>>>>>>> with
>>>>>>> local-
>>>>>>> fs-pre.target/start
>>>>>>> ```
>>>>>>
>>>>>> Interesting, I observed this same issue as well, but thought it
>>>>>> comes
>>>>>> from a downstream part. You're right, this cannot work:
>>>>>>
>>>>>> Citing systemd.exec:
>>>>>>
>>>>>> Similarly, units with PrivateTmp= enabled automatically get mount
>>>>>> unit
>>>>>> dependencies for all mounts required to access /tmp/ and
>>>>>> /var/tmp/.
>>>>>> They will also gain an automatic After= dependency on systemd-
>>>>>> tmpfiles-
>>>>>> setup.service(8). 
>>>>>>
>>>>>> If /var is the partition to be resized, this will break.
>>>>>>
>>>>>> Felix
>>>>>>
>>>>>
>>>>> The dependency conflict seems to be here:
>>>>>
>>>>> - expand-on-first-boot.service
>>>>> Before=local-fs-pre.target
>>>>> PrivateTmp=true # This means implicit "After=systemd-tmpfiles-
>>>>> setup.service" dependency0, according to 
>>>>> https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html
>>>>>
>>>>> - systemd-tmpfiles-setup.service 
>>>>> After=local-fs.target
>>>>>
>>>>> - local-fs.target
>>>>> After=local-fs-pre.target
>>>>>
>>>>>
>>>>
>>>> Finally, does this all mean we need to revert this v3 patch and get
>>>> back to "[PATCH v2] expand-on-first-boot: Add /tmp to
>>>> ConditionPathIsReadWrite" variant?
>>>
>>> The conditions are evaluated right before the service starts. By that,
>>> we might have non-deterministic behavior, depending on which service
>>> mounts /tmp (if at all) and when it is started relative to the expand-
>>> on-first-boot.
>>>
>>> I'm wondering if we should better create our own tmpfs in combination
>>> with TMPDIR, just for that service (and drop it after execution). For
>>> obvious reasons, the expanding needs to happen VERY early, but at that
>>> point in time not much can be assumed about the rootfs.
>>>
>>> CC'ing Florian.
>>
>> In many cases, expansion can ran also after the system is fully booted
>> and operational - unless it is then already complaining about too little
>> disk space ;)
> 
> Not sure if I can help here. expand-on-first-boot tends to explode
> every time we touch it. It seems that we don't have tests for most of
> the use cases (like encrypted disks, ...) which makes it hard to prove
> the correctness.
> 
> Couple of things I noticed while scanning / looking at the code:
> 
> - Why do we require /etc to be writable? We only read /etc/fstab
>   right?
> 

Was like that since day #1, and the original author forgot the reason.

> - I think all relevant file systems support growing the file system
>   while being active / online / mounted. Maybe we don't have to run 
>   so early? Might help to reduce rootfs requirements.
> 

Just found 7dff89e9a10b3b5d9fc07f73a74258e51a3fe2dd and
"Before=local-fs-pre.target" - seems we do need to be early in some cases.

> - Do we still need expand-on-first-boot as it is right now? Seems
>   systemd provides x-systemd.growfs flags in /etc/fstab. Which use
>   cases are not supported by that systemd feature? Could we migrate?
> 

Henning and Tobias tried that and had to give up, see
35f9487a849a10e40d4fbb3d9b6f13a25d4a1f96.

> - According to the patch description we need /tmp (or any tmpfs) 
>   so that mktemp is working. 
> 
>   We use the generated tmp directory as mount point only. We never 
>   add / write any files to it. Can't we just create any "random" 
>   (in terms of hardcoded...) directory for that to get rid of the
>   systemd dependency?

/.expand-on-first-boot-mountpoint?

Jan

-- 
Siemens AG, Technology
Linux Expert Center

-- 
You received this message because you are subscribed to the Google Groups "isar-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to isar-users+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/isar-users/0176a32d-9d97-40f1-91cf-7e6a292cb6a3%40siemens.com.