ALT Linux Bugzilla – Full Text Bug Listing
| Summary: | В copy_chroot_binaries() отсутствует проверка наличия необходимых библиотек | ||
|---|---|---|---|
| Product: | Sisyphus | Reporter: | solo <solo> |
| Component: | installer-scripts-remount-stage2 | Assignee: | Michael Shigorin <mike> |
| Status: | CLOSED FIXED | QA Contact: | qa-sisyphus |
| Severity: | normal | ||
| Priority: | P3 | CC: | mike |
| Version: | unstable | Keywords: | distro-blocker, patch, relnote |
| Hardware: | all | ||
| OS: | Linux | ||
| URL: | https://lists.altlinux.org/pipermail/devel/2015-October/200270.html | ||
| Bug Depends on: | |||
| Bug Blocks: | 21111, 30940 | ||
|
Description
solo
2015-10-09 19:28:57 MSK
Это, кстати, результат изменения графа зависимостей... похоже, придётся тырить/применять код из make-initrd. (В ответ на комментарий №1) > Это, кстати, результат изменения графа зависимостей... > похоже, придётся тырить/применять код из make-initrd. А это идея: Похоже можно заменить вызов cp на вызов /usr/share/make-initrd/tools/put-file из make-initrd -- такой вариант обещает быть более красивым, чем то что у меня на рисовалось (см. http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commitdiff;h=433c72492313dedde71fa783770a85b9f8eaf1bc). С учётом того, что put-file нужно будет вызывать в chroot`е алгоритм будет примерно такой: 1. Проверить, существует ли $destdir/usr/share/make-initrd/tools/put-file (если нет -- выводим предупреждение и ничего не делаем. 2. Создать временный каталог $destdir/tmp/installer.XXX 3. Смонтировать в $destdir/tmp/installer.XXX текущий корень: mount --bind / $destdir/tmp/installer.XXX 4. Выполнить копирование необходимых файлов из $destdir в $destdir/tmp/installer.XXX, используя вызов put-file внутри chroot`а $destdir 5. Отмонтировать $destdir/tmp/installer.XXX 6. Удалить каталог $destdir/tmp/installer.XXX Предлагаю такой вариант -- см. http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commitdiff;h=93bb697665fec05f1f71bb33f9581ef7b8729aa3. PS: chroot нужен для корректной обработки ldd. (В ответ на комментарий №3) > Предлагаю такой вариант -- см. > http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commitdiff;h=93bb697665fec05f1f71bb33f9581ef7b8729aa3. Вариант работает, судя по тестам. PS: Более красиво было бы: 1. Перенести /usr/share/make-initrd/tools/put-{file,tree} в /usr/bin. 2. Выделить в отдельный пакет (put-file, например). 3. Добавить скриптам ключ --from-chroot (что позволит избавиться от mount --bind и в chroot запускать только ldd). (В ответ на комментарий №4) > (В ответ на комментарий №3) > > Предлагаю такой вариант -- см. > > http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commitdiff;h=93bb697665fec05f1f71bb33f9581ef7b8729aa3. > > Вариант работает, судя по тестам. Тестировал: http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commit;h=dffdf4874b933e93a6f0e9fd13899bc2a6622edc installer-scripts-remount-stage2-0.5.2-alt2 -> sisyphus: * Tue Oct 13 2015 Aleksey Avdeev <solo@altlinux> 0.5.2-alt2 - Add copying the libraries necessary for binaries copied (ALT#31351). To copy using a script /usr/share/make-initrd/tools/put-file belonging to an installable distribution. Возможно, блокер bug 30940 стоило снять, хотя баг как таковой оставался: cryptsetup - utility to setup a encrypted disks with LUKS support * Thu Oct 08 2015 Gleb F-Malinovskiy <glebfm@altlinux> 1.6.8-alt2 - Replaced libpwquality with libpasswdqc. * Tue Sep 08 2015 Alexey Shabalin <shaba@altlinux> 1.6.8-alt1 По части залитого пакета 0.5.2: Лёш, большая человеческая просьба пакеты моего авторства заливать всё же по согласованию, т.к. патч в текущем виде я так и не принял; прокомментирую здесь: copy_chroot_binaries() { + if [ ! -x "$destdir$PUTFILE" ]; then + echo "remount: file does not exist or is not available for execution: $destdir$PUTFILE" >&2 + return + fi Здесь ты теоретически ломаешь работу функции без веского повода вместо graceful degradation, когда при отсутствии более совершенного инструмента выбирается менее совершенный (другое дело, что в нынешнем установленном альтовом чруте с ядром make-initrd будет почти обязательно). Этот кусок, вообще говоря, ломает логику и при добавлении -f это может выйти боком на ровном месте в некоторых случаях: if [ -f "$destdir/etc/lvm/lvm.conf" ]; then - mkdir -p /etc/lvm && - cp -p "$destdir/etc/lvm/lvm.conf" /etc/lvm + echo "remount: copying /etc/lvm/lvm.conf" >&2 + chroot "$destdir" "$PUTFILE" "$workdir" "/etc/lvm/lvm.conf" ||: fi (ты потерял &&, а зачем именно для lvm.conf использовать put-file -- я лично так пока и не понял) + umount "$binddir" + rm -rf "$binddir" } Вместо rm -rf напрашивается rmdir -- ты ведь перед монтированием делал пустой каталог :) Т.е. вроде и без блокеров, а в угловых ситуациях разработки и применения ты нечаянно заложил несколько дополнительных мин и этого делать не следовало. (В ответ на комментарий №7) > Возможно, блокер bug 30940 стоило снять, хотя баг как таковой оставался: > > cryptsetup - utility to setup a encrypted disks with LUKS support > * Thu Oct 08 2015 Gleb F-Malinovskiy <glebfm@altlinux> 1.6.8-alt2 > - Replaced libpwquality with libpasswdqc. > * Tue Sep 08 2015 Alexey Shabalin <shaba@altlinux> 1.6.8-alt1 > > По части залитого пакета 0.5.2: Лёш, большая человеческая просьба пакеты моего > авторства заливать всё же по согласованию, т.к. патч в текущем виде я так и не > принял; Поторопился, приношу извения. > прокомментирую здесь: > > copy_chroot_binaries() { > + if [ ! -x "$destdir$PUTFILE" ]; then > + echo "remount: file does not exist or is not available for > execution: $destdir$PUTFILE" >&2 > + return > + fi > > Здесь ты теоретически ломаешь работу функции без веского повода вместо graceful > degradation, когда при отсутствии более совершенного инструмента выбирается > менее совершенный (другое дело, что в нынешнем установленном альтовом чруте с > ядром make-initrd будет почти обязательно). > > Этот кусок, вообще говоря, ломает логику и при добавлении -f это может выйти > боком на ровном месте в некоторых случаях: > > if [ -f "$destdir/etc/lvm/lvm.conf" ]; then > - mkdir -p /etc/lvm && > - cp -p "$destdir/etc/lvm/lvm.conf" /etc/lvm > + echo "remount: copying /etc/lvm/lvm.conf" >&2 > + chroot "$destdir" "$PUTFILE" "$workdir" "/etc/lvm/lvm.conf" ||: > fi > > (ты потерял &&, а зачем именно для lvm.conf использовать put-file -- я лично > так пока и не понял) put-file корректно обрабатывает случай, если /etc/lvm/lvm.conf симлинк, cp (без -L) -- ловил за копированием симлинка. > > + umount "$binddir" > + rm -rf "$binddir" > } > > Вместо rm -rf напрашивается rmdir -- ты ведь перед монтированием делал пустой > каталог :) > > Т.е. вроде и без блокеров, а в угловых ситуациях разработки и применения ты > нечаянно заложил несколько дополнительных мин и этого делать не следовало. OK, Сейчас доработаю. PS: Доработку пакетом, или патчем? (В ответ на комментарий №8) > (В ответ на комментарий №7) ... > > Этот кусок, вообще говоря, ломает логику и при добавлении -f это может выйти > > боком на ровном месте в некоторых случаях: > > > > if [ -f "$destdir/etc/lvm/lvm.conf" ]; then > > - mkdir -p /etc/lvm && > > - cp -p "$destdir/etc/lvm/lvm.conf" /etc/lvm > > + echo "remount: copying /etc/lvm/lvm.conf" >&2 > > + chroot "$destdir" "$PUTFILE" "$workdir" "/etc/lvm/lvm.conf" ||: > > fi > > > > (ты потерял &&, а зачем именно для lvm.conf использовать put-file -- я лично > > так пока и не понял) Не понял, о добавлении какого -f идет речь. В остальном, смены логики не вижу: Было: Попытка копирования (cp) выполнялась только если /etc/lvm создан или существует -- т. к. в противном случаи mkdir вернёт ошибку и && не даст выполнить cp. Стало: Если по каким-то причинам put-file не может создать /etc/lvm -- он обламывается, возвращая ошибку. Т. е. в обоих вариантах в случаи невозможности создать /etc/lvm имеем отсуьсвие копирования $destdir/etc/lvm/lvm.conf и признак ошибки в $?. Прошу указать на слом логики. > > put-file корректно обрабатывает случай, если /etc/lvm/lvm.conf симлинк, cp > (без -L) -- ловил за копированием симлинка. (В ответ на комментарий №8) > (В ответ на комментарий №7) ... > > > > Т.е. вроде и без блокеров, а в угловых ситуациях разработки и применения ты > > нечаянно заложил несколько дополнительных мин и этого делать не следовало. > > OK, Сейчас доработаю. Доработанный вариант -- http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commitdiff;h=260ce0d154daa337a01f20c7b11d0065a5e10771 > > PS: Доработку пакетом, или патчем? Прошу смотреть http://git.altlinux.org/tasks/151373/. (В ответ на комментарий №9) > Не понял, о добавлении какого -f идет речь. #!/bin/sh -f (у нас порой производится устрожение скриптов, а тут последствия при таком стали бы зависимы от ситуации времени выполнения -- т.е. тестирование изменения могло бы не показать добавленной проблемы, которая потом могла вылезти фатальным сбоем инсталятора в ситуации, где ранее работало). (В ответ на комментарий №10) > PS: Доработку пакетом, или патчем? Обычно бывает удобней сперва отсмотреть и если что -- поправить (--amend), но на глаз теперь почти порядок; единственно что не понял: зачем делать put-file применительно к /etc/lvm/lvm.conf, оно же не должно быть бинарником -- или это ты механически переделал на put-file и второе вхождение cp? :) PPS: по результатам теста будто работает; на всякий обрати внимание -- regular-server.iso у меня с таким пакетом встал (при этом /mnt/destination/sbin/lvm и прочие нужные скрипту бинари были скопированы в /sbin инсталятора и не имели выпадений по библиотекам, судя по ldd), но не смог загрузиться с lvm-ного корня: initrd не выполнил vgchange -ay, после ручного подъёма в lvm, монтирования в /root и exec /root/sbin/init установка загрузилась. В смысле закидывай. (В ответ на комментарий №12) > (В ответ на комментарий №9) > > Не понял, о добавлении какого -f идет речь. > > #!/bin/sh -f (у нас порой производится устрожение скриптов, а тут последствия > при таком стали бы зависимы от ситуации времени выполнения -- т.е. тестирование > изменения могло бы не показать добавленной проблемы, которая потом могла > вылезти фатальным сбоем инсталятора в ситуации, где ранее работало). Судя по man sh, при sh -f поведение измениться не должно, т. к. нет автодополнение имён файлов (что -f отключает) не используется. А вот при sh -e поведение изменено: сделана маскировка кода возврата при ошибках (через ||:), т. к. требуется корректно отмонтировать примонтированное. > > (В ответ на комментарий №10) > > PS: Доработку пакетом, или патчем? > Обычно бывает удобней сперва отсмотреть и если что -- поправить (--amend), но > на глаз теперь почти порядок; единственно что не понял: зачем делать put-file > применительно к /etc/lvm/lvm.conf, оно же не должно быть бинарником -- или это > ты механически переделал на put-file и второе вхождение cp? :) Не, специально: 1. Мне нравиться как put-file обрабатывает симлинки -- при копировании структуры вида <симлинк> -> <файл> она воссоздаётся. 2. Можно выкинуть mkdir. 3. Однообразие. На самом деле, если положиться исключительно на put-file, то суть copy_chroot_binaries() можно сократить до: copy_chroot_binaries() { binddir="$(mktemp -d "$destdir/tmp/copy_chroot_binaries.XXXXXXXXX")" workdir="${binddir#$destdir}" mount --bind / "$binddir" chroot "$destdir" "$PUTFILE" "$workdir" "$MDADM" "$LVM" "$CRYPTSETUP" /etc/lvm/lvm.conf ||: umount "$binddir" rmdir "$binddir" } т. к. put-file практически все необходимые проверки выполняет. > > PPS: по результатам теста будто работает; на всякий обрати внимание -- > regular-server.iso у меня с таким пакетом встал (при этом > /mnt/destination/sbin/lvm и прочие нужные скрипту бинари были скопированы в > /sbin инсталятора и не имели выпадений по библиотекам, судя по ldd), но не смог > загрузиться с lvm-ного корня: initrd не выполнил vgchange -ay, после ручного > подъёма в lvm, монтирования в /root и exec /root/sbin/init установка > загрузилась. Для информации: у меня в системе поставленной с того инсталлятора, который я сейчас мучаю (пакет с предлагаемыми правками там всегда) LVM автоматом стартует. Но там у меня systemd (по ТЗ он там нужен). (В ответ на комментарий №13) > В смысле закидывай. OK. Закинул. Но после -- нашёл ляп... Прошу смотреть очередной фикс http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commitdiff;h=7c26c2ff301304cc0988590fad5990918be2d612. Там возвращается потерянная скобка: - if -n "$useputfile" ]; then + if [ -n "$useputfile" ]; then (В ответ на комментарий №14) ... > > Но после -- нашёл ляп... Прошу смотреть очередной фикс > http://git.altlinux.org/people/solo/packages/installer-scripts-remount-stage2.git?p=installer-scripts-remount-stage2.git;a=commitdiff;h=7c26c2ff301304cc0988590fad5990918be2d612. > Там возвращается потерянная скобка: > > - if -n "$useputfile" ]; then > + if [ -n "$useputfile" ]; then Прошу пропуска для installer-scripts-remount-stage2-0.5.4-alt1 (см. http://git.altlinux.org/tasks/151413/logs/events.1.1.log). (В ответ на комментарий №14) > > > Не понял, о добавлении какого -f идет речь. > > #!/bin/sh -f В смысле -e, конечно. > > ты механически переделал на put-file и второе вхождение cp? :) > Не, специально: А. > На самом деле, если положиться исключительно на put-file, то суть > copy_chroot_binaries() можно сократить до: У нас пока есть mkinitrd (хоть уже и не поддерживается, но в некоторых армовых образах задействовалось). > (В ответ на комментарий №13) > > В смысле закидывай. > OK. Закинул. Но после -- нашёл ляп... Прошу смотреть очередной фикс Что самое смешное, я его тоже проморгал, читая дифф. Закидывай, конечно. |
