On Tue, Dec 13, 2022 at 11:27:25AM -0800, Darrick J. Wong wrote:
> On Tue, Dec 13, 2022 at 08:08:31PM +0100, David Disseldorp wrote:
> > On Wed, 14 Dec 2022 02:25:31 +0800, Zorro Lang wrote:
> >
> > > We should let xfsprogs decide how xfs_info works. xfs_info doesn't always
> > > depends on xfs_spaceman, old xfs_info might use xfs_db or other things to
> > > get xfs info, and I don't know if it'll be changed to use other command in
> > > the future again.
> > >
> > > So this change will cause all cases which call _require_acl_get_max will notrun
> > > on old (maybe future) xfsprogs which doesn't has xfs_spaceman "info" command.
> > > I think that's not what we want, so I'd like to drop this patch. Thanks for
> > > your understanding.
> >
> > I wasn't aware that the xfs_info->xfs_spaceman call path was a new
> > change. With that in mind, I agree it makes sense to drop this patch.
>
> Since 4.17, xfs_info requires xfs_spaceman if the fs is mounted, or
> xfs_db if unmounted. Before that, it required xfs_growfs and only
> worked on mounted filesystems.
>
> That said, perhaps _acl_get_max should be doing:
>
> $XFS_INFO_PROG $TEST_DIR | _filter_mkfs > /dev/null 2> $tmp.info
> test "${PIPESTATUS[0]}" -ne 0 && \
> _fail "cannot get maximum acl count for xfs"
>
> Rather than spraying whatever nonsense it does if spaceman is missing?
Make more sense, but I don't think it's necessary to check return status
of each command line. xfs_info generally works well, we can do this change
when we have a real requirement in one day :)
Thanks,
Zorro
>
> --D
>
> > Cheers, David
>
