Re: [cip-dev] [isar-cip-core][PATCH v5] initramfs-overlay-hook: Check file system of INITRAMFS_OVERLAY_STORAGE_DEVICE

[Jan Kiszka <jan.kiszka@web.de>]

On 01.08.24 09:26, Clara Kowalsky wrote:
>
>
> On 31.07.24 19:01, Jan Kiszka wrote:
>> On 23.07.24 14:59, Clara Kowalsky wrote:
>>> In case of ext*, this detects and fixes file system errors in the
>>> partition device before doing the partition mount.
>>> If the partition turns out to be broken, the factory state is recovered.
>>>
>>> Signed-off-by: Clara Kowalsky <clara.kowalsky@siemens.com>
>>> ---
>>>   .../initramfs-overlay-hook/files/overlay.hook      | 11 +++++++++--
>>>   .../files/overlay.script.tmpl                      | 11 ++++++++++-
>>>   .../files/overlay_recovery_action.script           | 14 ++++++++++++++
>>>   .../initramfs-overlay-hook_0.1.bb                  |  8 +++++++-
>>>   4 files changed, 40 insertions(+), 4 deletions(-)
>>>   create mode 100644
>>> recipes-initramfs/initramfs-overlay-hook/files/overlay_recovery_action.script
>>>
>>> diff --git
>>> a/recipes-initramfs/initramfs-overlay-hook/files/overlay.hook
>>> b/recipes-initramfs/initramfs-overlay-hook/files/overlay.hook
>>> index 8b00ecf..6f634c5 100644
>>> --- a/recipes-initramfs/initramfs-overlay-hook/files/overlay.hook
>>> +++ b/recipes-initramfs/initramfs-overlay-hook/files/overlay.hook
>>> @@ -22,6 +22,13 @@ esac
>>>
>>>   . /usr/share/initramfs-tools/hook-functions
>>>
>>> +hook_error() {
>>> +    echo "(ERROR): $1" >&2
>>> +    exit 1
>>> +}
>>> +
>>>   manual_add_modules overlay
>>> -copy_exec /usr/bin/mountpoint
>>> -copy_exec /usr/bin/awk
>>> +copy_exec /usr/bin/mountpoint || hook_error "/usr/bin/mountpoint not
>>> found"
>>> +copy_exec /usr/bin/awk || hook_error "/usr/bin/awk not found"
>>> +copy_exec /usr/sbin/e2fsck || hook_error "/usr/sbin/e2fsck not found"
>>> +copy_exec /usr/sbin/mke2fs || hook_error "/usr/sbin/mke2fs not found"
>>
>> Is there no "copy_exec_and_complain+fail_if_missing"? This pattern looks
>> like it does not scale very well across all our scripts (including isar
>> upstream).
>>
> I have not found anything like this that covers both in one command.
>
>>> diff --git
>>> a/recipes-initramfs/initramfs-overlay-hook/files/overlay.script.tmpl
>>> b/recipes-initramfs/initramfs-overlay-hook/files/overlay.script.tmpl
>>> index 42eb59c..c6f69b9 100644
>>> --- a/recipes-initramfs/initramfs-overlay-hook/files/overlay.script.tmpl
>>> +++ b/recipes-initramfs/initramfs-overlay-hook/files/overlay.script.tmpl
>>> @@ -31,12 +31,21 @@
>>> ovl_partition_device="${INITRAMFS_OVERLAY_STORAGE_DEVICE}"
>>>   ovl_storage_path="${INITRAMFS_OVERLAY_STORAGE_PATH}"
>>>   ovl_lower_dirs="${INITRAMFS_OVERLAY_PATHS}"
>>>   ovl_mount_option="${INITRAMFS_OVERLAY_MOUNT_OPTION}"
>>> +ovl_recovery_script="/scripts/${INITRAMFS_OVERLAY_RECOVERY_SCRIPT}"
>>>
>>>   root_mount_storage=${rootmnt}${ovl_storage_path}
>>>   storage_mount_point="$(echo "${ovl_storage_path}" | awk -F/ '{print
>>> FS$2}' )"
>>> +partition_fstype=$(get_fstype "${ovl_partition_device}")
>>>
>>>   if ! mountpoint -q "${rootmnt}${storage_mount_point}"; then
>>> -    if ! mount -t $(get_fstype ${ovl_partition_device}) \
>>> +    case $partition_fstype in
>>> +    ext*)
>>> +        if ! e2fsck -p -f "$ovl_partition_device" && [ -f
>>> "$ovl_recovery_script" ] && [ -x "$ovl_recovery_script" ]; then
>>
>> Why testing both -f and -x?
>
> I was thinking, what if someone sets INITRAMFS_OVERLAY_RECOVERY_SCRIPT
> to an empty string? Then ovl_recovery_script would be a directory and
> the next line would give a permission denied.
>

OK - but in that case, "file://${INITRAMFS_OVERLAY_RECOVERY_SCRIPT}"
will make things fail much earlier.

Jan