Re: [make-initrd] udhcpc script в фиче network

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

Привет!


23.09.2021 14:38, Alexey Gladkov пишет:
> On Thu, Sep 23, 2021 at 05:40:10AM +0300, Leonid Krivoshein wrote:
>>> Этот репозитории повторяет стратегию make-initrd-propagator, который
>>> как-то встраивался в make-initrd. И то и другое отдельно стоящие вещи в
>>> себе. Новый, правда, интегрирован с основным кодом лучше.
>> Я так понимаю, это всё из-за интеграции с network. Но посуди сам: pipeline и
>> network, плюс ещё несколько новых фич, это и есть тот набор, что ты сделал
>> на замену пропагатора, я лишь приделываю к этому диалоги и совместимость с
>> нынешним stage2. Если что-то не так приделываю, можно же это предметно
>> обсуждать. Да, я сначала думал делать полный или частичный форк фичи
>> network, потому что были большие сомнения, что удастся с ней
>> интегрироваться. Но это последнее, что нужно было altboot, чтобы быть больше
>> похожим на propagator (по возможностям) и удалось обойтись без форка, в
>> отличии от pipeline. Ты можешь сказать, чем плоха такая интеграция? http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918
>> -- работает просто идеально.

Тут надо пояснить, что о работе и интеграции говорилось с учётом ранее 
сказанного и того факта, что waitnet времянка снаружи, а не 
заапстримленный код внутри make-initrd. При создании waitnet ставилась 
совершенно определённая цель и в этом плане она выполнена на 100%, далее 
объясню, почему...


> К вопросу о ревью: вот и как предполагается комментировать код с git.alt ?
> Ок, буду по ссылкам.
>
> http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918#l12

Уже понял, что лучше отдельными патчами в рассылку. Просто, когда-то 
давно ты и так принимал.


> Про это я уже писал. Это проверяется не так.

С has_feature() я разобрался. И в новой версии уже она. Но пока код не 
заапстримлен, полагаться на API в make-initrd и тем более привязываться 
жёстко к каким-то фичам весьма рисково: любая твоя новая сборка с таким 
изменением сможет поломать работу ещё не заапстримленного altboot, 
поэтому я иду по пути минимальной привязки на данном этапе. Поэтому и:

> Остальной интегрируется так, что не использует ничего из того с чем
> интегрируется.
>
> http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918#l32
>
> В network-sh-functions есть ipv4_enabled.

Разумеется, я внимательно изучил код фичи и видел эту функцию.


> http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918#l51
>
> Теряются абсолютно все ошибки. Ошибки в ip=, в macaddr, в указании масок.
> Вместо всего этого будет "can't reconfigure network settings". Этого быть
> не должно.

Тут я полностью согласен. Но уже говорил, что код не финальный. waitnet 
-- времянка для выяснения конкретной возможности. Если выкинуть этот 
код, выкинуть шаг waitnet из цепочки загрузки, все 4 сетевых метода 
сейчас будут работать без него. Надо будет указывать ip=dhcp в 
/proc/cmdline или задавать другие сетевые настройки. Поэтому отправка в 
/dev/null сейчас сделана чисто чтобы не портить внешний вид диалога, 
наверное, лучше отправлять в 1>&2 в этом месте.


> Также я уже говорил, что я думаю про дёргание /lib/initrd/cmdline.d/network.
>
> http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918#l56
>
> Этот шаг можно выполнить лишь один раз.

Не согласен! :-) И waitnet демонстрирует это без чистки предыдущей 
конфигурации. Объясню почему. Сейчас waitnet смотрит, была ли выполнена 
конфигурация. И если конфигурировался только lo, то повторный запуск без 
чистки ни на что не повлияет. Он же не создаст дублирующего правила 
udev, т.к. перед дёрганием явно задаётся IFNAME="0".


> Это не вяжется у меня с
>
> "Can't bridge up network interface. Try with other network settings, wired
> connection and/or cable."
>
> Если пользователь попробует тот же интерфейс с другими настройками, то
> второй раз проверки не будет ???

На этом диалоге ранее выполнялся перезапуск машины, в версии 0.1.5-alt3 
решил в этом месте перезапустить altboot -- сейчас тут возврат в 
начальное меню, где можно выбрать другой метод загрузки, не по сети. 
Т.е., прошло 16 секунд, сеть так и не поднялась. Если этого мало, 
пользователь дойдёт сюда снова. Но может и другой путь выбрать для загрузки.


> http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918#l65
>
> Хардкод имени интерфейса. is_loopback().

Не только, это ещё и break loop / защита от пустого каталога. Обычно я 
тут ставлю "_".


> http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918#l67
>
> Для чего эта проверка ?

Чтобы предотвратить дальнейшую работу цикла, если мы имеем дело не с 
сетевым интерфейсом. И ещё проверка того факта, что с момента 
вышестоящей команды ls интерфейс не исчез.


> http://git.altlinux.org/tasks/index/sisyphus/tested/285722/gears/100/git?p=git;a=blob;f=bootchain-waitnet/data/lib/bootchain/waitnet;h=9f555ab3a44c83e2c54c949f2d9401d6f6995ed2;hb=0b2522e81ea6279295bae02a8aa77e84c86d3918#l73
>
> Я полагаю это проверка на то что интерфейс поднят ? Если да, то is_link_up
> Если проверка на присутствие адреса, то "/.initrd/online/$NET_IF".

Да, разумеется, про более тесную интеграцию я уже выше написал. Где-то 
ранее мы как раз обсуждали, ты же сам и говорил, что надёжнее не дёргать 
внутренности make-initrd, а проверять через ip.

waitnet -- "проверка боем" того факта, что внутри make-initrd можно 
повторно дёрнуть фичу network, выполнив конфигурирование в том случае, 
если его ещё не было, возможность передать ему вполне конкретные 
настройки. Правильное название этой фичи -- bootchain-network, 
правильное название шага с диалогами (!) -- neconfig или netsetup, 
основная цель -- "не делать форк фичи network" достигнута.

Разумеется, netconfig/netsetup при апстириме нужно будет интегрировать с 
фичей network более жёстко, но тогда тебе придётся следить в обеих 
местах, чтобы не разъезжалось. Собственно, именно это и было целью 
эксперимента "waitnet", и целью одной из так и не состоявшихся встреч -- 
обсудить принципиальную возможность интеграции существующих фич с 
bootchain/interactive.


>>> Кроме того, если тебе нужно поведение propagator, то он уже есть и незачем
>>> его переизобретать.
>> Выше озвучивались не просто общие слова -- у нас куча граблей и нет способа
>> быстро перейти на make-initrd + pipeline с ними. Вот простейший пример, мы
>> готовимся избавиться от isolinux для legacy/x86, но сколько это будет
>> тянуться -- неизвестно. Он сам (его брэндинг) добавляет опции пропагатора в
>> загрузку и на это приходится ориентироваться, так как если не работает,
>> сразу баг.
> Я уж не знаю вашей кухни, но вообще-то, да, это сразу бага.
> Бага на брэндинг. Он передаёт устаревшие параметры.

Всё так, но надо учитывать, что их куча, и нет цели и возможности 
исправить это всё сразу.


> Новый make-initrd+bootchain всё равно требует либо дополнительных, либо
> других параметров для включения новых фич.

Да, но только в тех местах, где добавляются новые фичи, типа netstart, 
где требуется поддержка их со стороны stage2, так они будут предметно 
исправляться/добавляться. И даже в этих случаях стараюсь пока справиться 
с проблемой в stage1. Подход тут такой похож на твой -- make-initrd же 
решает проблемы и зависимостями в ядре, и фирмварью, где возникает, итд. 
Вот и altboot это первая часть альтового инсталлятора, а не только 
загрузка. Никто на эту конструкцию не переползёт, пока она не взлетит.


>>> Считаю ли я, что поведение propagator нужно воспроизводить ? Нет.
>> В каком смысле? Я не копировал его повеление в точности. Но методы -- нужны,
>> совместимость со stage2 -- нужна. Многое сделано лучше и модульно, это
>> нельзя назвать 100% копией поведения. И всё делалось в пределах исходной
>> концепции pipeline, код декомпозирован.
> Если ты не копируешь повеление в точности, то пожалуйста поясни свой
> тезис:
>
>> Но сейчас и тебе надо принять, как данность, что пусть
>> "функционал пропагатора" с твоей точки зрения не идеален, лучше чтобы он был
>> внутри make-initrd
> Если ты не занимаешься копированием, то неправильного "функционала
> пропагатора" в новом коде быть не должно.

Тут я с тобой полностью согласен. И я тоже не хочу пихать в make-initrd 
плохой код.


>>> Я считаю, что должна быть сохранена совместимость до определённой степени.
>>> Я не считаю, что нужна полная копия поведения. Для последнего у вас есть
>>> сам propagator.
>> Его уже начали капитально переделывать под новые реалии. Но без меня.
> http://git.altlinux.org/gears/p/propagator.git?p=propagator.git;a=commitdiff;h=08639c8fafa30e0f78d19f1c5831fd2b1cc2bc02
>
> Это, кстати, интересно.

Да, там много интересного и полезного, хоть я и не со всем согласен. 
Кстати, вывод на /dev/ttyprintk сейчас в bootchian реализуем через 
сборку образа с BC_LOG_VT=printk.


>>>> Можно и нужно обсуждать, как лучше сделать это, не
>>>> ломая совместимости, и учитывая потребности перехода на что-то более
>>>> правильное в перспективе.
>>> Ты сам себе противоречишь. Я должен принять, как данность функционал
>>> пропагатора и не ломая совместимости с кодом 17 летней (на самом деле
>>> большей) давности переходить на что-то более правильное.
>> Коду инсталлятора уже 17 лет?
> Ваш _инсталлятор_ был написан для Server 4 (который был ещё до branch4).
> Этот инсталлер, как и предыдущий использовал propagator как stage1. Так
> что вашему инсталлеру уже примерно 11-12 лет, а propagator же был за долго
> до него.

Тебе история видней, я лишь борюсь с конкретными взрывами. :-)


> [...]
> Обычно так и происходит. Коммитер пишет код, тестирует, документирует его,
> отдаёт в апстрим и идёт своей дорогой. Но так происходит лишь тогда, когда
> апстрим понимает и согласен со всеми предложенными изменениями.
>
> То что ты написал не подпадает под последнее.

Ну вот опять, не надо спешить с выводами. Будем над этим работать!


> У меня есть много вопросов
> как архитектуре фич, так и к реализации. Когда ты решишь разрешишь все эти
> вопросы, тогда я заберу код, а ты, если хочешь, можешь не участвовать в
> его дальнейшей судьбе.

ОК


>
>> Но я могу помогать с поддержкой, по возможности, хотя слабо представляю,
>> как это возможно, когда это войдёт в состав make-initrd.
> После вхождения изменений в мой репозиторий я не требую от тебя какой-то
> поддержки. Но такое возможно лишь при разрешении всех моих вопросов.

OK


>
>>> Потому что сейчас всё что, сказал выглядит не очень здорово. Выглядит так,
>>> что пропагатор переписанный разом на форк pipeline мне пытаются скинуть на
>>> поддержку.
>> Да, есть немного и такого дела. А как бы ты хотел? Я тебя прекрасно понимаю.
>> Вчера собрались регулярки и не грузится RPi4, сразу ко мне вопрос. В
>> результате altboot оказался не причём, но так мне хотя бы спокойнее будет, а
>> тебе какая разница, 80 тыс. строк кода поддерживать или 86 тыс., ты же к
>> такому уже привык!? :-)
> Так вот оно что.

Не, ну смайлики тоже надо учитывать. ))


> То есть ты просто единовременно переписал propagator на шелле, а не на
> make-initrd, потому что твой код использует остальной мой код по
> минимуму, получил make-initrd-propagator, но теперь с банановым вкусом и
> хочешь, чтобы уже с новым неподдерживаемым кодом возился я (ты сам
> пишешь, что не тянешь его поддержку).
>
> Нет, Леонид, так просто у тебя скинуть на меня это не получится. Я заберу
> у тебя только то с чем буду полностью согласен.

OK


>
>>> Патчей или PR так и не было.
>> Я собирал тестовые задания с патчами, приводил тут на них ссылки. Нет у меня
>> на гитхабе аккаунта или я не очень понимаю в этой терминологии.
> Леонид, я не сотрудник, который работает с тобой. У меня нет возможности
> анализировать тестовые задания и патчи в них.
>
> Патчи можно слать по почте в рассылку или лично, можно завести аккаунт на
> github и делать PR там, как это делает Пётр Михалицын.
>
> Ещё раз для ясности: Я не готов лазать по каким-то там тестовым заданиям,
> пакетам в сизифе с целью "а что бы такого ещё запстримить". Мне это не
> нужно.
>
>>>> А полная совместимость с пропагатором требует другого синтаксиса
>>> А зачем эта совместимость ? Ты пишешь про это как про аксиому.
>> Потому что есть целая куча пакетов и документации в продуктах к ним, которые
>> его используют. Например, для работы со слоями NFS, для сетевой установки. Я
>> уже говорил выше, что речь о совместимости именно с этими многолетними
>> наработками, которые разом не выкинуть, не заменить.
> Их не нужно выкидывать. Они могут быть реализованы иначе. Я не говорю, что
> нужно назло переименовывать всё. Я говорю, что старый подход плохо
> реализуется в новом коде, то его нужно менять и менять документацию.

Реально, реализуемо, но не в обозримом будущем. Нет воли, нет 
исполнителей. Даже то, что мы с тобой с разных сторон делаем, пытаясь 
заменить propagator, делается с 2018 года.


> [...]
> А тут ты про некую последовательность коммитов говорил, но где они ???
> Я до сих пор не получил ни одного коммита. Все эти разговоры пустая
> болтовня какая-то.

И с этим вроде разобрались, а то github, ещё PR какой-то.


-- 
Best regards,
Leonid Krivoshein.