Re: [make-initrd] [PATCH v6 08/22] bootchain-waitdev: introduces an optional waitdev_timeout

[Leonid Krivoshein <klark.devel@gmail.com>]

26.10.2021 17:05, Alexey Gladkov пишет:
> On Tue, Oct 26, 2021 at 03:55:52PM +0200, Alexey Gladkov wrote:
>> On Tue, Oct 26, 2021 at 02:25:55PM +0300, Leonid Krivoshein wrote:
>>> 26.10.2021 13:55, Alexey Gladkov пишет:
>>>> On Sun, Oct 24, 2021 at 08:21:08PM +0300, Leonid Krivoshein wrote:
>>>>> "waitdev_timeout" describes a common timeout for all "waitdev" steps
>>>>> in the "bootchain". Defining a timeout allows to use a fallback if
>>>>> the specified devices are not ready yet. By default is not set, which
>>>>> makes to wait forever.
>>>>>
>>>>> Signed-off-by: Leonid Krivoshein <klark.devel@gmail.com>
>>>>> ---
>>>>>    features/bootchain-waitdev/README.md           |  3 +++
>>>>>    .../etc/initrd/cmdline.d/bootchain-waitdev     |  1 +
>>>>>    .../data/lib/bootchain/waitdev                 | 18 ++++++++++++++++++
>>>>>    .../data/lib/initrd/pre/bootchain/300-waitdev  |  3 +++
>>>>>    4 files changed, 25 insertions(+)
>>>>>
>>>>> diff --git a/features/bootchain-waitdev/README.md b/features/bootchain-waitdev/README.md
>>>>> index 126a904..d6bae63 100644
>>>>> --- a/features/bootchain-waitdev/README.md
>>>>> +++ b/features/bootchain-waitdev/README.md
>>>>> @@ -13,6 +13,9 @@ feature. It allows to wait a specified block or character special devices.
>>>>>       the same as `root=`, but with optional `CDROM:` prefix. This parameter can be
>>>>>       specified more than once depending on how many times a corresponding element
>>>>>       is mentioned in the `bootchain`.
>>>>> +- `waitdev_timeout` describes a common timeout for all `waitdev` steps in the
>>>>> +  `bootchain`. Defining a timeout allows to use a fallback if the specified
>>>>> +  devices are not ready yet. By default is not set, which makes to wait forever.
>>>> Почему он общий между всеми waitdev ???
>>> Логика в этом железная: если для сборки корня шагам, следующим за
>>> wiatdev'ами, нужны все указанные устройства, то не имеет значения, какого
>>> устройства в цепочке не хватает. Это очень похоже на общий rootdelay и схему
>>> "И". Общий таймаут нужен для того, чтобы не ждать вечно и дать возможность
>>> отработать некоему fallback'у, следующим за этими шагами, или выдать общий
>>> на все отказы диалог, и вернуться в начало.
>> Аналогия с rootdelay= тут не корректна т.к. rootdelay= распространяется на
>> весь процесс загрузки. Я бы понял, если бы timeout распространялся на весь
>> bootchain=, тогда было бы логично.
>>
>> Таймаут логичен если после него будет либо ошибка, либо предполагаются
>> какие-то действия.
>>
>> Я предложил добавить таймаут как параметр конкретного waitdev.
>>
>>>> Получается, если первый waitdev прождал весь wait_timeout, то остальные
>>>> даже пробовать не будут, но это же неправильно так как они другой девайс
>>>> ожидают.
>>> Можно было бы им дать попробовать без дополнительных sleep'ов, но есть два
>>> контраргумента:
>>>
>>> 1) Цепочка могла быть построена исходя из знания, что к устройству "Б"
>>> нельзя обращаться, пока не будет подключено устройство "А". Обращение к "Б"
>>> без готовности "А" может привести к нехорошим последствиям, типа зависания.
>>>
>>> 2) Чтобы отработал fallback, по всей видимости, последний waitdev не должен
>>> быть готов.
>> А как происходит этот fallback ?
>>
>> В твоей реализации если достигнут таймаут, то последующие waitdev просто
>> exit 0 делают и невозможно понять дождались они чего-то или нет.
>>
>> Получается, что следующий шаг может только гадать о результате waitdev.
>>
>> Потому что мне сейчас приходит в голову сделать параметр (или шаг)
>> onfail, но это явно не то чем пользуешься ты для failback.
> Ох. Ты спрятал его в 16 патче в next_bootchain. Но тогда у меня всё равно
> вопрос, как будет происходить fallback если next_bootchain waitdev не
> вызывает ?

Без вызова next_bootchain и многоходовочки:

waitdev,waidev,localdev waitdev_timeout=7 waitdev=... waitdev=... 
altboot_localdev=...

Имеем два wiatdev, один localdev и общий таймаут на все waitdev'ы.

Если первый или второй waidev не уложатся в 7 секунд, т.е. если за 
заданное время не будет найдено всех заданных устройств waitdev, не 
тормозим, а переходим к следующему шагу localdev. В этом суть fallback'а 
и общего таймаута. localdev может проверить результат предыдущего шага, 
ему достаточно проверить только последний waitdev.

В одном из первых коммитов давалось объяснение, что в ближайшее время 
планируется заапстримить несколько модулей bootchain, так что не 
удивительно, что мы не видим всей картины сразу.


>>> Если игнорировать эти моменты, можно дать хотя бы попробовать проверить
>>> готовность следующих устройств без дополнительных sleep'ов, благо асинхронно
>>> работающая конструкция это позволяет сделать.
>>>
>>>
>>>> Например, мы ждём сначала cdrom, а потом usb-флешку с ключом.
>>>>
>>>>>    ## Example
>>>>> diff --git a/features/bootchain-waitdev/data/etc/initrd/cmdline.d/bootchain-waitdev b/features/bootchain-waitdev/data/etc/initrd/cmdline.d/bootchain-waitdev
>>>>> index 3544c25..6929a86 100644
>>>>> --- a/features/bootchain-waitdev/data/etc/initrd/cmdline.d/bootchain-waitdev
>>>>> +++ b/features/bootchain-waitdev/data/etc/initrd/cmdline.d/bootchain-waitdev
>>>>> @@ -1 +1,2 @@
>>>>>    register_array string WAITDEV
>>>>> +register_parameter number WAITDEV_TIMEOUT
>>>>> diff --git a/features/bootchain-waitdev/data/lib/bootchain/waitdev b/features/bootchain-waitdev/data/lib/bootchain/waitdev
>>>>> index 60464d9..fa99c45 100755
>>>>> --- a/features/bootchain-waitdev/data/lib/bootchain/waitdev
>>>>> +++ b/features/bootchain-waitdev/data/lib/bootchain/waitdev
>>>>> @@ -3,6 +3,14 @@
>>>>>    . bootchain-sh-functions
>>>>>    check_parameter WAITDEV
>>>>> +
>>>>> +timeout=
>>>>> +timecnt=/.initrd/bootchain/waitdev/TIMECNT
>>>>> +
>>>>> +[ ! -s "$timecnt" ] ||
>>>>> +	read -r timeout < "$timecnt" ||:
>>>>> +[ "$timeout" != 0 ] ||
>>>>> +	exit 0
>>>>>    devspec="$(get_parameter WAITDEV)"
>>>>>    while [ -n "$devspec" ]; do
>>>>> @@ -25,4 +33,14 @@ while [ -n "$devspec" ]; do
>>>>>    	fi
>>>>>    	sleep 1
>>>>> +	[ -n "$timeout" ] ||
>>>>> +		continue
>>>>> +	timeout=$(($timeout - 1))
>>>>> +
>>>>> +	if [ "$timeout" = 0 ]; then
>>>>> +		message "device waiting timeout exceedded"
>>>>> +		break
>>>>> +	fi
>>>>>    done
>>>>> +
>>>>> +[ -z "$timeout" ] || printf '%s\n' "$timeout" > "$timecnt"
>>>>> diff --git a/features/bootchain-waitdev/data/lib/initrd/pre/bootchain/300-waitdev b/features/bootchain-waitdev/data/lib/initrd/pre/bootchain/300-waitdev
>>>>> index 3642722..5e0f040 100755
>>>>> --- a/features/bootchain-waitdev/data/lib/initrd/pre/bootchain/300-waitdev
>>>>> +++ b/features/bootchain-waitdev/data/lib/initrd/pre/bootchain/300-waitdev
>>>>> @@ -5,6 +5,9 @@
>>>>>    dir=/.initrd/bootchain/waitdev
>>>>>    mkdir -p -- "$dir"
>>>>> +[ -z "${WAITDEV_TIMEOUT-}" ] ||
>>>>> +	printf '%s\n' "$WAITDEV_TIMEOUT" >"$dir"/TIMECNT
>>>>> +
>>>>>    i=0
>>>>>    while [ "$i" -lt "${WAITDEV:-0}" ]; do
>>>>>    	touch "$dir/$i"
>>>>> -- 
>>>>> 2.24.1
>>>>>
>>>>> _______________________________________________
>>>>> Make-initrd mailing list
>>>>> Make-initrd@lists.altlinux.org
>>>>> https://lists.altlinux.org/mailman/listinfo/make-initrd
>>>>>
>>> -- 
>>> Best regards,
>>> Leonid Krivoshein.
>>>
>>> _______________________________________________
>>> Make-initrd mailing list
>>> Make-initrd@lists.altlinux.org
>>> https://lists.altlinux.org/mailman/listinfo/make-initrd
>> -- 
>> Rgrds, legion
>>
>> _______________________________________________
>> Make-initrd mailing list
>> Make-initrd@lists.altlinux.org
>> https://lists.altlinux.org/mailman/listinfo/make-initrd

-- 
Best regards,
Leonid Krivoshein.