On Wed, 2022-12-07 at 15:38 -0800, Darrick J. Wong wrote: > On Tue, Dec 06, 2022 at 06:23:46PM -0800, Catherine Hoang wrote: > > Adapt this tool to call xfs_io to retrieve the UUID of a mounted > > filesystem. > > This is a precursor to enabling xfs_admin to set the UUID of a > > mounted > > filesystem. > > > > Signed-off-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx> > > --- > > db/xfs_admin.sh | 21 ++++++++++++++++++--- > > 1 file changed, 18 insertions(+), 3 deletions(-) > > > > diff --git a/db/xfs_admin.sh b/db/xfs_admin.sh > > index 409975b2..0dcb9940 100755 > > --- a/db/xfs_admin.sh > > +++ b/db/xfs_admin.sh > > @@ -6,6 +6,8 @@ > > > > status=0 > > DB_OPTS="" > > +DB_EXTRA_OPTS="" > > +IO_OPTS="" > > This seemed oddly familiar until I remembered that we've been here > before: > https://lore.kernel.org/linux-xfs/ac736821-83de-4bde-a1a1-d0d2711932d7@xxxxxxxxxxx/ > > And now that I've reread that thread, I've now realized /why/ I gave > up > on adding things to this script -- there were too many questions from > the maintainer for which I couldn't come up with any satisfying > answer. > Then I burned out and gave up. > > Ofc now we have a new maintainer, so I'll put the questions to the > new > one. To summarize: > > What happens if there are multiple sources of truth because the fs is > mounted? Do we stop after processing the online options and ignore > the > offline ones? Do we keep going, even though -f is almost certainly > required? > > If the user specifies multiple options, is it ok to change the order > in > which we run them so that we can run xfs_io and then xfs_db? > > If it's not ok to change the order, how do we make the two tools run > in > lockstep so we only have to open the filesystem once? > > If it's not ok to change the order and we cannot do lockstep, is it > ok > to invoke io/db once for each subcommand instead of assembling a > giant > cli option array like we now for db? > > If we have to invoke io/db multiple times, what do we do if the state > changes between invocations (e.g. someone else mounts the block dev > or > unmounts the fs)? What happens if this all results in multiple > xfs_repair invocations? > > Can we prohibit people from running multiple subcommands? Even if > that's a breaking change for someone who might be relying on the > exact > behaviors of this shell script? > > What if, instead of trying to find answers to all these annoying > questions, we instead decide that either all the subcommands have to > target a mountpoint or they all have to target a blockdev, or > xfs_admin > will exit with an error code? > > --D So I applied Catherines patches, but it doesnt exactly have the same behaviors Eric commented on since this set doesnt have the extra label changes. Unless I missed some other behavioral issues with other flags that you are meaning to refer to? Otherwise I don't see that this introducing any odd behaviors. It's certainly not that the above questions are not valid, but I think it's reasonable for them to be addressed as a separate fix. As far as the scope of this set is concerned tho, I think what Catherine has here is reasonable. Reviewed-by: Allison Henderson <allison.henderson@xxxxxxxxxx> > > > REPAIR_OPTS="" > > REPAIR_DEV_OPTS="" > > LOG_OPTS="" > > @@ -23,7 +25,8 @@ do > > O) REPAIR_OPTS=$REPAIR_OPTS" -c $OPTARG";; > > p) DB_OPTS=$DB_OPTS" -c 'version projid32bit'";; > > r) REPAIR_DEV_OPTS=" -r '$OPTARG'";; > > - u) DB_OPTS=$DB_OPTS" -r -c uuid";; > > + u) DB_EXTRA_OPTS=$DB_EXTRA_OPTS" -r -c uuid"; > > + IO_OPTS=$IO_OPTS" -r -c fsuuid";; > > U) DB_OPTS=$DB_OPTS" -c 'uuid "$OPTARG"'";; > > V) xfs_db -p xfs_admin -V > > status=$? > > @@ -38,14 +41,26 @@ set -- extra $@ > > shift $OPTIND > > case $# in > > 1|2) > > + # Use xfs_io if mounted and xfs_db if not mounted > > + if [ -n "$(findmnt -t xfs -T $1)" ]; then > > + DB_EXTRA_OPTS="" > > + else > > + IO_OPTS="" > > + fi > > + > > # Pick up the log device, if present > > if [ -n "$2" ]; then > > LOG_OPTS=" -l '$2'" > > fi > > > > - if [ -n "$DB_OPTS" ] > > + if [ -n "$DB_OPTS" ] || [ -n "$DB_EXTRA_OPTS" ] > > + then > > + eval xfs_db -x -p xfs_admin $LOG_OPTS > > $DB_OPTS $DB_EXTRA_OPTS "$1" > > + status=$? > > + fi > > + if [ -n "$IO_OPTS" ] > > then > > - eval xfs_db -x -p xfs_admin $LOG_OPTS > > $DB_OPTS "$1" > > + eval xfs_io -x -p xfs_admin $IO_OPTS "$1" > > status=$? > > fi > > if [ -n "$REPAIR_OPTS" ] > > -- > > 2.25.1 > >
