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@xxxxxxxxxx> 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@xxxxxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxx>
> >
> > --D
> >
> > > esac
> > > -if [ -n "$PS1" -o -t 0 ]; then
> > > +if [ -t 0 ]; then
> > > FORCE=false
> > > fi
> > >
> > > --
> > > 2.43.0
> > >
> > >
>
