From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Tue, 26 Oct 2021 15:55:52 +0200 From: Alexey Gladkov To: make-initrd@lists.altlinux.org Message-ID: <20211026135552.wratyys3jkvboqm6@example.org> References: <20211024172108.668CDA5E4C@lists.altlinux.org> <20211026105516.jfbatbtjfah74vou@example.org> <37c46f84-217f-81bd-56a9-1d5b8d02d670@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <37c46f84-217f-81bd-56a9-1d5b8d02d670@gmail.com> Subject: Re: [make-initrd] [PATCH v6 08/22] bootchain-waitdev: introduces an optional waitdev_timeout X-BeenThere: make-initrd@lists.altlinux.org X-Mailman-Version: 2.1.12 Precedence: list Reply-To: make-initrd@lists.altlinux.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Oct 2021 13:55:57 -0000 Archived-At: List-Archive: 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 > > > --- > > > 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. > Если игнорировать эти моменты, можно дать хотя бы попробовать проверить > готовность следующих устройств без дополнительных 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