| Message ID | 20240813072815.1655916-1-gerald.yang@canonical.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | fsck.xfs: fix fsck.xfs run by different shells when fsck.mode=force is set | expand |
On Tue, Aug 13, 2024 at 03:25:51PM +0800, Gerald Yang wrote: > When fsck.mode=force is specified in the kernel command line, fsck.xfs > is executed during the boot process. However, when the default shell is > not bash, $PS1 should be a different value, consider the following script: > cat ps1.sh > echo "$PS1" > > run ps1.sh with different shells: > ash ./ps1.sh > $ > bash ./ps1.sh > > dash ./ps1.sh > $ > ksh ./ps1.sh > > zsh ./ps1.sh > > On systems like Ubuntu, where dash is the default shell during the boot > process to improve startup speed. This results in FORCE being incorrectly > set to false and then xfs_repair is not invoked: > if [ -n "$PS1" -o -t 0 ]; then > FORCE=false > fi > > Other distros may encounter this issue too if the default shell is set > to anoother shell. > > Check "-t 0" is enough to determine if we are in interactive mode, and > xfs_repair is invoked as expected regardless of the shell used. > > Fixes: 04a2d5dc ("fsck.xfs: allow forced repairs using xfs_repair") > Signed-off-by: Gerald Yang <gerald.yang@canonical.com> > --- > fsck/xfs_fsck.sh | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fsck/xfs_fsck.sh b/fsck/xfs_fsck.sh > index 62a1e0b3..19ada9a7 100755 > --- a/fsck/xfs_fsck.sh > +++ b/fsck/xfs_fsck.sh > @@ -55,12 +55,12 @@ fi > # directly. > # > # Use multiple methods to capture most of the cases: > -# The case for *i* and -n "$PS1" are commonly suggested in bash manual > +# The case for *i* is commonly suggested in bash manual > # and the -t 0 test checks stdin > case $- in > *i*) FORCE=false ;; I can't remember why we allow any argument with the letter 'i' in it to derail an xfs_repair -f invocation?? Regardless, the bits you changed look correct so Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > esac > -if [ -n "$PS1" -o -t 0 ]; then > +if [ -t 0 ]; then > FORCE=false > fi > > -- > 2.43.0 > >
Thank you Darrick for the review I just would like to ask if this will be merged into for-next soon? On Tue, Aug 13, 2024 at 10:53 PM Darrick J. Wong <djwong@kernel.org> wrote: > > On Tue, Aug 13, 2024 at 03:25:51PM +0800, Gerald Yang wrote: > > When fsck.mode=force is specified in the kernel command line, fsck.xfs > > is executed during the boot process. However, when the default shell is > > not bash, $PS1 should be a different value, consider the following script: > > cat ps1.sh > > echo "$PS1" > > > > run ps1.sh with different shells: > > ash ./ps1.sh > > $ > > bash ./ps1.sh > > > > dash ./ps1.sh > > $ > > ksh ./ps1.sh > > > > zsh ./ps1.sh > > > > On systems like Ubuntu, where dash is the default shell during the boot > > process to improve startup speed. This results in FORCE being incorrectly > > set to false and then xfs_repair is not invoked: > > if [ -n "$PS1" -o -t 0 ]; then > > FORCE=false > > fi > > > > Other distros may encounter this issue too if the default shell is set > > to anoother shell. > > > > Check "-t 0" is enough to determine if we are in interactive mode, and > > xfs_repair is invoked as expected regardless of the shell used. > > > > Fixes: 04a2d5dc ("fsck.xfs: allow forced repairs using xfs_repair") > > Signed-off-by: Gerald Yang <gerald.yang@canonical.com> > > --- > > fsck/xfs_fsck.sh | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/fsck/xfs_fsck.sh b/fsck/xfs_fsck.sh > > index 62a1e0b3..19ada9a7 100755 > > --- a/fsck/xfs_fsck.sh > > +++ b/fsck/xfs_fsck.sh > > @@ -55,12 +55,12 @@ fi > > # directly. > > # > > # Use multiple methods to capture most of the cases: > > -# The case for *i* and -n "$PS1" are commonly suggested in bash manual > > +# The case for *i* is commonly suggested in bash manual > > # and the -t 0 test checks stdin > > case $- in > > *i*) FORCE=false ;; > > I can't remember why we allow any argument with the letter 'i' in it to > derail an xfs_repair -f invocation?? > > Regardless, the bits you changed look correct so > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > --D > > > esac > > -if [ -n "$PS1" -o -t 0 ]; then > > +if [ -t 0 ]; then > > FORCE=false > > fi > > > > -- > > 2.43.0 > > > >
On Fri, Sep 06, 2024 at 11:44:37AM GMT, Gerald Yang wrote: > Thank you Darrick for the review > I just would like to ask if this will be merged into for-next soon? This is already in my queue, but got delayed due the problems with C++ compiler, hopefully some day next week it should hit for-next. Carlos > > > On Tue, Aug 13, 2024 at 10:53 PM Darrick J. Wong <djwong@kernel.org> wrote: > > > > On Tue, Aug 13, 2024 at 03:25:51PM +0800, Gerald Yang wrote: > > > When fsck.mode=force is specified in the kernel command line, fsck.xfs > > > is executed during the boot process. However, when the default shell is > > > not bash, $PS1 should be a different value, consider the following script: > > > cat ps1.sh > > > echo "$PS1" > > > > > > run ps1.sh with different shells: > > > ash ./ps1.sh > > > $ > > > bash ./ps1.sh > > > > > > dash ./ps1.sh > > > $ > > > ksh ./ps1.sh > > > > > > zsh ./ps1.sh > > > > > > On systems like Ubuntu, where dash is the default shell during the boot > > > process to improve startup speed. This results in FORCE being incorrectly > > > set to false and then xfs_repair is not invoked: > > > if [ -n "$PS1" -o -t 0 ]; then > > > FORCE=false > > > fi > > > > > > Other distros may encounter this issue too if the default shell is set > > > to anoother shell. > > > > > > Check "-t 0" is enough to determine if we are in interactive mode, and > > > xfs_repair is invoked as expected regardless of the shell used. > > > > > > Fixes: 04a2d5dc ("fsck.xfs: allow forced repairs using xfs_repair") > > > Signed-off-by: Gerald Yang <gerald.yang@canonical.com> > > > --- > > > fsck/xfs_fsck.sh | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/fsck/xfs_fsck.sh b/fsck/xfs_fsck.sh > > > index 62a1e0b3..19ada9a7 100755 > > > --- a/fsck/xfs_fsck.sh > > > +++ b/fsck/xfs_fsck.sh > > > @@ -55,12 +55,12 @@ fi > > > # directly. > > > # > > > # Use multiple methods to capture most of the cases: > > > -# The case for *i* and -n "$PS1" are commonly suggested in bash manual > > > +# The case for *i* is commonly suggested in bash manual > > > # and the -t 0 test checks stdin > > > case $- in > > > *i*) FORCE=false ;; > > > > I can't remember why we allow any argument with the letter 'i' in it to > > derail an xfs_repair -f invocation?? > > > > Regardless, the bits you changed look correct so > > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > > > --D > > > > > esac > > > -if [ -n "$PS1" -o -t 0 ]; then > > > +if [ -t 0 ]; then > > > FORCE=false > > > fi > > > > > > -- > > > 2.43.0 > > > > > > >
diff --git a/fsck/xfs_fsck.sh b/fsck/xfs_fsck.sh index 62a1e0b3..19ada9a7 100755 --- a/fsck/xfs_fsck.sh +++ b/fsck/xfs_fsck.sh @@ -55,12 +55,12 @@ fi # directly. # # Use multiple methods to capture most of the cases: -# The case for *i* and -n "$PS1" are commonly suggested in bash manual +# The case for *i* is commonly suggested in bash manual # and the -t 0 test checks stdin case $- in *i*) FORCE=false ;; esac -if [ -n "$PS1" -o -t 0 ]; then +if [ -t 0 ]; then FORCE=false fi
When fsck.mode=force is specified in the kernel command line, fsck.xfs is executed during the boot process. However, when the default shell is not bash, $PS1 should be a different value, consider the following script: cat ps1.sh echo "$PS1" run ps1.sh with different shells: ash ./ps1.sh $ bash ./ps1.sh dash ./ps1.sh $ ksh ./ps1.sh zsh ./ps1.sh On systems like Ubuntu, where dash is the default shell during the boot process to improve startup speed. This results in FORCE being incorrectly set to false and then xfs_repair is not invoked: if [ -n "$PS1" -o -t 0 ]; then FORCE=false fi Other distros may encounter this issue too if the default shell is set to anoother shell. Check "-t 0" is enough to determine if we are in interactive mode, and xfs_repair is invoked as expected regardless of the shell used. Fixes: 04a2d5dc ("fsck.xfs: allow forced repairs using xfs_repair") Signed-off-by: Gerald Yang <gerald.yang@canonical.com> --- fsck/xfs_fsck.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)