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

[Alexey Gladkov <gladkov.alexey@gmail.com>]

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 не
вызывает ?

> > Если игнорировать эти моменты, можно дать хотя бы попробовать проверить
> > готовность следующих устройств без дополнительных 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

-- 
Rgrds, legion