| Message ID | 20250318061300.12805-4-ubely@ilbers.de |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Fixes for unattended installation | expand |
On 18.03.25 07:10, Uladzimir Bely wrote: > This fixes race between two unattended installer instances running > on serial "ttyS0" and graphic "tty1" terminals. > > While one of them starts writing the disk, another one fails > and schedules reboot in 60 seconds. Depending on build machine > performance we can get incomplete installation and broken target > filesystem. > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de> > --- > .../deploy-image/files/usr/bin/deploy-image-wic.sh | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh > index 7f552eee..bd580694 100755 > --- a/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh > +++ b/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh > @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f -- "$0"; )"; ) > > . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh > > +if $installer_unattended; then > + if [ "$(tty)" != "/dev/ttyS0" ]; then This is wrong. "ttyS0" is target-specific. We need a different, generic mechanism to detect multiple executions. Jan > + echo "Disable unattended mode on $(tty), it's active on /dev/ttyS0" > + installer_unattended=0 > + fi > +fi > > if ! $installer_unattended; then > installer_image_uri=$(find "$installdata" -type f -iname "*.wic*" -a -not -iname "*.wic.bmap" -exec basename {} \;)
On 18.03.25 07:25, 'Jan Kiszka' via isar-users wrote: > On 18.03.25 07:10, Uladzimir Bely wrote: >> This fixes race between two unattended installer instances running >> on serial "ttyS0" and graphic "tty1" terminals. >> >> While one of them starts writing the disk, another one fails >> and schedules reboot in 60 seconds. Depending on build machine >> performance we can get incomplete installation and broken target >> filesystem. >> >> Signed-off-by: Uladzimir Bely <ubely@ilbers.de> >> --- >> .../deploy-image/files/usr/bin/deploy-image-wic.sh | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh >> index 7f552eee..bd580694 100755 >> --- a/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh >> +++ b/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh >> @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f -- "$0"; )"; ) >> >> . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh >> >> +if $installer_unattended; then >> + if [ "$(tty)" != "/dev/ttyS0" ]; then > > This is wrong. "ttyS0" is target-specific. We need a different, generic > mechanism to detect multiple executions. > > Jan > >> + echo "Disable unattended mode on $(tty), it's active on /dev/ttyS0" >> + installer_unattended=0 And this would be also wrong. But all this does not make sense yet. We have a single service that is supposed to run a single script. I don't see why systemd should instantiate the service multiple times. Is our service file incorrect? Jan >> + fi >> +fi >> >> if ! $installer_unattended; then >> installer_image_uri=$(find "$installdata" -type f -iname "*.wic*" -a -not -iname "*.wic.bmap" -exec basename {} \;) > >
On Tue, 2025-03-18 at 07:29 +0100, Jan Kiszka wrote: > On 18.03.25 07:25, 'Jan Kiszka' via isar-users wrote: > > On 18.03.25 07:10, Uladzimir Bely wrote: > > > This fixes race between two unattended installer instances > > > running > > > on serial "ttyS0" and graphic "tty1" terminals. > > > > > > While one of them starts writing the disk, another one fails > > > and schedules reboot in 60 seconds. Depending on build machine > > > performance we can get incomplete installation and broken target > > > filesystem. > > > > > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de> > > > --- > > > .../deploy-image/files/usr/bin/deploy-image-wic.sh | 6 > > > ++++++ > > > 1 file changed, 6 insertions(+) > > > > > > diff --git a/meta-isar/recipes-installer/deploy- > > > image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes- > > > installer/deploy-image/files/usr/bin/deploy-image-wic.sh > > > index 7f552eee..bd580694 100755 > > > --- a/meta-isar/recipes-installer/deploy- > > > image/files/usr/bin/deploy-image-wic.sh > > > +++ b/meta-isar/recipes-installer/deploy- > > > image/files/usr/bin/deploy-image-wic.sh > > > @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f -- > > > "$0"; )"; ) > > > > > > . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh > > > > > > +if $installer_unattended; then > > > + if [ "$(tty)" != "/dev/ttyS0" ]; then > > > > This is wrong. "ttyS0" is target-specific. We need a different, > > generic > > mechanism to detect multiple executions. > > > > Jan > > > > > + echo "Disable unattended mode on $(tty), it's active on > > > /dev/ttyS0" > > > + installer_unattended=0 > > And this would be also wrong. > > But all this does not make sense yet. We have a single service that > is > supposed to run a single script. I don't see why systemd should > instantiate the service multiple times. Is our service file > incorrect? > The service itself is correct, the problem is that it's run twice. https://github.com/ilbers/isar/blob/master/meta-isar/recipes-installer/deploy-image-service/deploy-image-service.bb#L19-L20 For non-unattended mode it doesn't matter, but in unattended mode one of instances fails and schedules rebooting. > Jan > > > > + fi > > > +fi > > > > > > if ! $installer_unattended; then > > > installer_image_uri=$(find "$installdata" -type f -iname > > > "*.wic*" -a -not -iname "*.wic.bmap" -exec basename {} \;) > > > > >
On Tue, 2025-03-18 at 07:25 +0100, Jan Kiszka wrote: > On 18.03.25 07:10, Uladzimir Bely wrote: > > This fixes race between two unattended installer instances running > > on serial "ttyS0" and graphic "tty1" terminals. > > > > While one of them starts writing the disk, another one fails > > and schedules reboot in 60 seconds. Depending on build machine > > performance we can get incomplete installation and broken target > > filesystem. > > > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de> > > --- > > .../deploy-image/files/usr/bin/deploy-image-wic.sh | 6 > > ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/meta-isar/recipes-installer/deploy- > > image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes- > > installer/deploy-image/files/usr/bin/deploy-image-wic.sh > > index 7f552eee..bd580694 100755 > > --- a/meta-isar/recipes-installer/deploy- > > image/files/usr/bin/deploy-image-wic.sh > > +++ b/meta-isar/recipes-installer/deploy- > > image/files/usr/bin/deploy-image-wic.sh > > @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f -- > > "$0"; )"; ) > > > > . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh > > > > +if $installer_unattended; then > > + if [ "$(tty)" != "/dev/ttyS0" ]; then > > This is wrong. "ttyS0" is target-specific. We need a different, > generic > mechanism to detect multiple executions. > > Jan I agree. But this is also hardcoded in the service recipe, so this generic mechanism should also make change in it. I think, it could be some "main" tty variable defined in the recipe and some additional ttys to run always in "attended" mode. > > > + echo "Disable unattended mode on $(tty), it's active on > > /dev/ttyS0" > > + installer_unattended=0 > > + fi > > +fi > > > > if ! $installer_unattended; then > > installer_image_uri=$(find "$installdata" -type f -iname > > "*.wic*" -a -not -iname "*.wic.bmap" -exec basename {} \;) > >
On 18.03.25 07:33, Uladzimir Bely wrote: > On Tue, 2025-03-18 at 07:29 +0100, Jan Kiszka wrote: >> On 18.03.25 07:25, 'Jan Kiszka' via isar-users wrote: >>> On 18.03.25 07:10, Uladzimir Bely wrote: >>>> This fixes race between two unattended installer instances >>>> running >>>> on serial "ttyS0" and graphic "tty1" terminals. >>>> >>>> While one of them starts writing the disk, another one fails >>>> and schedules reboot in 60 seconds. Depending on build machine >>>> performance we can get incomplete installation and broken target >>>> filesystem. >>>> >>>> Signed-off-by: Uladzimir Bely <ubely@ilbers.de> >>>> --- >>>> .../deploy-image/files/usr/bin/deploy-image-wic.sh | 6 >>>> ++++++ >>>> 1 file changed, 6 insertions(+) >>>> >>>> diff --git a/meta-isar/recipes-installer/deploy- >>>> image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes- >>>> installer/deploy-image/files/usr/bin/deploy-image-wic.sh >>>> index 7f552eee..bd580694 100755 >>>> --- a/meta-isar/recipes-installer/deploy- >>>> image/files/usr/bin/deploy-image-wic.sh >>>> +++ b/meta-isar/recipes-installer/deploy- >>>> image/files/usr/bin/deploy-image-wic.sh >>>> @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f -- >>>> "$0"; )"; ) >>>> >>>> . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh >>>> >>>> +if $installer_unattended; then >>>> + if [ "$(tty)" != "/dev/ttyS0" ]; then >>> >>> This is wrong. "ttyS0" is target-specific. We need a different, >>> generic >>> mechanism to detect multiple executions. >>> >>> Jan >>> >>>> + echo "Disable unattended mode on $(tty), it's active on >>>> /dev/ttyS0" >>>> + installer_unattended=0 >> >> And this would be also wrong. >> >> But all this does not make sense yet. We have a single service that >> is >> supposed to run a single script. I don't see why systemd should >> instantiate the service multiple times. Is our service file >> incorrect? >> > > The service itself is correct, the problem is that it's run twice. > > https://github.com/ilbers/isar/blob/master/meta-isar/recipes-installer/deploy-image-service/deploy-image-service.bb#L19-L20 > > For non-unattended mode it doesn't matter, but in unattended mode one > of instances fails and schedules rebooting. > Ah, now I remember. But, again, we need a proper fix here. Re-entrance needs to be detected, the core copy job needs to be run only once. And all other entrances of the script skip over the copying core or otherwise wait at the same dialog that semi-unattended modes may show. Jan
On 18.03.25 07:37, Uladzimir Bely wrote: > On Tue, 2025-03-18 at 07:25 +0100, Jan Kiszka wrote: >> On 18.03.25 07:10, Uladzimir Bely wrote: >>> This fixes race between two unattended installer instances running >>> on serial "ttyS0" and graphic "tty1" terminals. >>> >>> While one of them starts writing the disk, another one fails >>> and schedules reboot in 60 seconds. Depending on build machine >>> performance we can get incomplete installation and broken target >>> filesystem. >>> >>> Signed-off-by: Uladzimir Bely <ubely@ilbers.de> >>> --- >>> .../deploy-image/files/usr/bin/deploy-image-wic.sh | 6 >>> ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/meta-isar/recipes-installer/deploy- >>> image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes- >>> installer/deploy-image/files/usr/bin/deploy-image-wic.sh >>> index 7f552eee..bd580694 100755 >>> --- a/meta-isar/recipes-installer/deploy- >>> image/files/usr/bin/deploy-image-wic.sh >>> +++ b/meta-isar/recipes-installer/deploy- >>> image/files/usr/bin/deploy-image-wic.sh >>> @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f -- >>> "$0"; )"; ) >>> >>> . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh >>> >>> +if $installer_unattended; then >>> + if [ "$(tty)" != "/dev/ttyS0" ]; then >> >> This is wrong. "ttyS0" is target-specific. We need a different, >> generic >> mechanism to detect multiple executions. >> >> Jan > > I agree. But this is also hardcoded in the service recipe, so this > generic mechanism should also make change in it. > That's no excuse for hard-coding that in the script as well. Jan > I think, it could be some "main" tty variable defined in the recipe and > some additional ttys to run always in "attended" mode. > >> >>> + echo "Disable unattended mode on $(tty), it's active on >>> /dev/ttyS0" >>> + installer_unattended=0 >>> + fi >>> +fi >>> >>> if ! $installer_unattended; then >>> installer_image_uri=$(find "$installdata" -type f -iname >>> "*.wic*" -a -not -iname "*.wic.bmap" -exec basename {} \;) >> >> >
On Tue, 2025-03-18 at 08:08 +0100, Jan Kiszka wrote: > On 18.03.25 07:33, Uladzimir Bely wrote: > > On Tue, 2025-03-18 at 07:29 +0100, Jan Kiszka wrote: > > > On 18.03.25 07:25, 'Jan Kiszka' via isar-users wrote: > > > > On 18.03.25 07:10, Uladzimir Bely wrote: > > > > > This fixes race between two unattended installer instances > > > > > running > > > > > on serial "ttyS0" and graphic "tty1" terminals. > > > > > > > > > > While one of them starts writing the disk, another one fails > > > > > and schedules reboot in 60 seconds. Depending on build > > > > > machine > > > > > performance we can get incomplete installation and broken > > > > > target > > > > > filesystem. > > > > > > > > > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de> > > > > > --- > > > > > .../deploy-image/files/usr/bin/deploy-image-wic.sh > > > > > | 6 > > > > > ++++++ > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > diff --git a/meta-isar/recipes-installer/deploy- > > > > > image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes- > > > > > installer/deploy-image/files/usr/bin/deploy-image-wic.sh > > > > > index 7f552eee..bd580694 100755 > > > > > --- a/meta-isar/recipes-installer/deploy- > > > > > image/files/usr/bin/deploy-image-wic.sh > > > > > +++ b/meta-isar/recipes-installer/deploy- > > > > > image/files/usr/bin/deploy-image-wic.sh > > > > > @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f - > > > > > - > > > > > "$0"; )"; ) > > > > > > > > > > . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh > > > > > > > > > > +if $installer_unattended; then > > > > > + if [ "$(tty)" != "/dev/ttyS0" ]; then > > > > > > > > This is wrong. "ttyS0" is target-specific. We need a different, > > > > generic > > > > mechanism to detect multiple executions. > > > > > > > > Jan > > > > > > > > > + echo "Disable unattended mode on $(tty), it's active > > > > > on > > > > > /dev/ttyS0" > > > > > + installer_unattended=0 > > > > > > And this would be also wrong. > > > > > > But all this does not make sense yet. We have a single service > > > that > > > is > > > supposed to run a single script. I don't see why systemd should > > > instantiate the service multiple times. Is our service file > > > incorrect? > > > > > > > The service itself is correct, the problem is that it's run twice. > > > > https://github.com/ilbers/isar/blob/master/meta-isar/recipes-installer/deploy-image-service/deploy-image-service.bb#L19-L20 > > > > For non-unattended mode it doesn't matter, but in unattended mode > > one > > of instances fails and schedules rebooting. > > > > Ah, now I remember. > > But, again, we need a proper fix here. Re-entrance needs to be > detected, > the core copy job needs to be run only once. And all other entrances > of > the script skip over the copying core or otherwise wait at the same > dialog that semi-unattended modes may show. > > Jan > OK, I see. So, there are two ways to deal with the issue: 1. Make one of terminals (for example, first virtual console ttyS1) main one, e.g.: INSTALLER_UNATTENDED_CONSOLE ?= "ttyS1" And refer to this variable instead of hardcoded value. 2. Unattended installation will be done by the service which was the first one in the race (first who run bmaptool and locked e.g. /dev/sda). Other services just should not trigger reboot in this case, that means "reboot" should be moved from service unit to script again. As for me, I would prefer option 1 since it's more predictable.
On Tue, 2025-03-18 at 10:51 +0300, Uladzimir Bely wrote: > On Tue, 2025-03-18 at 08:08 +0100, Jan Kiszka wrote: > > On 18.03.25 07:33, Uladzimir Bely wrote: > > > On Tue, 2025-03-18 at 07:29 +0100, Jan Kiszka wrote: > > > > On 18.03.25 07:25, 'Jan Kiszka' via isar-users wrote: > > > > > On 18.03.25 07:10, Uladzimir Bely wrote: > > > > > > This fixes race between two unattended installer instances > > > > > > running > > > > > > on serial "ttyS0" and graphic "tty1" terminals. > > > > > > > > > > > > While one of them starts writing the disk, another one > > > > > > fails > > > > > > and schedules reboot in 60 seconds. Depending on build > > > > > > machine > > > > > > performance we can get incomplete installation and broken > > > > > > target > > > > > > filesystem. > > > > > > > > > > > > Signed-off-by: Uladzimir Bely <ubely@ilbers.de> > > > > > > --- > > > > > > .../deploy-image/files/usr/bin/deploy-image- > > > > > > wic.sh > > > > > > > 6 > > > > > > ++++++ > > > > > > 1 file changed, 6 insertions(+) > > > > > > > > > > > > diff --git a/meta-isar/recipes-installer/deploy- > > > > > > image/files/usr/bin/deploy-image-wic.sh b/meta- > > > > > > isar/recipes- > > > > > > installer/deploy-image/files/usr/bin/deploy-image-wic.sh > > > > > > index 7f552eee..bd580694 100755 > > > > > > --- a/meta-isar/recipes-installer/deploy- > > > > > > image/files/usr/bin/deploy-image-wic.sh > > > > > > +++ b/meta-isar/recipes-installer/deploy- > > > > > > image/files/usr/bin/deploy-image-wic.sh > > > > > > @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f > > > > > > - > > > > > > - > > > > > > "$0"; )"; ) > > > > > > > > > > > > . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh > > > > > > > > > > > > +if $installer_unattended; then > > > > > > + if [ "$(tty)" != "/dev/ttyS0" ]; then > > > > > > > > > > This is wrong. "ttyS0" is target-specific. We need a > > > > > different, > > > > > generic > > > > > mechanism to detect multiple executions. > > > > > > > > > > Jan > > > > > > > > > > > + echo "Disable unattended mode on $(tty), it's > > > > > > active > > > > > > on > > > > > > /dev/ttyS0" > > > > > > + installer_unattended=0 > > > > > > > > And this would be also wrong. > > > > > > > > But all this does not make sense yet. We have a single service > > > > that > > > > is > > > > supposed to run a single script. I don't see why systemd should > > > > instantiate the service multiple times. Is our service file > > > > incorrect? > > > > > > > > > > The service itself is correct, the problem is that it's run > > > twice. > > > > > > https://github.com/ilbers/isar/blob/master/meta-isar/recipes-installer/deploy-image-service/deploy-image-service.bb#L19-L20 > > > > > > For non-unattended mode it doesn't matter, but in unattended mode > > > one > > > of instances fails and schedules rebooting. > > > > > > > Ah, now I remember. > > > > But, again, we need a proper fix here. Re-entrance needs to be > > detected, > > the core copy job needs to be run only once. And all other > > entrances > > of > > the script skip over the copying core or otherwise wait at the same > > dialog that semi-unattended modes may show. > > > > Jan > > > > OK, I see. > > So, there are two ways to deal with the issue: > > 1. Make one of terminals (for example, first virtual console ttyS1) > main one, e.g.: > > INSTALLER_UNATTENDED_CONSOLE ?= "ttyS1" > > And refer to this variable instead of hardcoded value. ...Actually, we could simply check for MACHINE_SERIAL here without adding new variable. > > 2. Unattended installation will be done by the service which was the > first one in the race (first who run bmaptool and locked e.g. > /dev/sda). Other services just should not trigger reboot in this > case, > that means "reboot" should be moved from service unit to script > again. > > As for me, I would prefer option 1 since it's more predictable. > > > -- > Best regards, > Uladzimir. >
diff --git a/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh b/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh index 7f552eee..bd580694 100755 --- a/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh +++ b/meta-isar/recipes-installer/deploy-image/files/usr/bin/deploy-image-wic.sh @@ -10,6 +10,12 @@ SCRIPT_DIR=$( dirname -- "$( readlink -f -- "$0"; )"; ) . ${SCRIPT_DIR}/../lib/deploy-image-wic/handle-config.sh +if $installer_unattended; then + if [ "$(tty)" != "/dev/ttyS0" ]; then + echo "Disable unattended mode on $(tty), it's active on /dev/ttyS0" + installer_unattended=0 + fi +fi if ! $installer_unattended; then installer_image_uri=$(find "$installdata" -type f -iname "*.wic*" -a -not -iname "*.wic.bmap" -exec basename {} \;)
This fixes race between two unattended installer instances running on serial "ttyS0" and graphic "tty1" terminals. While one of them starts writing the disk, another one fails and schedules reboot in 60 seconds. Depending on build machine performance we can get incomplete installation and broken target filesystem. Signed-off-by: Uladzimir Bely <ubely@ilbers.de> --- .../deploy-image/files/usr/bin/deploy-image-wic.sh | 6 ++++++ 1 file changed, 6 insertions(+)