Re: [PATCH v6 7/7] xfs: add error injection for per-AG resv failure when shrinkfs

[Gao Xiang <hsiangkao@redhat.com>]

Hi Brian,

On Wed, Feb 03, 2021 at 09:23:59AM -0500, Brian Foster wrote:
> On Tue, Jan 26, 2021 at 08:56:21PM +0800, Gao Xiang wrote:
> > per-AG resv failure after fixing up freespace is hard to test in an
> > effective way, so directly add an error injection path to observe
> > such error handling path works as expected.
> > 
> > Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> > Signed-off-by: Gao Xiang <hsiangkao@redhat.com>
> > ---
> >  fs/xfs/libxfs/xfs_ag.c       | 5 +++++
> >  fs/xfs/libxfs/xfs_errortag.h | 4 +++-
> >  fs/xfs/xfs_error.c           | 2 ++
> >  3 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
> > index c6e68e265269..5076913c153f 100644
> > --- a/fs/xfs/libxfs/xfs_ag.c
> > +++ b/fs/xfs/libxfs/xfs_ag.c
> > @@ -23,6 +23,7 @@
> >  #include "xfs_ag_resv.h"
> >  #include "xfs_health.h"
> >  #include "xfs_error.h"
> > +#include "xfs_errortag.h"
> >  #include "xfs_bmap.h"
> >  #include "xfs_defer.h"
> >  #include "xfs_log_format.h"
> > @@ -559,6 +560,10 @@ xfs_ag_shrink_space(
> >  	be32_add_cpu(&agf->agf_length, -len);
> >  
> >  	err2 = xfs_ag_resv_init(agibp->b_pag, *tpp);
> > +
> > +	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_SHRINKFS_AG_RESV_FAIL))
> > +		err2 = -ENOSPC;
> > +
> 
> Seems reasonable, but I feel like this could be broadened to serve as a
> generic perag reservation error tag. I suppose we might not be able to
> use it on a clean mount, but perhaps it could be reused for growfs and
> remount. Hm?

I think it could be done in that way, yet currently the logic is just to
verify the shrink error handling case above rather than extend to actually
error inject per-AG reservation for now... I could rename the errortag
for later reuse (some better naming? I'm not good at this...) in advance
yet real per-AG reservation error injection might be more complicated
than just error out with -ENOSPC, and it's somewhat out of scope of this
patchset for now...

Thanks,
Gao Xiang

> 
> Brian