[PATCH 0/6] xfsprogs-4.20: various fixes

[PATCH 0/6] xfsprogs-4.20: various fixes

[Darrick J. Wong]

Here are some patches for xfsprogs 4.20.

The first patch splits up the commands documented in the xfs_io manpage
into more categories than "file i/o", "mmap i/o", and "everything else".

The second patch (re)fixes a bug in xfs_repair where it would assert if
the first record in a directory da btree block pointed to block 0.

The third patch fixes a problem where we try to allocate a transaction
with an unnecessary block reservation in order to fix the AGFL.  AGFL
blocks are counted in fdblocks, and at that point we haven't written out
the new fdblocks anyway.

The fourth patch fixes xfs_scrub's bulkstat code to handle the case of
an inode chunk with all 64 inodes free, which can happen when more than
64 inodes can be stored in a fs block.

The fifth patch fixes scrub's phase 7 reporting so that we don't print
single-unit numbers ("3.0 inodes") with fractional components.

The sixth patch moves xfs_scrub and xfs_scrub_all to /usr/sbin because
they both depend on components that live in /usr.

--D

[PATCH 1/6] xfs_io.8: rearrange command listings by section

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Most of the commands listed under "OTHER COMMANDS" apply to files or
filesystems.  Create two new sections for that and populate them
appropriately.

Here's what moves:

fsmap: moves from file io commands to filesystem commands
utimes: moves from file io commands to file commands

>From the OTHER COMMANDS section:

lsattr/chattr: moves to file commands
flink: moves to file commands
stat/statx: moves to file commands
lsproj/chproj: moves to file commands
parent: moves to file commands
[gs]et_encpolicy: moves to file io commands
freeze/thaw: move to filesystem commands
inject: move to filesystem commands
resblks: move to filesystem commands
shutdown: move to filesystem commands
statfs: move to filesystem commands
label: move to filesystem commands

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 man/man8/xfs_io.8 |  446 +++++++++++++++++++++++++++--------------------------
 1 file changed, 226 insertions(+), 220 deletions(-)


diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index f1099c32..938aa0c2 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -338,108 +338,6 @@ or ends in a hole,
 will print the hole, truncated to the requested range.
 .RE
 .TP
-.BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ]
-Prints the mapping of disk blocks used by the filesystem hosting the current
-file.  The map lists each extent used by files, allocation group metadata,
-journalling logs, and static filesystem metadata, as well as any
-regions that are unused.
-Each line of the listings takes the following form:
-.PP
-.RS
-.IR extent ": " major ":" minor " [" startblock .. endblock "]: " owner " " startoffset .. endoffset " " length
-.PP
-Static filesystem metadata, allocation group metadata, btrees,
-journalling logs, and free space are marked by replacing the
-.IR startoffset .. endoffset
-with the appropriate marker.
-All blocks, offsets, and lengths are specified in units of 512-byte
-blocks, no matter what the filesystem's block size is.
-The optional
-.I start
-and
-.I end
-arguments can be used to constrain the output to a particular range of
-disk blocks.
-If these two options are specified, exactly one of
-.BR "-d" ", " "-l" ", or " "-r"
-must also be set.
-.RE
-.RS 1.0i
-.PD 0
-.TP
-.BI \-d
-Display only extents from the data device.
-This option only applies for XFS filesystems.
-.TP
-.BI \-l
-Display only extents from the external log device.
-This option only applies to XFS filesystems.
-.TP
-.BI \-r
-Display only extents from the realtime device.
-This option only applies to XFS filesystems.
-.TP
-.BI \-m
-Display results in a machine readable format (CSV).
-This option is not compatible with the
-.B \-v
-flag.
-The columns of the output are: extent number, device major, device minor,
-physical start, physical end, owner, offset start, offset end, length.
-The start, end, and length numbers are provided in units of 512b.
-The owner field is a special string that takes the form:
-
-.RS 1.0i
-.PD 0
-.TP 0.4i
-.I inode_%lld_data
-for inode data.
-.TP
-.I inode_%lld_data_bmbt
-for inode data extent maps.
-.TP
-.I inode_%lld_attr
-for inode extended attribute data.
-.TP
-.I inode_%lld_attr_bmbt
-for inode extended attribute extent maps.
-.TP
-.I special_%u:%u
-for other filesystem metadata.
-.PD
-.RE
-
-.TP
-.BI \-n " num_extents"
-If this option is given,
-.B fsmap
-obtains the extent list of the file in groups of
-.I num_extents
-extents.
-In the absence of
-.BR "-n" ", " "fsmap"
-queries the system for extents in groups of 131,072 records.
-.TP
-.B \-v
-Shows verbose information.
-When this flag is specified, additional AG specific information is
-appended to each line in the following form:
-.IP
-.RS 1.2i
-.IR agno " (" startagblock .. endagblock ") " nblocks " " flags
-.RE
-.IP
-A second
-.B \-v
-option will print out the
-.I flags
-legend.
-This option is not compatible with the
-.B \-m
-flag.
-.RE
-.PD
-.TP
 .BI "extsize [ \-R | \-D ] [ " value " ]"
 Display and/or modify the preferred extent size used when allocating
 space for the currently open file. If the
@@ -782,18 +680,37 @@ bytes of data.
 .RE
 .PD
 .TP
-.BI utimes " atime_sec atime_nsec mtime_sec mtime_nsec"
-The utimes command changes the atime and mtime of the current file.
-sec uses UNIX timestamp notation and is the seconds elapsed since
-1970-01-01 00:00:00 UTC.
-nsec is the nanoseconds since the sec. This value needs to be in
-the range 0-999999999 with UTIME_NOW and UTIME_OMIT being exceptions.
-Each (sec, nsec) pair constitutes a single timestamp value.
-.TP
 .BI swapext " donor_file "
 Swaps extent forks between files. The current open file is the target. The donor
 file is specified by path. Note that file data is not copied (file content moves
 with the fork(s)).
+.TP
+.BI "set_encpolicy [ \-c " mode " ] [ \-n " mode " ] [ \-f " flags " ] [ \-v " version " ] [ " keydesc " ]"
+On filesystems that support encryption, assign an encryption policy to the
+current file.
+.I keydesc
+is a 16-byte hex string which identifies the encryption key to use.
+If not specified, a "default" key descriptor of all 0's will be used.
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.BI \-c " mode"
+contents encryption mode (e.g. AES-256-XTS)
+.TP
+.BI \-n " mode"
+filenames encryption mode (e.g. AES-256-CTS)
+.TP
+.BI \-f " flags"
+policy flags (numeric)
+.TP
+.BI \-v " version"
+version of policy structure (numeric)
+.RE
+.PD
+.TP
+.BR get_encpolicy
+On filesystems that support encryption, display the encryption policy of the
+current file.
 
 .SH MEMORY MAPPED I/O COMMANDS
 .TP
@@ -946,29 +863,7 @@ which forces the maximum readahead.
 Dumps a list of pages or ranges of pages that are currently in core,
 for the current memory mapping.
 
-.SH OTHER COMMANDS
-.TP
-.BR "help [ " command " ]"
-Display a brief description of one or all commands.
-.TP
-.B print
-Display a list of all open files and memory mapped regions.
-The current file and current mapping are distinguishable from
-any others.
-.TP
-.B p
-See the
-.B print
-command.
-.TP
-.B quit
-Exit
-.BR xfs_io .
-.TP
-.B q
-See the
-.B quit
-command.
+.SH FILE COMMANDS
 .TP
 .BR lsattr " [ " \-R " | " \-D " | " \-a " | " \-v " ]"
 List extended inode flags on the currently open file. If the
@@ -992,50 +887,9 @@ for the full list) is available via the
 .B help
 command.
 .TP
-.B freeze
-Suspend all write I/O requests to the filesystem of the current file.
-Only available in expert mode and requires privileges.
-.TP
-.B thaw
-Undo the effects of a filesystem freeze operation.
-Only available in expert mode and requires privileges.
-.TP
 .BI "flink " path
 Link the currently open file descriptor into the filesystem namespace.
 .TP
-.BI "inject [ " tag " ]"
-Inject errors into a filesystem to observe filesystem behavior at
-specific points under adverse conditions. Without the
-.I tag
-argument, displays the list of error tags available.
-Only available in expert mode and requires privileges.
-.TP
-.BI "resblks [ " blocks " ]"
-Get and/or set count of reserved filesystem blocks using the
-XFS_IOC_GET_RESBLKS or XFS_IOC_SET_RESBLKS system calls.
-Note \-\- this can be useful for exercising out of space behavior.
-Only available in expert mode and requires privileges.
-.TP
-.BR shutdown " [ " \-f " ]"
-Force the filesystem to shut down, preventing any further IO.
-XFS and other filesystems implement this functionality, although implementation
-details may differ slightly.
-Only available in expert mode and requires privileges.
-.PP
-.RS
-By default, the filesystem will not attempt to flush completed transactions to
-disk before shutting down the filesystem.  This simulates a disk failure or
-crash.
-.RE
-.RS 1.0i
-.PD 0
-.TP 0.4i
-.B \-f
-Force the filesystem to flush all completed transactions to disk before shutting
-down, matching XFS behavior when critical corruption is encountered.
-.PD
-.RE
-.TP
 .BR stat " [ " \-v "|" \-r " ]"
 Selected statistics from
 .BR stat (2)
@@ -1076,12 +930,6 @@ Don't sync attributes with the server.
 .PD
 .RE
 .TP
-.B statfs
-Selected statistics from
-.BR statfs (2)
-and the XFS_IOC_FSGEOMETRY
-system call on the filesystem where the current file resides.
-.TP
 .BR chproj " [ " \-R | \-D " ]"
 Modifies the project identifier associated with the current path. The 
 .B \-R
@@ -1122,6 +970,64 @@ verbose output will be printed.
 .RE
 .PD
 .TP
+.BI utimes " atime_sec atime_nsec mtime_sec mtime_nsec"
+The utimes command changes the atime and mtime of the current file.
+sec uses UNIX timestamp notation and is the seconds elapsed since
+1970-01-01 00:00:00 UTC.
+nsec is the nanoseconds since the sec. This value needs to be in
+the range 0-999999999 with UTIME_NOW and UTIME_OMIT being exceptions.
+Each (sec, nsec) pair constitutes a single timestamp value.
+
+
+.SH FILESYSTEM COMMANDS
+.TP
+.B freeze
+Suspend all write I/O requests to the filesystem of the current file.
+Only available in expert mode and requires privileges.
+.TP
+.B thaw
+Undo the effects of a filesystem freeze operation.
+Only available in expert mode and requires privileges.
+.TP
+.BI "inject [ " tag " ]"
+Inject errors into a filesystem to observe filesystem behavior at
+specific points under adverse conditions. Without the
+.I tag
+argument, displays the list of error tags available.
+Only available in expert mode and requires privileges.
+.TP
+.BI "resblks [ " blocks " ]"
+Get and/or set count of reserved filesystem blocks using the
+XFS_IOC_GET_RESBLKS or XFS_IOC_SET_RESBLKS system calls.
+Note \-\- this can be useful for exercising out of space behavior.
+Only available in expert mode and requires privileges.
+.TP
+.BR shutdown " [ " \-f " ]"
+Force the filesystem to shut down, preventing any further IO.
+XFS and other filesystems implement this functionality, although implementation
+details may differ slightly.
+Only available in expert mode and requires privileges.
+.PP
+.RS
+By default, the filesystem will not attempt to flush completed transactions to
+disk before shutting down the filesystem.  This simulates a disk failure or
+crash.
+.RE
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.B \-f
+Force the filesystem to flush all completed transactions to disk before shutting
+down, matching XFS behavior when critical corruption is encountered.
+.PD
+.RE
+.TP
+.B statfs
+Selected statistics from
+.BR statfs (2)
+and the XFS_IOC_FSGEOMETRY
+system call on the filesystem where the current file resides.
+.TP
 .BI "inode  [ [ -n ] " number " ] [ -v ]"
 The inode command queries physical information about an inode. With
 no arguments, it will return 1 or 0, indicating whether or not any
@@ -1146,33 +1052,6 @@ was specified on the command line, the maximum possible inode number in
 the system will be printed along with its size.
 .PD
 .TP
-.BI "set_encpolicy [ \-c " mode " ] [ \-n " mode " ] [ \-f " flags " ] [ \-v " version " ] [ " keydesc " ]"
-On filesystems that support encryption, assign an encryption policy to the
-current file.
-.I keydesc
-is a 16-byte hex string which identifies the encryption key to use.
-If not specified, a "default" key descriptor of all 0's will be used.
-.RS 1.0i
-.PD 0
-.TP 0.4i
-.BI \-c " mode"
-contents encryption mode (e.g. AES-256-XTS)
-.TP
-.BI \-n " mode"
-filenames encryption mode (e.g. AES-256-CTS)
-.TP
-.BI \-f " flags"
-policy flags (numeric)
-.TP
-.BI \-v " version"
-version of policy structure (numeric)
-.RE
-.PD
-.TP
-.BR get_encpolicy
-On filesystems that support encryption, display the encryption policy of the
-current file.
-.TP
 .BI "scrub " type " [ " agnumber " | " "ino" " " "gen" " ]"
 Scrub internal XFS filesystem metadata.  The
 .BI type
@@ -1191,6 +1070,146 @@ For AG metadata, one AG number must be specified.
 For file metadata, the repair is applied to the open file unless the
 inode number and generation number are specified.
 .TP
+.BI "label" " " "[ -c | -s " label " ] "
+On filesystems that support online label manipulation, get, set, or clear the
+filesystem label.  With no options, print the current filesystem label.  The
+.B \-c
+option clears the filesystem label by setting it to the null string.  The
+.BI "\-s " label
+option sets the filesystem label to
+.IR label .
+If the label is longer than the filesystem will accept,
+.B xfs_io
+will print an error message.  XFS filesystem labels can be at most 12
+characters long.
+.TP
+.BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ]
+Prints the mapping of disk blocks used by the filesystem hosting the current
+file.  The map lists each extent used by files, allocation group metadata,
+journalling logs, and static filesystem metadata, as well as any
+regions that are unused.
+Each line of the listings takes the following form:
+.PP
+.RS
+.IR extent ": " major ":" minor " [" startblock .. endblock "]: " owner " " startoffset .. endoffset " " length
+.PP
+Static filesystem metadata, allocation group metadata, btrees,
+journalling logs, and free space are marked by replacing the
+.IR startoffset .. endoffset
+with the appropriate marker.
+All blocks, offsets, and lengths are specified in units of 512-byte
+blocks, no matter what the filesystem's block size is.
+The optional
+.I start
+and
+.I end
+arguments can be used to constrain the output to a particular range of
+disk blocks.
+If these two options are specified, exactly one of
+.BR "-d" ", " "-l" ", or " "-r"
+must also be set.
+.RE
+.RS 1.0i
+.PD 0
+.TP
+.BI \-d
+Display only extents from the data device.
+This option only applies for XFS filesystems.
+.TP
+.BI \-l
+Display only extents from the external log device.
+This option only applies to XFS filesystems.
+.TP
+.BI \-r
+Display only extents from the realtime device.
+This option only applies to XFS filesystems.
+.TP
+.BI \-m
+Display results in a machine readable format (CSV).
+This option is not compatible with the
+.B \-v
+flag.
+The columns of the output are: extent number, device major, device minor,
+physical start, physical end, owner, offset start, offset end, length.
+The start, end, and length numbers are provided in units of 512b.
+The owner field is a special string that takes the form:
+
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.I inode_%lld_data
+for inode data.
+.TP
+.I inode_%lld_data_bmbt
+for inode data extent maps.
+.TP
+.I inode_%lld_attr
+for inode extended attribute data.
+.TP
+.I inode_%lld_attr_bmbt
+for inode extended attribute extent maps.
+.TP
+.I special_%u:%u
+for other filesystem metadata.
+.PD
+.RE
+
+.TP
+.BI \-n " num_extents"
+If this option is given,
+.B fsmap
+obtains the extent list of the file in groups of
+.I num_extents
+extents.
+In the absence of
+.BR "-n" ", " "fsmap"
+queries the system for extents in groups of 131,072 records.
+.TP
+.B \-v
+Shows verbose information.
+When this flag is specified, additional AG specific information is
+appended to each line in the following form:
+.IP
+.RS 1.2i
+.IR agno " (" startagblock .. endagblock ") " nblocks " " flags
+.RE
+.IP
+A second
+.B \-v
+option will print out the
+.I flags
+legend.
+This option is not compatible with the
+.B \-m
+flag.
+.RE
+.PD
+
+
+.SH OTHER COMMANDS
+.TP
+.BR "help [ " command " ]"
+Display a brief description of one or all commands.
+.TP
+.B print
+Display a list of all open files and memory mapped regions.
+The current file and current mapping are distinguishable from
+any others.
+.TP
+.B p
+See the
+.B print
+command.
+.TP
+.B quit
+Exit
+.BR xfs_io .
+.TP
+.B q
+See the
+.B quit
+command.
+.TP
 .BI "log_writes \-d " device " \-m "  mark
 Create a mark named
 .I mark
@@ -1210,19 +1229,6 @@ See the
 .B log_writes
 command.
 .TP
-.BI "label" " " "[ -c | -s " label " ] "
-On filesystems that support online label manipulation, get, set, or clear the
-filesystem label.  With no options, print the current filesystem label.  The
-.B \-c
-option clears the filesystem label by setting it to the null string.  The
-.BI "\-s " label
-option sets the filesystem label to
-.IR label .
-If the label is longer than the filesystem will accept,
-.B xfs_io
-will print an error message.  XFS filesystem labels can be at most 12
-characters long.
-.TP
 .B crc32cselftest
 Test the internal crc32c implementation to make sure that it computes results
 correctly.

Re: [PATCH 1/6] xfs_io.8: rearrange command listings by section

[Eric Sandeen]

On 11/9/18 6:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Most of the commands listed under "OTHER COMMANDS" apply to files or
> filesystems.  Create two new sections for that and populate them
> appropriately.
> 
> Here's what moves:
> 
> fsmap: moves from file io commands to filesystem commands
> utimes: moves from file io commands to file commands
> 
>>From the OTHER COMMANDS section:
> 
> lsattr/chattr: moves to file commands
> flink: moves to file commands
> stat/statx: moves to file commands
> lsproj/chproj: moves to file commands
> parent: moves to file commands
> [gs]et_encpolicy: moves to file io commands
> freeze/thaw: move to filesystem commands
> inject: move to filesystem commands
> resblks: move to filesystem commands
> shutdown: move to filesystem commands
> statfs: move to filesystem commands
> label: move to filesystem commands

Thank you for getting this better organized.  Of course, I have
some questions.  :)

What does "FILE COMMANDS" vs. "FILE I/O COMMANDS" mean, exactly?
I ask because for example bmap & fiemap are still under FILE IO but
they... really don't do file IO.  What's your intended distinction
with these new sections?

Thanks,
-Eric

Re: [PATCH 1/6] xfs_io.8: rearrange command listings by section

[Darrick J. Wong]

On Wed, Nov 28, 2018 at 04:39:47PM -0600, Eric Sandeen wrote:
> On 11/9/18 6:45 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Most of the commands listed under "OTHER COMMANDS" apply to files or
> > filesystems.  Create two new sections for that and populate them
> > appropriately.
> > 
> > Here's what moves:
> > 
> > fsmap: moves from file io commands to filesystem commands
> > utimes: moves from file io commands to file commands
> > 
> >>From the OTHER COMMANDS section:
> > 
> > lsattr/chattr: moves to file commands
> > flink: moves to file commands
> > stat/statx: moves to file commands
> > lsproj/chproj: moves to file commands
> > parent: moves to file commands
> > [gs]et_encpolicy: moves to file io commands
> > freeze/thaw: move to filesystem commands
> > inject: move to filesystem commands
> > resblks: move to filesystem commands
> > shutdown: move to filesystem commands
> > statfs: move to filesystem commands
> > label: move to filesystem commands
> 
> Thank you for getting this better organized.  Of course, I have
> some questions.  :)
> 
> What does "FILE COMMANDS" vs. "FILE I/O COMMANDS" mean, exactly?
> I ask because for example bmap & fiemap are still under FILE IO but
> they... really don't do file IO.  What's your intended distinction
> with these new sections?

Admittedly, it's been a looong time since I wrote this patch.

Um... I think my intent was for FILE COMMANDS to pertain to file
attributes, whereas FILE I/O COMMANDS would pertain to reading, writing,
encoding, mapping, and otherwise messing with the contents of a file.

--D

> 
> Thanks,
> -Eric

Re: [PATCH 1/6] xfs_io.8: rearrange command listings by section

[Eric Sandeen]

On 11/28/18 5:34 PM, Darrick J. Wong wrote:
> On Wed, Nov 28, 2018 at 04:39:47PM -0600, Eric Sandeen wrote:
>> On 11/9/18 6:45 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> Most of the commands listed under "OTHER COMMANDS" apply to files or
>>> filesystems.  Create two new sections for that and populate them
>>> appropriately.
>>>
>>> Here's what moves:
>>>
>>> fsmap: moves from file io commands to filesystem commands
>>> utimes: moves from file io commands to file commands
>>>
>>> >From the OTHER COMMANDS section:
>>>
>>> lsattr/chattr: moves to file commands
>>> flink: moves to file commands
>>> stat/statx: moves to file commands
>>> lsproj/chproj: moves to file commands
>>> parent: moves to file commands
>>> [gs]et_encpolicy: moves to file io commands
>>> freeze/thaw: move to filesystem commands
>>> inject: move to filesystem commands
>>> resblks: move to filesystem commands
>>> shutdown: move to filesystem commands
>>> statfs: move to filesystem commands
>>> label: move to filesystem commands
>>
>> Thank you for getting this better organized.  Of course, I have
>> some questions.  :)
>>
>> What does "FILE COMMANDS" vs. "FILE I/O COMMANDS" mean, exactly?
>> I ask because for example bmap & fiemap are still under FILE IO but
>> they... really don't do file IO.  What's your intended distinction
>> with these new sections?
> 
> Admittedly, it's been a looong time since I wrote this patch.

yeah, that's my fault, sorry.

> Um... I think my intent was for FILE COMMANDS to pertain to file
> attributes, whereas FILE I/O COMMANDS would pertain to reading, writing,
> encoding, mapping, and otherwise messing with the contents of a file.

Because the distinction requires a fair bit of explanation I think it'd be
better to just lump them all into FILE OPERATIONS i.e. "these operate
on individual files not the filesystem in general."

Explicitly re-categorizing into rather vague categories doesn't seem
overly helpful.  :)

-Eric

[PATCH 1/6 V2] xfs_io.8: rearrange command listings by section

[Eric Sandeen]

From: Darrick J. Wong <darrick.wong@oracle.com>

Most of the commands listed under "OTHER COMMANDS" apply to files
or filesystems.  Create a new "FILESYSTEM COMMANDS" section and
populate it appropriately, and move others to "FILE IO"

Here's what moves:

fsmap: moves from file io commands to filesystem commands

>From the OTHER COMMANDS section:

lsattr/chattr: moves to file io commands
flink: moves to file io commands
stat/statx: moves to file io commands
lsproj/chproj: moves to file io commands
parent: moves to file io commands
[gs]et_encpolicy: moves to file io commands
freeze/thaw: move to filesystem commands
inject: move to filesystem commands
resblks: move to filesystem commands
shutdown: move to filesystem commands
statfs: move to filesystem commands
label: move to filesystem commands

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
[sandeen: do away with FILE IO / FILE command distinction]
---

V2: move all of the "FILE COMMANDS" up into the "FILE I/O" section
because I cannot tell what the difference between the two is intended
to be, and arbitrary distinctions only seems more confusing.

If anybody really wants to try to suss that out it can be categorized
further in a later patch.

diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
index f1099c3..a37ed3c 100644
--- a/man/man8/xfs_io.8
+++ b/man/man8/xfs_io.8
@@ -338,108 +338,6 @@ or ends in a hole,
 will print the hole, truncated to the requested range.
 .RE
 .TP
-.BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ]
-Prints the mapping of disk blocks used by the filesystem hosting the current
-file.  The map lists each extent used by files, allocation group metadata,
-journalling logs, and static filesystem metadata, as well as any
-regions that are unused.
-Each line of the listings takes the following form:
-.PP
-.RS
-.IR extent ": " major ":" minor " [" startblock .. endblock "]: " owner " " startoffset .. endoffset " " length
-.PP
-Static filesystem metadata, allocation group metadata, btrees,
-journalling logs, and free space are marked by replacing the
-.IR startoffset .. endoffset
-with the appropriate marker.
-All blocks, offsets, and lengths are specified in units of 512-byte
-blocks, no matter what the filesystem's block size is.
-The optional
-.I start
-and
-.I end
-arguments can be used to constrain the output to a particular range of
-disk blocks.
-If these two options are specified, exactly one of
-.BR "-d" ", " "-l" ", or " "-r"
-must also be set.
-.RE
-.RS 1.0i
-.PD 0
-.TP
-.BI \-d
-Display only extents from the data device.
-This option only applies for XFS filesystems.
-.TP
-.BI \-l
-Display only extents from the external log device.
-This option only applies to XFS filesystems.
-.TP
-.BI \-r
-Display only extents from the realtime device.
-This option only applies to XFS filesystems.
-.TP
-.BI \-m
-Display results in a machine readable format (CSV).
-This option is not compatible with the
-.B \-v
-flag.
-The columns of the output are: extent number, device major, device minor,
-physical start, physical end, owner, offset start, offset end, length.
-The start, end, and length numbers are provided in units of 512b.
-The owner field is a special string that takes the form:
-
-.RS 1.0i
-.PD 0
-.TP 0.4i
-.I inode_%lld_data
-for inode data.
-.TP
-.I inode_%lld_data_bmbt
-for inode data extent maps.
-.TP
-.I inode_%lld_attr
-for inode extended attribute data.
-.TP
-.I inode_%lld_attr_bmbt
-for inode extended attribute extent maps.
-.TP
-.I special_%u:%u
-for other filesystem metadata.
-.PD
-.RE
-
-.TP
-.BI \-n " num_extents"
-If this option is given,
-.B fsmap
-obtains the extent list of the file in groups of
-.I num_extents
-extents.
-In the absence of
-.BR "-n" ", " "fsmap"
-queries the system for extents in groups of 131,072 records.
-.TP
-.B \-v
-Shows verbose information.
-When this flag is specified, additional AG specific information is
-appended to each line in the following form:
-.IP
-.RS 1.2i
-.IR agno " (" startagblock .. endagblock ") " nblocks " " flags
-.RE
-.IP
-A second
-.B \-v
-option will print out the
-.I flags
-legend.
-This option is not compatible with the
-.B \-m
-flag.
-.RE
-.PD
-.TP
 .BI "extsize [ \-R | \-D ] [ " value " ]"
 Display and/or modify the preferred extent size used when allocating
 space for the currently open file. If the
@@ -782,6 +680,144 @@ bytes of data.
 .RE
 .PD
 .TP
+.BI swapext " donor_file "
+Swaps extent forks between files. The current open file is the target. The donor
+file is specified by path. Note that file data is not copied (file content moves
+with the fork(s)).
+.TP
+.BI "set_encpolicy [ \-c " mode " ] [ \-n " mode " ] [ \-f " flags " ] [ \-v " version " ] [ " keydesc " ]"
+On filesystems that support encryption, assign an encryption policy to the
+current file.
+.I keydesc
+is a 16-byte hex string which identifies the encryption key to use.
+If not specified, a "default" key descriptor of all 0's will be used.
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.BI \-c " mode"
+contents encryption mode (e.g. AES-256-XTS)
+.TP
+.BI \-n " mode"
+filenames encryption mode (e.g. AES-256-CTS)
+.TP
+.BI \-f " flags"
+policy flags (numeric)
+.TP
+.BI \-v " version"
+version of policy structure (numeric)
+.RE
+.PD
+.TP
+.BR get_encpolicy
+On filesystems that support encryption, display the encryption policy of the
+current file.
+
+.TP
+.BR lsattr " [ " \-R " | " \-D " | " \-a " | " \-v " ]"
+List extended inode flags on the currently open file. If the
+.B \-R
+option is specified, a recursive descent is performed
+for all directory entries below the currently open file
+.RB ( \-D
+can be used to restrict the output to directories only).
+This is a depth first descent, it does not follow symlinks and
+it also does not cross mount points.
+.TP
+.BR chattr " [ " \-R " | " \-D " ] [ " + / \-riasAdtPneEfSxC " ]"
+Change extended inode flags on the currently open file. The
+.B \-R
+and
+.B \-D
+options have the same meaning as above. The mapping between each
+letter and the inode flags (refer to
+.BR xfsctl (3)
+for the full list) is available via the
+.B help
+command.
+.TP
+.BI "flink " path
+Link the currently open file descriptor into the filesystem namespace.
+.TP
+.BR stat " [ " \-v "|" \-r " ]"
+Selected statistics from
+.BR stat (2)
+and the XFS_IOC_GETXATTR system call on the current file. If the
+.B \-v
+option is specified, the atime (last access), mtime
+(last modify), and ctime (last change) timestamps are also displayed.  The
+.B \-r
+option dumps raw fields from the stat structure.
+.TP
+.BI "statx [ \-v|\-r ][ \-m " basic " | \-m " all " | -m " <mask> " ][ \-FD ]"
+Selected statistics from
+.BR stat (2)
+and the XFS_IOC_GETXATTR system call on the current file.
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.B \-v
+Show timestamps.
+.TP
+.B \-r
+Dump raw statx structure values.
+.TP
+.B \-m basic
+Set the field mask for the statx call to STATX_BASIC_STATS.
+.TP
+.B \-m all
+Set the the field mask for the statx call to STATX_ALL (default).
+.TP
+.B \-m <mask>
+Specify a numeric field mask for the statx call.
+.TP
+.B \-F
+Force the attributes to be synced with the server.
+.TP
+.B \-D
+Don't sync attributes with the server.
+.PD
+.RE
+.TP
+.BR chproj " [ " \-R | \-D " ]"
+Modifies the project identifier associated with the current path. The 
+.B \-R
+option will recursively descend if the current path is a directory. The 
+.B \-D
+option will also recursively descend, only setting modifying projects 
+on subdirectories.  See the
+.BR xfs_quota (8)
+manual page for more information about project identifiers.
+.TP
+.BR lsproj " [ " \-R | \-D " ]"
+Displays the project identifier associated with the current path. The 
+.B \-R
+and
+.B \-D
+options behave as described above, in
+.B chproj.
+.TP
+.BR parent " [ " \-cpv " ]"
+By default this command prints out the parent inode numbers,
+inode generation numbers and basenames of all the hardlinks which
+point to the inode of the current file.
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.B \-p
+the output is similar to the default output except pathnames up to
+the mount-point are printed out instead of the component name.
+.TP
+.B \-c
+the file's filesystem will check all the parent attributes for consistency.
+.TP
+.B \-v
+verbose output will be printed.
+.RE
+.IP
+.B [NOTE: Not currently operational on Linux.]
+.RE
+.PD
+.TP
 .BI utimes " atime_sec atime_nsec mtime_sec mtime_nsec"
 The utimes command changes the atime and mtime of the current file.
 sec uses UNIX timestamp notation and is the seconds elapsed since
@@ -789,11 +825,7 @@ sec uses UNIX timestamp notation and is the seconds elapsed since
 nsec is the nanoseconds since the sec. This value needs to be in
 the range 0-999999999 with UTIME_NOW and UTIME_OMIT being exceptions.
 Each (sec, nsec) pair constitutes a single timestamp value.
-.TP
-.BI swapext " donor_file "
-Swaps extent forks between files. The current open file is the target. The donor
-file is specified by path. Note that file data is not copied (file content moves
-with the fork(s)).
+
 
 .SH MEMORY MAPPED I/O COMMANDS
 .TP
@@ -946,63 +978,16 @@ which forces the maximum readahead.
 Dumps a list of pages or ranges of pages that are currently in core,
 for the current memory mapping.
 
-.SH OTHER COMMANDS
+.SH FILESYSTEM COMMANDS
 .TP
-.BR "help [ " command " ]"
-Display a brief description of one or all commands.
-.TP
-.B print
-Display a list of all open files and memory mapped regions.
-The current file and current mapping are distinguishable from
-any others.
-.TP
-.B p
-See the
-.B print
-command.
-.TP
-.B quit
-Exit
-.BR xfs_io .
-.TP
-.B q
-See the
-.B quit
-command.
-.TP
-.BR lsattr " [ " \-R " | " \-D " | " \-a " | " \-v " ]"
-List extended inode flags on the currently open file. If the
-.B \-R
-option is specified, a recursive descent is performed
-for all directory entries below the currently open file
-.RB ( \-D
-can be used to restrict the output to directories only).
-This is a depth first descent, it does not follow symlinks and
-it also does not cross mount points.
-.TP
-.BR chattr " [ " \-R " | " \-D " ] [ " + / \-riasAdtPneEfSxC " ]"
-Change extended inode flags on the currently open file. The
-.B \-R
-and
-.B \-D
-options have the same meaning as above. The mapping between each
-letter and the inode flags (refer to
-.BR xfsctl (3)
-for the full list) is available via the
-.B help
-command.
-.TP
-.B freeze
-Suspend all write I/O requests to the filesystem of the current file.
-Only available in expert mode and requires privileges.
+.B freeze
+Suspend all write I/O requests to the filesystem of the current file.
+Only available in expert mode and requires privileges.
 .TP
 .B thaw
 Undo the effects of a filesystem freeze operation.
 Only available in expert mode and requires privileges.
 .TP
-.BI "flink " path
-Link the currently open file descriptor into the filesystem namespace.
-.TP
 .BI "inject [ " tag " ]"
 Inject errors into a filesystem to observe filesystem behavior at
 specific points under adverse conditions. Without the
@@ -1036,92 +1021,12 @@ down, matching XFS behavior when critical corruption is encountered.
 .PD
 .RE
 .TP
-.BR stat " [ " \-v "|" \-r " ]"
-Selected statistics from
-.BR stat (2)
-and the XFS_IOC_GETXATTR system call on the current file. If the
-.B \-v
-option is specified, the atime (last access), mtime
-(last modify), and ctime (last change) timestamps are also displayed.  The
-.B \-r
-option dumps raw fields from the stat structure.
-.TP
-.BI "statx [ \-v|\-r ][ \-m " basic " | \-m " all " | -m " <mask> " ][ \-FD ]"
-Selected statistics from
-.BR stat (2)
-and the XFS_IOC_GETXATTR system call on the current file.
-.RS 1.0i
-.PD 0
-.TP 0.4i
-.B \-v
-Show timestamps.
-.TP
-.B \-r
-Dump raw statx structure values.
-.TP
-.B \-m basic
-Set the field mask for the statx call to STATX_BASIC_STATS.
-.TP
-.B \-m all
-Set the the field mask for the statx call to STATX_ALL (default).
-.TP
-.B \-m <mask>
-Specify a numeric field mask for the statx call.
-.TP
-.B \-F
-Force the attributes to be synced with the server.
-.TP
-.B \-D
-Don't sync attributes with the server.
-.PD
-.RE
-.TP
 .B statfs
 Selected statistics from
 .BR statfs (2)
 and the XFS_IOC_FSGEOMETRY
 system call on the filesystem where the current file resides.
 .TP
-.BR chproj " [ " \-R | \-D " ]"
-Modifies the project identifier associated with the current path. The 
-.B \-R
-option will recursively descend if the current path is a directory. The 
-.B \-D
-option will also recursively descend, only setting modifying projects 
-on subdirectories.  See the
-.BR xfs_quota (8)
-manual page for more information about project identifiers.
-.TP
-.BR lsproj " [ " \-R | \-D " ]"
-Displays the project identifier associated with the current path. The 
-.B \-R
-and
-.B \-D
-options behave as described above, in
-.B chproj.
-.TP
-.BR parent " [ " \-cpv " ]"
-By default this command prints out the parent inode numbers,
-inode generation numbers and basenames of all the hardlinks which
-point to the inode of the current file.
-.RS 1.0i
-.PD 0
-.TP 0.4i
-.B \-p
-the output is similar to the default output except pathnames up to
-the mount-point are printed out instead of the component name.
-.TP
-.B \-c
-the file's filesystem will check all the parent attributes for consistency.
-.TP
-.B \-v
-verbose output will be printed.
-.RE
-.IP
-.B [NOTE: Not currently operational on Linux.]
-.RE
-.PD
-.TP
 .BI "inode  [ [ -n ] " number " ] [ -v ]"
 The inode command queries physical information about an inode. With
 no arguments, it will return 1 or 0, indicating whether or not any
@@ -1146,33 +1051,6 @@ was specified on the command line, the maximum possible inode number in
 the system will be printed along with its size.
 .PD
 .TP
-.BI "set_encpolicy [ \-c " mode " ] [ \-n " mode " ] [ \-f " flags " ] [ \-v " version " ] [ " keydesc " ]"
-On filesystems that support encryption, assign an encryption policy to the
-current file.
-.I keydesc
-is a 16-byte hex string which identifies the encryption key to use.
-If not specified, a "default" key descriptor of all 0's will be used.
-.RS 1.0i
-.PD 0
-.TP 0.4i
-.BI \-c " mode"
-contents encryption mode (e.g. AES-256-XTS)
-.TP
-.BI \-n " mode"
-filenames encryption mode (e.g. AES-256-CTS)
-.TP
-.BI \-f " flags"
-policy flags (numeric)
-.TP
-.BI \-v " version"
-version of policy structure (numeric)
-.RE
-.PD
-.TP
-.BR get_encpolicy
-On filesystems that support encryption, display the encryption policy of the
-current file.
-.TP
 .BI "scrub " type " [ " agnumber " | " "ino" " " "gen" " ]"
 Scrub internal XFS filesystem metadata.  The
 .BI type
@@ -1191,6 +1069,146 @@ For AG metadata, one AG number must be specified.
 For file metadata, the repair is applied to the open file unless the
 inode number and generation number are specified.
 .TP
+.BI "label" " " "[ -c | -s " label " ] "
+On filesystems that support online label manipulation, get, set, or clear the
+filesystem label.  With no options, print the current filesystem label.  The
+.B \-c
+option clears the filesystem label by setting it to the null string.  The
+.BI "\-s " label
+option sets the filesystem label to
+.IR label .
+If the label is longer than the filesystem will accept,
+.B xfs_io
+will print an error message.  XFS filesystem labels can be at most 12
+characters long.
+.TP
+.BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ]
+Prints the mapping of disk blocks used by the filesystem hosting the current
+file.  The map lists each extent used by files, allocation group metadata,
+journalling logs, and static filesystem metadata, as well as any
+regions that are unused.
+Each line of the listings takes the following form:
+.PP
+.RS
+.IR extent ": " major ":" minor " [" startblock .. endblock "]: " owner " " startoffset .. endoffset " " length
+.PP
+Static filesystem metadata, allocation group metadata, btrees,
+journalling logs, and free space are marked by replacing the
+.IR startoffset .. endoffset
+with the appropriate marker.
+All blocks, offsets, and lengths are specified in units of 512-byte
+blocks, no matter what the filesystem's block size is.
+The optional
+.I start
+and
+.I end
+arguments can be used to constrain the output to a particular range of
+disk blocks.
+If these two options are specified, exactly one of
+.BR "-d" ", " "-l" ", or " "-r"
+must also be set.
+.RE
+.RS 1.0i
+.PD 0
+.TP
+.BI \-d
+Display only extents from the data device.
+This option only applies for XFS filesystems.
+.TP
+.BI \-l
+Display only extents from the external log device.
+This option only applies to XFS filesystems.
+.TP
+.BI \-r
+Display only extents from the realtime device.
+This option only applies to XFS filesystems.
+.TP
+.BI \-m
+Display results in a machine readable format (CSV).
+This option is not compatible with the
+.B \-v
+flag.
+The columns of the output are: extent number, device major, device minor,
+physical start, physical end, owner, offset start, offset end, length.
+The start, end, and length numbers are provided in units of 512b.
+The owner field is a special string that takes the form:
+
+.RS 1.0i
+.PD 0
+.TP 0.4i
+.I inode_%lld_data
+for inode data.
+.TP
+.I inode_%lld_data_bmbt
+for inode data extent maps.
+.TP
+.I inode_%lld_attr
+for inode extended attribute data.
+.TP
+.I inode_%lld_attr_bmbt
+for inode extended attribute extent maps.
+.TP
+.I special_%u:%u
+for other filesystem metadata.
+.PD
+.RE
+
+.TP
+.BI \-n " num_extents"
+If this option is given,
+.B fsmap
+obtains the extent list of the file in groups of
+.I num_extents
+extents.
+In the absence of
+.BR "-n" ", " "fsmap"
+queries the system for extents in groups of 131,072 records.
+.TP
+.B \-v
+Shows verbose information.
+When this flag is specified, additional AG specific information is
+appended to each line in the following form:
+.IP
+.RS 1.2i
+.IR agno " (" startagblock .. endagblock ") " nblocks " " flags
+.RE
+.IP
+A second
+.B \-v
+option will print out the
+.I flags
+legend.
+This option is not compatible with the
+.B \-m
+flag.
+.RE
+.PD
+
+
+.SH OTHER COMMANDS
+.TP
+.BR "help [ " command " ]"
+Display a brief description of one or all commands.
+.TP
+.B print
+Display a list of all open files and memory mapped regions.
+The current file and current mapping are distinguishable from
+any others.
+.TP
+.B p
+See the
+.B print
+command.
+.TP
+.B quit
+Exit
+.BR xfs_io .
+.TP
+.B q
+See the
+.B quit
+command.
+.TP
 .BI "log_writes \-d " device " \-m "  mark
 Create a mark named
 .I mark
@@ -1210,19 +1228,6 @@ See the
 .B log_writes
 command.
 .TP
-.BI "label" " " "[ -c | -s " label " ] "
-On filesystems that support online label manipulation, get, set, or clear the
-filesystem label.  With no options, print the current filesystem label.  The
-.B \-c
-option clears the filesystem label by setting it to the null string.  The
-.BI "\-s " label
-option sets the filesystem label to
-.IR label .
-If the label is longer than the filesystem will accept,
-.B xfs_io
-will print an error message.  XFS filesystem labels can be at most 12
-characters long.
-.TP
 .B crc32cselftest
 Test the internal crc32c implementation to make sure that it computes results
 correctly.

Re: [PATCH 1/6 V2] xfs_io.8: rearrange command listings by section

[Darrick J. Wong]

On Tue, Dec 04, 2018 at 09:21:39PM -0600, Eric Sandeen wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Most of the commands listed under "OTHER COMMANDS" apply to files
> or filesystems.  Create a new "FILESYSTEM COMMANDS" section and
> populate it appropriately, and move others to "FILE IO"
> 
> Here's what moves:
> 
> fsmap: moves from file io commands to filesystem commands
> 
> >From the OTHER COMMANDS section:
> 
> lsattr/chattr: moves to file io commands
> flink: moves to file io commands
> stat/statx: moves to file io commands
> lsproj/chproj: moves to file io commands
> parent: moves to file io commands
> [gs]et_encpolicy: moves to file io commands
> freeze/thaw: move to filesystem commands
> inject: move to filesystem commands
> resblks: move to filesystem commands
> shutdown: move to filesystem commands
> statfs: move to filesystem commands
> label: move to filesystem commands
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> [sandeen: do away with FILE IO / FILE command distinction]
> ---
> 
> V2: move all of the "FILE COMMANDS" up into the "FILE I/O" section
> because I cannot tell what the difference between the two is intended
> to be, and arbitrary distinctions only seems more confusing.
> 
> If anybody really wants to try to suss that out it can be categorized
> further in a later patch.

Looks fine to me, thanks for fixing this up.

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> diff --git a/man/man8/xfs_io.8 b/man/man8/xfs_io.8
> index f1099c3..a37ed3c 100644
> --- a/man/man8/xfs_io.8
> +++ b/man/man8/xfs_io.8
> @@ -338,108 +338,6 @@ or ends in a hole,
>  will print the hole, truncated to the requested range.
>  .RE
>  .TP
> -.BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ]
> -Prints the mapping of disk blocks used by the filesystem hosting the current
> -file.  The map lists each extent used by files, allocation group metadata,
> -journalling logs, and static filesystem metadata, as well as any
> -regions that are unused.
> -Each line of the listings takes the following form:
> -.PP
> -.RS
> -.IR extent ": " major ":" minor " [" startblock .. endblock "]: " owner " " startoffset .. endoffset " " length
> -.PP
> -Static filesystem metadata, allocation group metadata, btrees,
> -journalling logs, and free space are marked by replacing the
> -.IR startoffset .. endoffset
> -with the appropriate marker.
> -All blocks, offsets, and lengths are specified in units of 512-byte
> -blocks, no matter what the filesystem's block size is.
> -The optional
> -.I start
> -and
> -.I end
> -arguments can be used to constrain the output to a particular range of
> -disk blocks.
> -If these two options are specified, exactly one of
> -.BR "-d" ", " "-l" ", or " "-r"
> -must also be set.
> -.RE
> -.RS 1.0i
> -.PD 0
> -.TP
> -.BI \-d
> -Display only extents from the data device.
> -This option only applies for XFS filesystems.
> -.TP
> -.BI \-l
> -Display only extents from the external log device.
> -This option only applies to XFS filesystems.
> -.TP
> -.BI \-r
> -Display only extents from the realtime device.
> -This option only applies to XFS filesystems.
> -.TP
> -.BI \-m
> -Display results in a machine readable format (CSV).
> -This option is not compatible with the
> -.B \-v
> -flag.
> -The columns of the output are: extent number, device major, device minor,
> -physical start, physical end, owner, offset start, offset end, length.
> -The start, end, and length numbers are provided in units of 512b.
> -The owner field is a special string that takes the form:
> -
> -.RS 1.0i
> -.PD 0
> -.TP 0.4i
> -.I inode_%lld_data
> -for inode data.
> -.TP
> -.I inode_%lld_data_bmbt
> -for inode data extent maps.
> -.TP
> -.I inode_%lld_attr
> -for inode extended attribute data.
> -.TP
> -.I inode_%lld_attr_bmbt
> -for inode extended attribute extent maps.
> -.TP
> -.I special_%u:%u
> -for other filesystem metadata.
> -.PD
> -.RE
> -
> -.TP
> -.BI \-n " num_extents"
> -If this option is given,
> -.B fsmap
> -obtains the extent list of the file in groups of
> -.I num_extents
> -extents.
> -In the absence of
> -.BR "-n" ", " "fsmap"
> -queries the system for extents in groups of 131,072 records.
> -.TP
> -.B \-v
> -Shows verbose information.
> -When this flag is specified, additional AG specific information is
> -appended to each line in the following form:
> -.IP
> -.RS 1.2i
> -.IR agno " (" startagblock .. endagblock ") " nblocks " " flags
> -.RE
> -.IP
> -A second
> -.B \-v
> -option will print out the
> -.I flags
> -legend.
> -This option is not compatible with the
> -.B \-m
> -flag.
> -.RE
> -.PD
> -.TP
>  .BI "extsize [ \-R | \-D ] [ " value " ]"
>  Display and/or modify the preferred extent size used when allocating
>  space for the currently open file. If the
> @@ -782,6 +680,144 @@ bytes of data.
>  .RE
>  .PD
>  .TP
> +.BI swapext " donor_file "
> +Swaps extent forks between files. The current open file is the target. The donor
> +file is specified by path. Note that file data is not copied (file content moves
> +with the fork(s)).
> +.TP
> +.BI "set_encpolicy [ \-c " mode " ] [ \-n " mode " ] [ \-f " flags " ] [ \-v " version " ] [ " keydesc " ]"
> +On filesystems that support encryption, assign an encryption policy to the
> +current file.
> +.I keydesc
> +is a 16-byte hex string which identifies the encryption key to use.
> +If not specified, a "default" key descriptor of all 0's will be used.
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.BI \-c " mode"
> +contents encryption mode (e.g. AES-256-XTS)
> +.TP
> +.BI \-n " mode"
> +filenames encryption mode (e.g. AES-256-CTS)
> +.TP
> +.BI \-f " flags"
> +policy flags (numeric)
> +.TP
> +.BI \-v " version"
> +version of policy structure (numeric)
> +.RE
> +.PD
> +.TP
> +.BR get_encpolicy
> +On filesystems that support encryption, display the encryption policy of the
> +current file.
> +
> +.TP
> +.BR lsattr " [ " \-R " | " \-D " | " \-a " | " \-v " ]"
> +List extended inode flags on the currently open file. If the
> +.B \-R
> +option is specified, a recursive descent is performed
> +for all directory entries below the currently open file
> +.RB ( \-D
> +can be used to restrict the output to directories only).
> +This is a depth first descent, it does not follow symlinks and
> +it also does not cross mount points.
> +.TP
> +.BR chattr " [ " \-R " | " \-D " ] [ " + / \-riasAdtPneEfSxC " ]"
> +Change extended inode flags on the currently open file. The
> +.B \-R
> +and
> +.B \-D
> +options have the same meaning as above. The mapping between each
> +letter and the inode flags (refer to
> +.BR xfsctl (3)
> +for the full list) is available via the
> +.B help
> +command.
> +.TP
> +.BI "flink " path
> +Link the currently open file descriptor into the filesystem namespace.
> +.TP
> +.BR stat " [ " \-v "|" \-r " ]"
> +Selected statistics from
> +.BR stat (2)
> +and the XFS_IOC_GETXATTR system call on the current file. If the
> +.B \-v
> +option is specified, the atime (last access), mtime
> +(last modify), and ctime (last change) timestamps are also displayed.  The
> +.B \-r
> +option dumps raw fields from the stat structure.
> +.TP
> +.BI "statx [ \-v|\-r ][ \-m " basic " | \-m " all " | -m " <mask> " ][ \-FD ]"
> +Selected statistics from
> +.BR stat (2)
> +and the XFS_IOC_GETXATTR system call on the current file.
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.B \-v
> +Show timestamps.
> +.TP
> +.B \-r
> +Dump raw statx structure values.
> +.TP
> +.B \-m basic
> +Set the field mask for the statx call to STATX_BASIC_STATS.
> +.TP
> +.B \-m all
> +Set the the field mask for the statx call to STATX_ALL (default).
> +.TP
> +.B \-m <mask>
> +Specify a numeric field mask for the statx call.
> +.TP
> +.B \-F
> +Force the attributes to be synced with the server.
> +.TP
> +.B \-D
> +Don't sync attributes with the server.
> +.PD
> +.RE
> +.TP
> +.BR chproj " [ " \-R | \-D " ]"
> +Modifies the project identifier associated with the current path. The 
> +.B \-R
> +option will recursively descend if the current path is a directory. The 
> +.B \-D
> +option will also recursively descend, only setting modifying projects 
> +on subdirectories.  See the
> +.BR xfs_quota (8)
> +manual page for more information about project identifiers.
> +.TP
> +.BR lsproj " [ " \-R | \-D " ]"
> +Displays the project identifier associated with the current path. The 
> +.B \-R
> +and
> +.B \-D
> +options behave as described above, in
> +.B chproj.
> +.TP
> +.BR parent " [ " \-cpv " ]"
> +By default this command prints out the parent inode numbers,
> +inode generation numbers and basenames of all the hardlinks which
> +point to the inode of the current file.
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.B \-p
> +the output is similar to the default output except pathnames up to
> +the mount-point are printed out instead of the component name.
> +.TP
> +.B \-c
> +the file's filesystem will check all the parent attributes for consistency.
> +.TP
> +.B \-v
> +verbose output will be printed.
> +.RE
> +.IP
> +.B [NOTE: Not currently operational on Linux.]
> +.RE
> +.PD
> +.TP
>  .BI utimes " atime_sec atime_nsec mtime_sec mtime_nsec"
>  The utimes command changes the atime and mtime of the current file.
>  sec uses UNIX timestamp notation and is the seconds elapsed since
> @@ -789,11 +825,7 @@ sec uses UNIX timestamp notation and is the seconds elapsed since
>  nsec is the nanoseconds since the sec. This value needs to be in
>  the range 0-999999999 with UTIME_NOW and UTIME_OMIT being exceptions.
>  Each (sec, nsec) pair constitutes a single timestamp value.
> -.TP
> -.BI swapext " donor_file "
> -Swaps extent forks between files. The current open file is the target. The donor
> -file is specified by path. Note that file data is not copied (file content moves
> -with the fork(s)).
> +
>  
>  .SH MEMORY MAPPED I/O COMMANDS
>  .TP
> @@ -946,63 +978,16 @@ which forces the maximum readahead.
>  Dumps a list of pages or ranges of pages that are currently in core,
>  for the current memory mapping.
>  
> -.SH OTHER COMMANDS
> +.SH FILESYSTEM COMMANDS
>  .TP
> -.BR "help [ " command " ]"
> -Display a brief description of one or all commands.
> -.TP
> -.B print
> -Display a list of all open files and memory mapped regions.
> -The current file and current mapping are distinguishable from
> -any others.
> -.TP
> -.B p
> -See the
> -.B print
> -command.
> -.TP
> -.B quit
> -Exit
> -.BR xfs_io .
> -.TP
> -.B q
> -See the
> -.B quit
> -command.
> -.TP
> -.BR lsattr " [ " \-R " | " \-D " | " \-a " | " \-v " ]"
> -List extended inode flags on the currently open file. If the
> -.B \-R
> -option is specified, a recursive descent is performed
> -for all directory entries below the currently open file
> -.RB ( \-D
> -can be used to restrict the output to directories only).
> -This is a depth first descent, it does not follow symlinks and
> -it also does not cross mount points.
> -.TP
> -.BR chattr " [ " \-R " | " \-D " ] [ " + / \-riasAdtPneEfSxC " ]"
> -Change extended inode flags on the currently open file. The
> -.B \-R
> -and
> -.B \-D
> -options have the same meaning as above. The mapping between each
> -letter and the inode flags (refer to
> -.BR xfsctl (3)
> -for the full list) is available via the
> -.B help
> -command.
> -.TP
> -.B freeze
> -Suspend all write I/O requests to the filesystem of the current file.
> -Only available in expert mode and requires privileges.
> +.B freeze
> +Suspend all write I/O requests to the filesystem of the current file.
> +Only available in expert mode and requires privileges.
>  .TP
>  .B thaw
>  Undo the effects of a filesystem freeze operation.
>  Only available in expert mode and requires privileges.
>  .TP
> -.BI "flink " path
> -Link the currently open file descriptor into the filesystem namespace.
> -.TP
>  .BI "inject [ " tag " ]"
>  Inject errors into a filesystem to observe filesystem behavior at
>  specific points under adverse conditions. Without the
> @@ -1036,92 +1021,12 @@ down, matching XFS behavior when critical corruption is encountered.
>  .PD
>  .RE
>  .TP
> -.BR stat " [ " \-v "|" \-r " ]"
> -Selected statistics from
> -.BR stat (2)
> -and the XFS_IOC_GETXATTR system call on the current file. If the
> -.B \-v
> -option is specified, the atime (last access), mtime
> -(last modify), and ctime (last change) timestamps are also displayed.  The
> -.B \-r
> -option dumps raw fields from the stat structure.
> -.TP
> -.BI "statx [ \-v|\-r ][ \-m " basic " | \-m " all " | -m " <mask> " ][ \-FD ]"
> -Selected statistics from
> -.BR stat (2)
> -and the XFS_IOC_GETXATTR system call on the current file.
> -.RS 1.0i
> -.PD 0
> -.TP 0.4i
> -.B \-v
> -Show timestamps.
> -.TP
> -.B \-r
> -Dump raw statx structure values.
> -.TP
> -.B \-m basic
> -Set the field mask for the statx call to STATX_BASIC_STATS.
> -.TP
> -.B \-m all
> -Set the the field mask for the statx call to STATX_ALL (default).
> -.TP
> -.B \-m <mask>
> -Specify a numeric field mask for the statx call.
> -.TP
> -.B \-F
> -Force the attributes to be synced with the server.
> -.TP
> -.B \-D
> -Don't sync attributes with the server.
> -.PD
> -.RE
> -.TP
>  .B statfs
>  Selected statistics from
>  .BR statfs (2)
>  and the XFS_IOC_FSGEOMETRY
>  system call on the filesystem where the current file resides.
>  .TP
> -.BR chproj " [ " \-R | \-D " ]"
> -Modifies the project identifier associated with the current path. The 
> -.B \-R
> -option will recursively descend if the current path is a directory. The 
> -.B \-D
> -option will also recursively descend, only setting modifying projects 
> -on subdirectories.  See the
> -.BR xfs_quota (8)
> -manual page for more information about project identifiers.
> -.TP
> -.BR lsproj " [ " \-R | \-D " ]"
> -Displays the project identifier associated with the current path. The 
> -.B \-R
> -and
> -.B \-D
> -options behave as described above, in
> -.B chproj.
> -.TP
> -.BR parent " [ " \-cpv " ]"
> -By default this command prints out the parent inode numbers,
> -inode generation numbers and basenames of all the hardlinks which
> -point to the inode of the current file.
> -.RS 1.0i
> -.PD 0
> -.TP 0.4i
> -.B \-p
> -the output is similar to the default output except pathnames up to
> -the mount-point are printed out instead of the component name.
> -.TP
> -.B \-c
> -the file's filesystem will check all the parent attributes for consistency.
> -.TP
> -.B \-v
> -verbose output will be printed.
> -.RE
> -.IP
> -.B [NOTE: Not currently operational on Linux.]
> -.RE
> -.PD
> -.TP
>  .BI "inode  [ [ -n ] " number " ] [ -v ]"
>  The inode command queries physical information about an inode. With
>  no arguments, it will return 1 or 0, indicating whether or not any
> @@ -1146,33 +1051,6 @@ was specified on the command line, the maximum possible inode number in
>  the system will be printed along with its size.
>  .PD
>  .TP
> -.BI "set_encpolicy [ \-c " mode " ] [ \-n " mode " ] [ \-f " flags " ] [ \-v " version " ] [ " keydesc " ]"
> -On filesystems that support encryption, assign an encryption policy to the
> -current file.
> -.I keydesc
> -is a 16-byte hex string which identifies the encryption key to use.
> -If not specified, a "default" key descriptor of all 0's will be used.
> -.RS 1.0i
> -.PD 0
> -.TP 0.4i
> -.BI \-c " mode"
> -contents encryption mode (e.g. AES-256-XTS)
> -.TP
> -.BI \-n " mode"
> -filenames encryption mode (e.g. AES-256-CTS)
> -.TP
> -.BI \-f " flags"
> -policy flags (numeric)
> -.TP
> -.BI \-v " version"
> -version of policy structure (numeric)
> -.RE
> -.PD
> -.TP
> -.BR get_encpolicy
> -On filesystems that support encryption, display the encryption policy of the
> -current file.
> -.TP
>  .BI "scrub " type " [ " agnumber " | " "ino" " " "gen" " ]"
>  Scrub internal XFS filesystem metadata.  The
>  .BI type
> @@ -1191,6 +1069,146 @@ For AG metadata, one AG number must be specified.
>  For file metadata, the repair is applied to the open file unless the
>  inode number and generation number are specified.
>  .TP
> +.BI "label" " " "[ -c | -s " label " ] "
> +On filesystems that support online label manipulation, get, set, or clear the
> +filesystem label.  With no options, print the current filesystem label.  The
> +.B \-c
> +option clears the filesystem label by setting it to the null string.  The
> +.BI "\-s " label
> +option sets the filesystem label to
> +.IR label .
> +If the label is longer than the filesystem will accept,
> +.B xfs_io
> +will print an error message.  XFS filesystem labels can be at most 12
> +characters long.
> +.TP
> +.BI "fsmap [ \-d | \-l | \-r ] [ \-m | \-v ] [ \-n " nx " ] [ " start " ] [ " end " ]
> +Prints the mapping of disk blocks used by the filesystem hosting the current
> +file.  The map lists each extent used by files, allocation group metadata,
> +journalling logs, and static filesystem metadata, as well as any
> +regions that are unused.
> +Each line of the listings takes the following form:
> +.PP
> +.RS
> +.IR extent ": " major ":" minor " [" startblock .. endblock "]: " owner " " startoffset .. endoffset " " length
> +.PP
> +Static filesystem metadata, allocation group metadata, btrees,
> +journalling logs, and free space are marked by replacing the
> +.IR startoffset .. endoffset
> +with the appropriate marker.
> +All blocks, offsets, and lengths are specified in units of 512-byte
> +blocks, no matter what the filesystem's block size is.
> +The optional
> +.I start
> +and
> +.I end
> +arguments can be used to constrain the output to a particular range of
> +disk blocks.
> +If these two options are specified, exactly one of
> +.BR "-d" ", " "-l" ", or " "-r"
> +must also be set.
> +.RE
> +.RS 1.0i
> +.PD 0
> +.TP
> +.BI \-d
> +Display only extents from the data device.
> +This option only applies for XFS filesystems.
> +.TP
> +.BI \-l
> +Display only extents from the external log device.
> +This option only applies to XFS filesystems.
> +.TP
> +.BI \-r
> +Display only extents from the realtime device.
> +This option only applies to XFS filesystems.
> +.TP
> +.BI \-m
> +Display results in a machine readable format (CSV).
> +This option is not compatible with the
> +.B \-v
> +flag.
> +The columns of the output are: extent number, device major, device minor,
> +physical start, physical end, owner, offset start, offset end, length.
> +The start, end, and length numbers are provided in units of 512b.
> +The owner field is a special string that takes the form:
> +
> +.RS 1.0i
> +.PD 0
> +.TP 0.4i
> +.I inode_%lld_data
> +for inode data.
> +.TP
> +.I inode_%lld_data_bmbt
> +for inode data extent maps.
> +.TP
> +.I inode_%lld_attr
> +for inode extended attribute data.
> +.TP
> +.I inode_%lld_attr_bmbt
> +for inode extended attribute extent maps.
> +.TP
> +.I special_%u:%u
> +for other filesystem metadata.
> +.PD
> +.RE
> +
> +.TP
> +.BI \-n " num_extents"
> +If this option is given,
> +.B fsmap
> +obtains the extent list of the file in groups of
> +.I num_extents
> +extents.
> +In the absence of
> +.BR "-n" ", " "fsmap"
> +queries the system for extents in groups of 131,072 records.
> +.TP
> +.B \-v
> +Shows verbose information.
> +When this flag is specified, additional AG specific information is
> +appended to each line in the following form:
> +.IP
> +.RS 1.2i
> +.IR agno " (" startagblock .. endagblock ") " nblocks " " flags
> +.RE
> +.IP
> +A second
> +.B \-v
> +option will print out the
> +.I flags
> +legend.
> +This option is not compatible with the
> +.B \-m
> +flag.
> +.RE
> +.PD
> +
> +
> +.SH OTHER COMMANDS
> +.TP
> +.BR "help [ " command " ]"
> +Display a brief description of one or all commands.
> +.TP
> +.B print
> +Display a list of all open files and memory mapped regions.
> +The current file and current mapping are distinguishable from
> +any others.
> +.TP
> +.B p
> +See the
> +.B print
> +command.
> +.TP
> +.B quit
> +Exit
> +.BR xfs_io .
> +.TP
> +.B q
> +See the
> +.B quit
> +command.
> +.TP
>  .BI "log_writes \-d " device " \-m "  mark
>  Create a mark named
>  .I mark
> @@ -1210,19 +1228,6 @@ See the
>  .B log_writes
>  command.
>  .TP
> -.BI "label" " " "[ -c | -s " label " ] "
> -On filesystems that support online label manipulation, get, set, or clear the
> -filesystem label.  With no options, print the current filesystem label.  The
> -.B \-c
> -option clears the filesystem label by setting it to the null string.  The
> -.BI "\-s " label
> -option sets the filesystem label to
> -.IR label .
> -If the label is longer than the filesystem will accept,
> -.B xfs_io
> -will print an error message.  XFS filesystem labels can be at most 12
> -characters long.
> -.TP
>  .B crc32cselftest
>  Test the internal crc32c implementation to make sure that it computes results
>  correctly.
> 

[PATCH 2/6] xfs_repair: don't error out on dirs with a single leafn block

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

In process_node_dir2, we need to distinguish between a directory with a
single leafn block (yes, they exist) having no interior da nodes, and a
directory with a da tree that incorrectly points to dablk 0.  If we
happened to fill out any part of the da cursor then we have a da btree
with garbage in it; otherwise, we have a single leafn block.

This was found by repair repeatedly rebuilding a directory containing a
single leafn block.

Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
Reported-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/dir2.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)


diff --git a/repair/dir2.c b/repair/dir2.c
index ba5763ed..e67ec590 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -1243,10 +1243,21 @@ process_node_dir2(
 
 	/*
 	 * Skip directories with a root marked XFS_DIR2_LEAFN_MAGIC
+	 *
+	 * Be careful here: If any level of the da cursor was filled out then
+	 * the directory has a da btree containing an invalid before pointer to
+	 * dblock 0, and we should move on to rebuilding the directory.  If no
+	 * levels in the da cursor got filled out, then we just have a single
+	 * leafn block and we're done.
 	 */
 	if (bno == 0) {
-		release_da_cursor(mp, &da_cursor, 0);
-		return 0;
+		if (da_cursor.active > 0) {
+			err_release_da_cursor(mp, &da_cursor, 0);
+			return 1;
+		} else {
+			release_da_cursor(mp, &da_cursor, 0);
+			return 0;
+		}
 	} else {
 		/*
 		 * Now pass cursor and bno into leaf-block processing routine.

[PATCH v2 2/6] xfs_repair: don't error out on dirs with a single leafn block

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

In process_node_dir2, we need to distinguish between a directory with a
single leafn block (yes, they exist) having no interior da nodes, and a
directory with a da tree that incorrectly points to dablk 0.  If we
happened to fill out any part of the da cursor then we have a da btree
with garbage in it; otherwise, we have a single leafn block.

This was found by repair repeatedly rebuilding a directory containing a
single leafn block (xfs/495).

Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
Reported-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: now that the regression test has landed, mention that in the
changelog
---
 repair/dir2.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/repair/dir2.c b/repair/dir2.c
index ba5763ed..e67ec590 100644
--- a/repair/dir2.c
+++ b/repair/dir2.c
@@ -1243,10 +1243,21 @@ process_node_dir2(
 
 	/*
 	 * Skip directories with a root marked XFS_DIR2_LEAFN_MAGIC
+	 *
+	 * Be careful here: If any level of the da cursor was filled out then
+	 * the directory has a da btree containing an invalid before pointer to
+	 * dblock 0, and we should move on to rebuilding the directory.  If no
+	 * levels in the da cursor got filled out, then we just have a single
+	 * leafn block and we're done.
 	 */
 	if (bno == 0) {
-		release_da_cursor(mp, &da_cursor, 0);
-		return 0;
+		if (da_cursor.active > 0) {
+			err_release_da_cursor(mp, &da_cursor, 0);
+			return 1;
+		} else {
+			release_da_cursor(mp, &da_cursor, 0);
+			return 0;
+		}
 	} else {
 		/*
 		 * Now pass cursor and bno into leaf-block processing routine.

Re: [PATCH v2 2/6] xfs_repair: don't error out on dirs with a single leafn block

[Eric Sandeen]

On 11/22/18 12:13 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In process_node_dir2, we need to distinguish between a directory with a
> single leafn block (yes, they exist) having no interior da nodes, and a
> directory with a da tree that incorrectly points to dablk 0.  If we
> happened to fill out any part of the da cursor then we have a da btree
> with garbage in it; otherwise, we have a single leafn block.
> 
> This was found by repair repeatedly rebuilding a directory containing a
> single leafn block (xfs/495).
> 
> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
> Reported-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

I'm finding the commit log hard to parse/understand.

Let's reference

71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"

for starters, but can we do something like this....?

===

As explained in

71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"

a single root LEAFN block can exist in a directory until it grows further.

This is why, normally, we skip directories with a root marked
XFS_DIR2_LEAFN_MAGIC, as detected by the left-most leaf block being 
found at file block 0.

However, if we traversed any level of a btree to get here (as
indicated by da_cursor.active > 0), then a leaf block claiming block
0 indicates corruption, and we should handle it as such, and rebuild
the directory.

This was found by repair repeatedly rebuilding a directory containing a
single leafn block (xfs/495).

Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
Reported-by: Dave Chinner <david@fromorbit.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

===

I'm not hung up on rewriting the commit log, but if you like it give me
thumbs up.  I needed to think it through that way to grok the change.
Anyway, for the change now that I do grok it,

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
> v2: now that the regression test has landed, mention that in the
> changelog
> ---
>  repair/dir2.c |   15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/repair/dir2.c b/repair/dir2.c
> index ba5763ed..e67ec590 100644
> --- a/repair/dir2.c
> +++ b/repair/dir2.c
> @@ -1243,10 +1243,21 @@ process_node_dir2(
>  
>  	/*
>  	 * Skip directories with a root marked XFS_DIR2_LEAFN_MAGIC
> +	 *
> +	 * Be careful here: If any level of the da cursor was filled out then
> +	 * the directory has a da btree containing an invalid before pointer to
> +	 * dblock 0, and we should move on to rebuilding the directory.  If no
> +	 * levels in the da cursor got filled out, then we just have a single
> +	 * leafn block and we're done.
>  	 */
>  	if (bno == 0) {
> -		release_da_cursor(mp, &da_cursor, 0);
> -		return 0;
> +		if (da_cursor.active > 0) {
> +			err_release_da_cursor(mp, &da_cursor, 0);
> +			return 1;
> +		} else {
> +			release_da_cursor(mp, &da_cursor, 0);
> +			return 0;
> +		}
>  	} else {
>  		/*
>  		 * Now pass cursor and bno into leaf-block processing routine.
> 

Re: [PATCH v2 2/6] xfs_repair: don't error out on dirs with a single leafn block

[Eric Sandeen]

On 12/4/18 9:48 PM, Eric Sandeen wrote:
> On 11/22/18 12:13 PM, Darrick J. Wong wrote:
>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>
>> In process_node_dir2, we need to distinguish between a directory with a
>> single leafn block (yes, they exist) having no interior da nodes, and a
>> directory with a da tree that incorrectly points to dablk 0.  If we
>> happened to fill out any part of the da cursor then we have a da btree
>> with garbage in it; otherwise, we have a single leafn block.
>>
>> This was found by repair repeatedly rebuilding a directory containing a
>> single leafn block (xfs/495).
>>
>> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
>> Reported-by: Dave Chinner <david@fromorbit.com>
>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> I'm finding the commit log hard to parse/understand.
> 
> Let's reference
> 
> 71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"
> 
> for starters, but can we do something like this....?
> 
> ===
> 
> As explained in
> 
> 71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"
> 
> a single root LEAFN block can exist in a directory until it grows further.
> 
> This is why, normally, we skip directories with a root marked
> XFS_DIR2_LEAFN_MAGIC, as detected by the left-most leaf block being 
> found at file block 0.
> 
> However, if we traversed any level of a btree to get here (as
> indicated by da_cursor.active > 0), then a leaf block claiming block
> 0 indicates corruption, and we should handle it as such, and rebuild
> the directory.
> 
> This was found by repair repeatedly rebuilding a directory containing a
> single leafn block (xfs/495).
> 
> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
> Reported-by: Dave Chinner <david@fromorbit.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> 
> ===
> 
> I'm not hung up on rewriting the commit log, but if you like it give me
> thumbs up.  I needed to think it through that way to grok the change.
> Anyway, for the change now that I do grok it,
> 
> Reviewed-by: Eric Sandeen <sandeen@redhat.com>

Uh, the summary is wrong too,  o?  We /already/ "don't error out"
on dirs with a single leafn block.  Maybe:

xfs_repair: rebuild directory when non-root leafn blocks claim block 0

?

Re: [PATCH v2 2/6] xfs_repair: don't error out on dirs with a single leafn block

[Darrick J. Wong]

On Tue, Dec 04, 2018 at 10:11:50PM -0600, Eric Sandeen wrote:
> 
> 
> On 12/4/18 9:48 PM, Eric Sandeen wrote:
> > On 11/22/18 12:13 PM, Darrick J. Wong wrote:
> >> From: Darrick J. Wong <darrick.wong@oracle.com>
> >>
> >> In process_node_dir2, we need to distinguish between a directory with a
> >> single leafn block (yes, they exist) having no interior da nodes, and a
> >> directory with a da tree that incorrectly points to dablk 0.  If we
> >> happened to fill out any part of the da cursor then we have a da btree
> >> with garbage in it; otherwise, we have a single leafn block.
> >>
> >> This was found by repair repeatedly rebuilding a directory containing a
> >> single leafn block (xfs/495).
> >>
> >> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
> >> Reported-by: Dave Chinner <david@fromorbit.com>
> >> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > I'm finding the commit log hard to parse/understand.
> > 
> > Let's reference
> > 
> > 71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"
> > 
> > for starters, but can we do something like this....?
> > 
> > ===
> > 
> > As explained in
> > 
> > 71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"
> > 
> > a single root LEAFN block can exist in a directory until it grows further.
> > 
> > This is why, normally, we skip directories with a root marked
> > XFS_DIR2_LEAFN_MAGIC, as detected by the left-most leaf block being 
> > found at file block 0.
> > 
> > However, if we traversed any level of a btree to get here (as
> > indicated by da_cursor.active > 0), then a leaf block claiming block
> > 0 indicates corruption, and we should handle it as such, and rebuild
> > the directory.
> > 
> > This was found by repair repeatedly rebuilding a directory containing a
> > single leafn block (xfs/495).
> > 
> > Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
> > Reported-by: Dave Chinner <david@fromorbit.com>
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > ===
> > 
> > I'm not hung up on rewriting the commit log, but if you like it give me
> > thumbs up.  I needed to think it through that way to grok the change.
> > Anyway, for the change now that I do grok it,
> > 
> > Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> Uh, the summary is wrong too,  o?  We /already/ "don't error out"
> on dirs with a single leafn block.  Maybe:
> 
> xfs_repair: rebuild directory when non-root leafn blocks claim block 0
> 
> ?

Works for me.  Want a resend or are you fixing it up on the way in?

--D

Re: [PATCH v2 2/6] xfs_repair: don't error out on dirs with a single leafn block

[Eric Sandeen]

On 12/5/18 10:48 AM, Darrick J. Wong wrote:
> On Tue, Dec 04, 2018 at 10:11:50PM -0600, Eric Sandeen wrote:
>>
>>
>> On 12/4/18 9:48 PM, Eric Sandeen wrote:
>>> On 11/22/18 12:13 PM, Darrick J. Wong wrote:
>>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>>
>>>> In process_node_dir2, we need to distinguish between a directory with a
>>>> single leafn block (yes, they exist) having no interior da nodes, and a
>>>> directory with a da tree that incorrectly points to dablk 0.  If we
>>>> happened to fill out any part of the da cursor then we have a da btree
>>>> with garbage in it; otherwise, we have a single leafn block.
>>>>
>>>> This was found by repair repeatedly rebuilding a directory containing a
>>>> single leafn block (xfs/495).
>>>>
>>>> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
>>>> Reported-by: Dave Chinner <david@fromorbit.com>
>>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> I'm finding the commit log hard to parse/understand.
>>>
>>> Let's reference
>>>
>>> 71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"
>>>
>>> for starters, but can we do something like this....?
>>>
>>> ===
>>>
>>> As explained in
>>>
>>> 71a6af8 Revert "xfs_repair: treat zero da btree pointers as corruption"
>>>
>>> a single root LEAFN block can exist in a directory until it grows further.
>>>
>>> This is why, normally, we skip directories with a root marked
>>> XFS_DIR2_LEAFN_MAGIC, as detected by the left-most leaf block being 
>>> found at file block 0.
>>>
>>> However, if we traversed any level of a btree to get here (as
>>> indicated by da_cursor.active > 0), then a leaf block claiming block
>>> 0 indicates corruption, and we should handle it as such, and rebuild
>>> the directory.
>>>
>>> This was found by repair repeatedly rebuilding a directory containing a
>>> single leafn block (xfs/495).
>>>
>>> Fixes: 67a79e2cc932 ("xfs_repair: treat zero da btree pointers as corruption")
>>> Reported-by: Dave Chinner <david@fromorbit.com>
>>> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> ===
>>>
>>> I'm not hung up on rewriting the commit log, but if you like it give me
>>> thumbs up.  I needed to think it through that way to grok the change.
>>> Anyway, for the change now that I do grok it,
>>>
>>> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
>>
>> Uh, the summary is wrong too,  o?  We /already/ "don't error out"
>> on dirs with a single leafn block.  Maybe:
>>
>> xfs_repair: rebuild directory when non-root leafn blocks claim block 0
>>
>> ?
> 
> Works for me.  Want a resend or are you fixing it up on the way in?

I'll do it.

-Eric

[PATCH 3/6] xfs_repair: skip block reservation when fixing freelist

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

AGFL blocks are considered to be part of the fdblocks count, so there's
no need to obtain a block reservation when fixing the AGFL as part of
repair.  Asking for a reservation can cause repair to fail if the
superblock claims zero fdblocks because we haven't gotten far enough
into phase 5 to have reset the superblock counters.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 repair/rmap.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)


diff --git a/repair/rmap.c b/repair/rmap.c
index ebb5a3ad..c5a86646 100644
--- a/repair/rmap.c
+++ b/repair/rmap.c
@@ -1374,8 +1374,7 @@ fix_freelist(
 	args.agno = agno;
 	args.alignment = 1;
 	args.pag = libxfs_perag_get(mp, agno);
-	error = -libxfs_trans_alloc_rollable(mp,
-			libxfs_alloc_min_freelist(mp, args.pag), &tp);
+	error = -libxfs_trans_alloc_rollable(mp, 0, &tp);
 	if (error)
 		do_error(_("failed to fix AGFL on AG %d, error %d\n"),
 				agno, error);

Re: [PATCH 3/6] xfs_repair: skip block reservation when fixing freelist

[Allison Henderson]

On 11/9/18 5:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> AGFL blocks are considered to be part of the fdblocks count, so there's
> no need to obtain a block reservation when fixing the AGFL as part of
> repair.  Asking for a reservation can cause repair to fail if the
> superblock claims zero fdblocks because we haven't gotten far enough
> into phase 5 to have reset the superblock counters.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   repair/rmap.c |    3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> diff --git a/repair/rmap.c b/repair/rmap.c
> index ebb5a3ad..c5a86646 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -1374,8 +1374,7 @@ fix_freelist(
>   	args.agno = agno;
>   	args.alignment = 1;
>   	args.pag = libxfs_perag_get(mp, agno);
> -	error = -libxfs_trans_alloc_rollable(mp,
> -			libxfs_alloc_min_freelist(mp, args.pag), &tp);
> +	error = -libxfs_trans_alloc_rollable(mp, 0, &tp);
>   	if (error)
>   		do_error(_("failed to fix AGFL on AG %d, error %d\n"),
>   				agno, error);
> 

Ok, you can add my review:
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

Re: [PATCH 3/6] xfs_repair: skip block reservation when fixing freelist

[Eric Sandeen]

On 11/9/18 6:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> AGFL blocks are considered to be part of the fdblocks count, so there's
> no need to obtain a block reservation when fixing the AGFL as part of
> repair.  Asking for a reservation can cause repair to fail if the
> superblock claims zero fdblocks because we haven't gotten far enough
> into phase 5 to have reset the superblock counters.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>

OK, makes sense to me.

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> ---
>  repair/rmap.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> 
> diff --git a/repair/rmap.c b/repair/rmap.c
> index ebb5a3ad..c5a86646 100644
> --- a/repair/rmap.c
> +++ b/repair/rmap.c
> @@ -1374,8 +1374,7 @@ fix_freelist(
>  	args.agno = agno;
>  	args.alignment = 1;
>  	args.pag = libxfs_perag_get(mp, agno);
> -	error = -libxfs_trans_alloc_rollable(mp,
> -			libxfs_alloc_min_freelist(mp, args.pag), &tp);
> +	error = -libxfs_trans_alloc_rollable(mp, 0, &tp);
>  	if (error)
>  		do_error(_("failed to fix AGFL on AG %d, error %d\n"),
>  				agno, error);
> 

[PATCH 4/6] xfs_scrub: handle totally empty inode chunks

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

We try to load a single inobt record with each FSINUMBERS call.  If the
chunk is totally empty (which can happen when there are more than one
inobt record per block) we should skip to the next INUMBERS call since
there are no inodes to bulkstat.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/inodes.c |    6 ++++++
 1 file changed, 6 insertions(+)


diff --git a/scrub/inodes.c b/scrub/inodes.c
index 693f21ec..442a5978 100644
--- a/scrub/inodes.c
+++ b/scrub/inodes.c
@@ -139,6 +139,12 @@ xfs_iterate_inodes_range(
 		/* Load the inodes. */
 		ino = inogrp.xi_startino - 1;
 		bulkreq.icount = inogrp.xi_alloccount;
+		/*
+		 * We can have totally empty inode chunks on filesystems where
+		 * there are more than 64 inodes per block.  Skip these.
+		 */
+		if (inogrp.xi_alloccount == 0)
+			goto igrp_retry;
 		error = ioctl(ctx->mnt_fd, XFS_IOC_FSBULKSTAT, &bulkreq);
 		if (error)
 			str_info(ctx, descr, "%s", strerror_r(errno,

Re: [PATCH 4/6] xfs_scrub: handle totally empty inode chunks

[Allison Henderson]

On 11/9/18 5:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> We try to load a single inobt record with each FSINUMBERS call.  If the
> chunk is totally empty (which can happen when there are more than one
> inobt record per block) we should skip to the next INUMBERS call since
> there are no inodes to bulkstat.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   scrub/inodes.c |    6 ++++++
>   1 file changed, 6 insertions(+)
> 
> 
> diff --git a/scrub/inodes.c b/scrub/inodes.c
> index 693f21ec..442a5978 100644
> --- a/scrub/inodes.c
> +++ b/scrub/inodes.c
> @@ -139,6 +139,12 @@ xfs_iterate_inodes_range(
>   		/* Load the inodes. */
>   		ino = inogrp.xi_startino - 1;
>   		bulkreq.icount = inogrp.xi_alloccount;
> +		/*
> +		 * We can have totally empty inode chunks on filesystems where
> +		 * there are more than 64 inodes per block.  Skip these.
> +		 */
> +		if (inogrp.xi_alloccount == 0)
> +			goto igrp_retry;
>   		error = ioctl(ctx->mnt_fd, XFS_IOC_FSBULKSTAT, &bulkreq);
>   		if (error)
>   			str_info(ctx, descr, "%s", strerror_r(errno,
> 

Looks ok to me.

Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

Re: [PATCH 4/6] xfs_scrub: handle totally empty inode chunks

[Eric Sandeen]

On 11/9/18 6:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> We try to load a single inobt record with each FSINUMBERS call.  If the
> chunk is totally empty (which can happen when there are more than one
> inobt record per block) we should skip to the next INUMBERS call since
> there are no inodes to bulkstat.

It's not clear to me if this is a bugfix or an optimization, but meh,
seems like the right thing to do :)

Reviewed-by: Eric Sandeen <sandeen@redhat.com>

> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  scrub/inodes.c |    6 ++++++
>  1 file changed, 6 insertions(+)
> 
> 
> diff --git a/scrub/inodes.c b/scrub/inodes.c
> index 693f21ec..442a5978 100644
> --- a/scrub/inodes.c
> +++ b/scrub/inodes.c
> @@ -139,6 +139,12 @@ xfs_iterate_inodes_range(
>  		/* Load the inodes. */
>  		ino = inogrp.xi_startino - 1;
>  		bulkreq.icount = inogrp.xi_alloccount;
> +		/*
> +		 * We can have totally empty inode chunks on filesystems where
> +		 * there are more than 64 inodes per block.  Skip these.
> +		 */
> +		if (inogrp.xi_alloccount == 0)
> +			goto igrp_retry;
>  		error = ioctl(ctx->mnt_fd, XFS_IOC_FSBULKSTAT, &bulkreq);
>  		if (error)
>  			str_info(ctx, descr, "%s", strerror_r(errno,
> 

Re: [PATCH 4/6] xfs_scrub: handle totally empty inode chunks

[Darrick J. Wong]

On Wed, Nov 28, 2018 at 06:16:37PM -0600, Eric Sandeen wrote:
> On 11/9/18 6:45 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > We try to load a single inobt record with each FSINUMBERS call.  If the
> > chunk is totally empty (which can happen when there are more than one
> > inobt record per block) we should skip to the next INUMBERS call since
> > there are no inodes to bulkstat.
> 
> It's not clear to me if this is a bugfix or an optimization, but meh,
> seems like the right thing to do :)

It's a bug fix for 64k-block filesystems, which can have multiple empty
inobt records that map to a single 64k block; if we try to BULKSTAT an
empty inobt group then we get zero inodes and scrub erroneously says the
fs is broken when in reality it's just stupid.

--D

> Reviewed-by: Eric Sandeen <sandeen@redhat.com>
> 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >  scrub/inodes.c |    6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > 
> > diff --git a/scrub/inodes.c b/scrub/inodes.c
> > index 693f21ec..442a5978 100644
> > --- a/scrub/inodes.c
> > +++ b/scrub/inodes.c
> > @@ -139,6 +139,12 @@ xfs_iterate_inodes_range(
> >  		/* Load the inodes. */
> >  		ino = inogrp.xi_startino - 1;
> >  		bulkreq.icount = inogrp.xi_alloccount;
> > +		/*
> > +		 * We can have totally empty inode chunks on filesystems where
> > +		 * there are more than 64 inodes per block.  Skip these.
> > +		 */
> > +		if (inogrp.xi_alloccount == 0)
> > +			goto igrp_retry;
> >  		error = ioctl(ctx->mnt_fd, XFS_IOC_FSBULKSTAT, &bulkreq);
> >  		if (error)
> >  			str_info(ctx, descr, "%s", strerror_r(errno,
> > 

Re: [PATCH 4/6] xfs_scrub: handle totally empty inode chunks

[Eric Sandeen]

On 11/28/18 6:35 PM, Darrick J. Wong wrote:
> On Wed, Nov 28, 2018 at 06:16:37PM -0600, Eric Sandeen wrote:
>> On 11/9/18 6:45 PM, Darrick J. Wong wrote:
>>> From: Darrick J. Wong <darrick.wong@oracle.com>
>>>
>>> We try to load a single inobt record with each FSINUMBERS call.  If the
>>> chunk is totally empty (which can happen when there are more than one
>>> inobt record per block) we should skip to the next INUMBERS call since
>>> there are no inodes to bulkstat.
>>
>> It's not clear to me if this is a bugfix or an optimization, but meh,
>> seems like the right thing to do :)
> 
> It's a bug fix for 64k-block filesystems, which can have multiple empty
> inobt records that map to a single 64k block; if we try to BULKSTAT an
> empty inobt group then we get zero inodes and scrub erroneously says the
> fs is broken when in reality it's just stupid.

Ok thanks, didn't dig into whether it did needless no-op calls, or something
worse.

-eric

[PATCH 5/6] xfs_scrub: fix fractional reporting of single inodes

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

When there are fewer than 1024 inodes in the filesystem, scrub reports
fractional inodes in its final report:

35.2MiB data used;  5.0 inodes used.
34.2MiB data found; 5.0 inodes found.
5.0 inodes counted; 5.0 inodes checked.

Inodes are indivisible, so only report the fractional part when we have
a large enough number of inodes to perform a unit conversion:

35.2MiB data used;  5 inodes used.
34.2MiB data found; 5 inodes found.
5 inodes counted; 5 inodes checked.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/common.c |    5 ++++-
 scrub/common.h |    2 +-
 scrub/phase7.c |   34 ++++++++++++++++++----------------
 3 files changed, 23 insertions(+), 18 deletions(-)


diff --git a/scrub/common.c b/scrub/common.c
index 96b86a26..78afc4bf 100644
--- a/scrub/common.c
+++ b/scrub/common.c
@@ -200,10 +200,12 @@ auto_space_units(
 double
 auto_units(
 	unsigned long long	number,
-	char			**units)
+	char			**units,
+	int			*precision)
 {
 	if (debug > 1)
 		goto no_prefix;
+	*precision = 1;
 	if (number > 1000000000000ULL) {
 		*units = "T";
 		return number / 1000000000000.0;
@@ -220,6 +222,7 @@ auto_units(
 
 no_prefix:
 	*units = "";
+	*precision = 0;
 	return number;
 }
 
diff --git a/scrub/common.h b/scrub/common.h
index 5a43ac0c..e85a0333 100644
--- a/scrub/common.h
+++ b/scrub/common.h
@@ -62,7 +62,7 @@ debug_tweak_on(
 
 double timeval_subtract(struct timeval *tv1, struct timeval *tv2);
 double auto_space_units(unsigned long long kilobytes, char **units);
-double auto_units(unsigned long long number, char **units);
+double auto_units(unsigned long long number, char **units, int *precision);
 unsigned int scrub_nproc(struct scrub_ctx *ctx);
 unsigned int scrub_nproc_workqueue(struct scrub_ctx *ctx);
 
diff --git a/scrub/phase7.c b/scrub/phase7.c
index cac0c75a..504a6927 100644
--- a/scrub/phase7.c
+++ b/scrub/phase7.c
@@ -107,6 +107,7 @@ xfs_scan_summary(
 	unsigned long long		f_free;
 	bool				moveon;
 	bool				complain;
+	int				ip;
 	int				error;
 
 	/* Flush everything out to disk before we start counting. */
@@ -176,27 +177,27 @@ xfs_scan_summary(
 		if (used_rt || stat_rt) {
 			d = auto_space_units(used_data, &du);
 			r = auto_space_units(used_rt, &ru);
-			i = auto_units(used_files, &iu);
+			i = auto_units(used_files, &iu, &ip);
 			fprintf(stdout,
-_("%.1f%s data used;  %.1f%s realtime data used;  %.2f%s inodes used.\n"),
-					d, du, r, ru, i, iu);
+_("%.1f%s data used;  %.1f%s realtime data used;  %.*f%s inodes used.\n"),
+					d, du, r, ru, ip, i, iu);
 			d = auto_space_units(stat_data, &du);
 			r = auto_space_units(stat_rt, &ru);
-			i = auto_units(counted_inodes, &iu);
+			i = auto_units(counted_inodes, &iu, &ip);
 			fprintf(stdout,
-_("%.1f%s data found; %.1f%s realtime data found; %.2f%s inodes found.\n"),
-					d, du, r, ru, i, iu);
+_("%.1f%s data found; %.1f%s realtime data found; %.*f%s inodes found.\n"),
+					d, du, r, ru, ip, i, iu);
 		} else {
 			d = auto_space_units(used_data, &du);
-			i = auto_units(used_files, &iu);
+			i = auto_units(used_files, &iu, &ip);
 			fprintf(stdout,
-_("%.1f%s data used;  %.1f%s inodes used.\n"),
-					d, du, i, iu);
+_("%.1f%s data used;  %.*f%s inodes used.\n"),
+					d, du, ip, i, iu);
 			d = auto_space_units(stat_data, &du);
-			i = auto_units(counted_inodes, &iu);
+			i = auto_units(counted_inodes, &iu, &ip);
 			fprintf(stdout,
-_("%.1f%s data found; %.1f%s inodes found.\n"),
-					d, du, i, iu);
+_("%.1f%s data found; %.*f%s inodes found.\n"),
+					d, du, ip, i, iu);
 		}
 		fflush(stdout);
 	}
@@ -210,12 +211,13 @@ _("%.1f%s data found; %.1f%s inodes found.\n"),
 			_("checked inodes"))) {
 		double		i1, i2;
 		char		*i1u, *i2u;
+		int		i1p, i2p;
 
-		i1 = auto_units(counted_inodes, &i1u);
-		i2 = auto_units(ctx->inodes_checked, &i2u);
+		i1 = auto_units(counted_inodes, &i1u, &i1p);
+		i2 = auto_units(ctx->inodes_checked, &i2u, &i2p);
 		fprintf(stdout,
-_("%.1f%s inodes counted; %.1f%s inodes checked.\n"),
-				i1, i1u, i2, i2u);
+_("%.*f%s inodes counted; %.*f%s inodes checked.\n"),
+				i1p, i1, i1u, i2p, i2, i2u);
 		fflush(stdout);
 	}
 

Re: [PATCH 5/6] xfs_scrub: fix fractional reporting of single inodes

[Allison Henderson]

On 11/9/18 5:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When there are fewer than 1024 inodes in the filesystem, scrub reports
> fractional inodes in its final report:
> 
> 35.2MiB data used;  5.0 inodes used.
> 34.2MiB data found; 5.0 inodes found.
> 5.0 inodes counted; 5.0 inodes checked.
> 
> Inodes are indivisible, so only report the fractional part when we have
> a large enough number of inodes to perform a unit conversion:
> 
> 35.2MiB data used;  5 inodes used.
> 34.2MiB data found; 5 inodes found.
> 5 inodes counted; 5 inodes checked.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   scrub/common.c |    5 ++++-
>   scrub/common.h |    2 +-
>   scrub/phase7.c |   34 ++++++++++++++++++----------------
>   3 files changed, 23 insertions(+), 18 deletions(-)
> 
> 
> diff --git a/scrub/common.c b/scrub/common.c
> index 96b86a26..78afc4bf 100644
> --- a/scrub/common.c
> +++ b/scrub/common.c
> @@ -200,10 +200,12 @@ auto_space_units(
>   double
>   auto_units(
>   	unsigned long long	number,
> -	char			**units)
> +	char			**units,
> +	int			*precision)
>   {
>   	if (debug > 1)
>   		goto no_prefix;
> +	*precision = 1;
>   	if (number > 1000000000000ULL) {
>   		*units = "T";
>   		return number / 1000000000000.0;
> @@ -220,6 +222,7 @@ auto_units(
>   
>   no_prefix:
>   	*units = "";
> +	*precision = 0;
>   	return number;
>   }
>   
> diff --git a/scrub/common.h b/scrub/common.h
> index 5a43ac0c..e85a0333 100644
> --- a/scrub/common.h
> +++ b/scrub/common.h
> @@ -62,7 +62,7 @@ debug_tweak_on(
>   
>   double timeval_subtract(struct timeval *tv1, struct timeval *tv2);
>   double auto_space_units(unsigned long long kilobytes, char **units);
> -double auto_units(unsigned long long number, char **units);
> +double auto_units(unsigned long long number, char **units, int *precision);
>   unsigned int scrub_nproc(struct scrub_ctx *ctx);
>   unsigned int scrub_nproc_workqueue(struct scrub_ctx *ctx);
>   
> diff --git a/scrub/phase7.c b/scrub/phase7.c
> index cac0c75a..504a6927 100644
> --- a/scrub/phase7.c
> +++ b/scrub/phase7.c
> @@ -107,6 +107,7 @@ xfs_scan_summary(
>   	unsigned long long		f_free;
>   	bool				moveon;
>   	bool				complain;
> +	int				ip;
>   	int				error;
>   
>   	/* Flush everything out to disk before we start counting. */
> @@ -176,27 +177,27 @@ xfs_scan_summary(
>   		if (used_rt || stat_rt) {
>   			d = auto_space_units(used_data, &du);
>   			r = auto_space_units(used_rt, &ru);
> -			i = auto_units(used_files, &iu);
> +			i = auto_units(used_files, &iu, &ip);
>   			fprintf(stdout,
> -_("%.1f%s data used;  %.1f%s realtime data used;  %.2f%s inodes used.\n"),
> -					d, du, r, ru, i, iu);
> +_("%.1f%s data used;  %.1f%s realtime data used;  %.*f%s inodes used.\n"),
> +					d, du, r, ru, ip, i, iu);
>   			d = auto_space_units(stat_data, &du);
>   			r = auto_space_units(stat_rt, &ru);
> -			i = auto_units(counted_inodes, &iu);
> +			i = auto_units(counted_inodes, &iu, &ip);
>   			fprintf(stdout,
> -_("%.1f%s data found; %.1f%s realtime data found; %.2f%s inodes found.\n"),
> -					d, du, r, ru, i, iu);
> +_("%.1f%s data found; %.1f%s realtime data found; %.*f%s inodes found.\n"),
> +					d, du, r, ru, ip, i, iu);

Just a suggestion:
Is plumbing in the extra precision parameter really preferable to just 
doing something like (i == (int)i ? 0 : 1)  Or maybe a precision macro 
or something.

Otherwise looks fine.

Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
>   		} else {
>   			d = auto_space_units(used_data, &du);
> -			i = auto_units(used_files, &iu);
> +			i = auto_units(used_files, &iu, &ip);
>   			fprintf(stdout,
> -_("%.1f%s data used;  %.1f%s inodes used.\n"),
> -					d, du, i, iu);
> +_("%.1f%s data used;  %.*f%s inodes used.\n"),
> +					d, du, ip, i, iu);
>   			d = auto_space_units(stat_data, &du);
> -			i = auto_units(counted_inodes, &iu);
> +			i = auto_units(counted_inodes, &iu, &ip);
>   			fprintf(stdout,
> -_("%.1f%s data found; %.1f%s inodes found.\n"),
> -					d, du, i, iu);
> +_("%.1f%s data found; %.*f%s inodes found.\n"),
> +					d, du, ip, i, iu);
>   		}
>   		fflush(stdout);
>   	}
> @@ -210,12 +211,13 @@ _("%.1f%s data found; %.1f%s inodes found.\n"),
>   			_("checked inodes"))) {
>   		double		i1, i2;
>   		char		*i1u, *i2u;
> +		int		i1p, i2p;
>   
> -		i1 = auto_units(counted_inodes, &i1u);
> -		i2 = auto_units(ctx->inodes_checked, &i2u);
> +		i1 = auto_units(counted_inodes, &i1u, &i1p);
> +		i2 = auto_units(ctx->inodes_checked, &i2u, &i2p);
>   		fprintf(stdout,
> -_("%.1f%s inodes counted; %.1f%s inodes checked.\n"),
> -				i1, i1u, i2, i2u);
> +_("%.*f%s inodes counted; %.*f%s inodes checked.\n"),
> +				i1p, i1, i1u, i2p, i2, i2u);
>   		fflush(stdout);
>   	}
>   
> 

Re: [PATCH 5/6] xfs_scrub: fix fractional reporting of single inodes

[Darrick J. Wong]

On Sat, Nov 10, 2018 at 12:15:49AM -0700, Allison Henderson wrote:
> 
> 
> On 11/9/18 5:45 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > When there are fewer than 1024 inodes in the filesystem, scrub reports
> > fractional inodes in its final report:
> > 
> > 35.2MiB data used;  5.0 inodes used.
> > 34.2MiB data found; 5.0 inodes found.
> > 5.0 inodes counted; 5.0 inodes checked.
> > 
> > Inodes are indivisible, so only report the fractional part when we have
> > a large enough number of inodes to perform a unit conversion:
> > 
> > 35.2MiB data used;  5 inodes used.
> > 34.2MiB data found; 5 inodes found.
> > 5 inodes counted; 5 inodes checked.
> > 
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> > ---
> >   scrub/common.c |    5 ++++-
> >   scrub/common.h |    2 +-
> >   scrub/phase7.c |   34 ++++++++++++++++++----------------
> >   3 files changed, 23 insertions(+), 18 deletions(-)
> > 
> > 
> > diff --git a/scrub/common.c b/scrub/common.c
> > index 96b86a26..78afc4bf 100644
> > --- a/scrub/common.c
> > +++ b/scrub/common.c
> > @@ -200,10 +200,12 @@ auto_space_units(
> >   double
> >   auto_units(
> >   	unsigned long long	number,
> > -	char			**units)
> > +	char			**units,
> > +	int			*precision)
> >   {
> >   	if (debug > 1)
> >   		goto no_prefix;
> > +	*precision = 1;
> >   	if (number > 1000000000000ULL) {
> >   		*units = "T";
> >   		return number / 1000000000000.0;
> > @@ -220,6 +222,7 @@ auto_units(
> >   no_prefix:
> >   	*units = "";
> > +	*precision = 0;
> >   	return number;
> >   }
> > diff --git a/scrub/common.h b/scrub/common.h
> > index 5a43ac0c..e85a0333 100644
> > --- a/scrub/common.h
> > +++ b/scrub/common.h
> > @@ -62,7 +62,7 @@ debug_tweak_on(
> >   double timeval_subtract(struct timeval *tv1, struct timeval *tv2);
> >   double auto_space_units(unsigned long long kilobytes, char **units);
> > -double auto_units(unsigned long long number, char **units);
> > +double auto_units(unsigned long long number, char **units, int *precision);
> >   unsigned int scrub_nproc(struct scrub_ctx *ctx);
> >   unsigned int scrub_nproc_workqueue(struct scrub_ctx *ctx);
> > diff --git a/scrub/phase7.c b/scrub/phase7.c
> > index cac0c75a..504a6927 100644
> > --- a/scrub/phase7.c
> > +++ b/scrub/phase7.c
> > @@ -107,6 +107,7 @@ xfs_scan_summary(
> >   	unsigned long long		f_free;
> >   	bool				moveon;
> >   	bool				complain;
> > +	int				ip;
> >   	int				error;
> >   	/* Flush everything out to disk before we start counting. */
> > @@ -176,27 +177,27 @@ xfs_scan_summary(
> >   		if (used_rt || stat_rt) {
> >   			d = auto_space_units(used_data, &du);
> >   			r = auto_space_units(used_rt, &ru);
> > -			i = auto_units(used_files, &iu);
> > +			i = auto_units(used_files, &iu, &ip);
> >   			fprintf(stdout,
> > -_("%.1f%s data used;  %.1f%s realtime data used;  %.2f%s inodes used.\n"),
> > -					d, du, r, ru, i, iu);
> > +_("%.1f%s data used;  %.1f%s realtime data used;  %.*f%s inodes used.\n"),
> > +					d, du, r, ru, ip, i, iu);
> >   			d = auto_space_units(stat_data, &du);
> >   			r = auto_space_units(stat_rt, &ru);
> > -			i = auto_units(counted_inodes, &iu);
> > +			i = auto_units(counted_inodes, &iu, &ip);
> >   			fprintf(stdout,
> > -_("%.1f%s data found; %.1f%s realtime data found; %.2f%s inodes found.\n"),
> > -					d, du, r, ru, i, iu);
> > +_("%.1f%s data found; %.1f%s realtime data found; %.*f%s inodes found.\n"),
> > +					d, du, r, ru, ip, i, iu);
> 
> Just a suggestion:
> Is plumbing in the extra precision parameter really preferable to just doing
> something like (i == (int)i ? 0 : 1)  Or maybe a precision macro or
> something.

auto_units() is supposed to turn a big number into something more
human-friendly for display, so it's in charge of figuring out all the
formatting stuff (label, precision, unit converted number).  Better to
keep all that in a single function than open-code it everywhere.

--D

> 
> Otherwise looks fine.
> 
> Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
> >   		} else {
> >   			d = auto_space_units(used_data, &du);
> > -			i = auto_units(used_files, &iu);
> > +			i = auto_units(used_files, &iu, &ip);
> >   			fprintf(stdout,
> > -_("%.1f%s data used;  %.1f%s inodes used.\n"),
> > -					d, du, i, iu);
> > +_("%.1f%s data used;  %.*f%s inodes used.\n"),
> > +					d, du, ip, i, iu);
> >   			d = auto_space_units(stat_data, &du);
> > -			i = auto_units(counted_inodes, &iu);
> > +			i = auto_units(counted_inodes, &iu, &ip);
> >   			fprintf(stdout,
> > -_("%.1f%s data found; %.1f%s inodes found.\n"),
> > -					d, du, i, iu);
> > +_("%.1f%s data found; %.*f%s inodes found.\n"),
> > +					d, du, ip, i, iu);
> >   		}
> >   		fflush(stdout);
> >   	}
> > @@ -210,12 +211,13 @@ _("%.1f%s data found; %.1f%s inodes found.\n"),
> >   			_("checked inodes"))) {
> >   		double		i1, i2;
> >   		char		*i1u, *i2u;
> > +		int		i1p, i2p;
> > -		i1 = auto_units(counted_inodes, &i1u);
> > -		i2 = auto_units(ctx->inodes_checked, &i2u);
> > +		i1 = auto_units(counted_inodes, &i1u, &i1p);
> > +		i2 = auto_units(ctx->inodes_checked, &i2u, &i2p);
> >   		fprintf(stdout,
> > -_("%.1f%s inodes counted; %.1f%s inodes checked.\n"),
> > -				i1, i1u, i2, i2u);
> > +_("%.*f%s inodes counted; %.*f%s inodes checked.\n"),
> > +				i1p, i1, i1u, i2p, i2, i2u);
> >   		fflush(stdout);
> >   	}
> > 

[PATCH 6/6] xfs_scrub: move everything to /usr/sbin

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

Recently, it was pointed out that xfs_scrub{,_all} depend on components
and libraries (libicu, python) that live in /usr.  /sbin binaries
shouldn't depend on /usr, so let's move the scrub binaries to /usr/sbin.

Reported-by: xfs@tlinx.org
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 scrub/Makefile |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)


diff --git a/scrub/Makefile b/scrub/Makefile
index aebc7e43..c6473c12 100644
--- a/scrub/Makefile
+++ b/scrub/Makefile
@@ -103,7 +103,7 @@ default: depend $(LTCOMMAND) $(XFS_SCRUB_ALL_PROG) $(OPTIONAL_TARGETS)
 
 xfs_scrub_all: xfs_scrub_all.in
 	@echo "    [SED]    $@"
-	$(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \
+	$(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \
 		   -e "s|@pkg_version@|$(PKG_VERSION)|g" \
 		   -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" < $< > $@
 	$(Q)chmod a+x $@
@@ -116,14 +116,14 @@ install: $(INSTALL_SCRUB)
 
 %.service: %.service.in
 	@echo "    [SED]    $@"
-	$(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \
+	$(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \
 		   -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" \
 		   -e "s|@pkg_lib_dir@|$(PKG_LIB_DIR)|g" \
 		   -e "s|@pkg_name@|$(PKG_NAME)|g" < $< > $@
 
 %.cron: %.cron.in
 	@echo "    [SED]    $@"
-	$(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" < $< > $@
+	$(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" < $< > $@
 
 install-systemd: default $(SYSTEMD_SERVICES)
 	$(INSTALL) -m 755 -d $(SYSTEMD_SYSTEM_UNIT_DIR)
@@ -136,9 +136,9 @@ install-crond: default $(CRONTABS)
 	$(INSTALL) -m 644 $(CRONTABS) $(CROND_DIR)
 
 install-scrub: default
-	$(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR)
-	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR)
-	$(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_ROOT_SBIN_DIR)
+	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
+	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
+	$(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_SBIN_DIR)
 
 install-dev:
 

Re: [PATCH 6/6] xfs_scrub: move everything to /usr/sbin

[Allison Henderson]

Looks ok to me.  You can add my review:

Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

On 11/9/18 5:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Recently, it was pointed out that xfs_scrub{,_all} depend on components
> and libraries (libicu, python) that live in /usr.  /sbin binaries
> shouldn't depend on /usr, so let's move the scrub binaries to /usr/sbin.
> 
> Reported-by: xfs@tlinx.org
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>   scrub/Makefile |   12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> 
> diff --git a/scrub/Makefile b/scrub/Makefile
> index aebc7e43..c6473c12 100644
> --- a/scrub/Makefile
> +++ b/scrub/Makefile
> @@ -103,7 +103,7 @@ default: depend $(LTCOMMAND) $(XFS_SCRUB_ALL_PROG) $(OPTIONAL_TARGETS)
>   
>   xfs_scrub_all: xfs_scrub_all.in
>   	@echo "    [SED]    $@"
> -	$(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \
> +	$(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \
>   		   -e "s|@pkg_version@|$(PKG_VERSION)|g" \
>   		   -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" < $< > $@
>   	$(Q)chmod a+x $@
> @@ -116,14 +116,14 @@ install: $(INSTALL_SCRUB)
>   
>   %.service: %.service.in
>   	@echo "    [SED]    $@"
> -	$(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" \
> +	$(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" \
>   		   -e "s|@scrub_args@|$(XFS_SCRUB_ARGS)|g" \
>   		   -e "s|@pkg_lib_dir@|$(PKG_LIB_DIR)|g" \
>   		   -e "s|@pkg_name@|$(PKG_NAME)|g" < $< > $@
>   
>   %.cron: %.cron.in
>   	@echo "    [SED]    $@"
> -	$(Q)$(SED) -e "s|@sbindir@|$(PKG_ROOT_SBIN_DIR)|g" < $< > $@
> +	$(Q)$(SED) -e "s|@sbindir@|$(PKG_SBIN_DIR)|g" < $< > $@
>   
>   install-systemd: default $(SYSTEMD_SERVICES)
>   	$(INSTALL) -m 755 -d $(SYSTEMD_SYSTEM_UNIT_DIR)
> @@ -136,9 +136,9 @@ install-crond: default $(CRONTABS)
>   	$(INSTALL) -m 644 $(CRONTABS) $(CROND_DIR)
>   
>   install-scrub: default
> -	$(INSTALL) -m 755 -d $(PKG_ROOT_SBIN_DIR)
> -	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_ROOT_SBIN_DIR)
> -	$(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_ROOT_SBIN_DIR)
> +	$(INSTALL) -m 755 -d $(PKG_SBIN_DIR)
> +	$(LTINSTALL) -m 755 $(LTCOMMAND) $(PKG_SBIN_DIR)
> +	$(INSTALL) -m 755 $(XFS_SCRUB_ALL_PROG) $(PKG_SBIN_DIR)
>   
>   install-dev:
>   
> 

Re: [PATCH 6/6] xfs_scrub: move everything to /usr/sbin

[L A Walsh]

On 11/9/2018 4:45 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Recently, it was pointed out that xfs_scrub{,_all} depend on components
> and libraries (libicu, python) that live in /usr.  /sbin binaries
> shouldn't depend on /usr, so let's move the scrub binaries to /usr/sbin.
----
	As stated in the original comment, only "xfs_scrub_all"
depends on /usr, as it depends on /usr/bin/python.

System binaries that might be used to repair problems should, IMO,
reside on /sbin (or /bin) if they don't depend on /usr.


xfs_scrub has no non-root dependencies on my version:
> ldd /sbin/xfs_scrub                                                
  linux-vdso.so.1 (0x00007ffe275d1000)
  libc.so.6 => /lib64/libc.so.6 (0x00007fd1c3561000)
  libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd1c3342000)
  /lib64/ld-linux-x86-64.so.2 (0x00007fd1c3b32000)


Ideally the same should go for xfs_restore (and dump), xfs.mkfs,
xfs_repair, xfs_admin, maybe xfs_db (hopefully rare), and xfs_irecover.

Please don't _unnecessarily_ move binaries that might be needed for fs repair
onto /usr if possible.   I've found that using 'xfs' for a root 
file system eliminates my need for an indirect boot off of a
ram disk.

xfs has been far more stable and easy to use than indirect methods involving
a separate ramdisk.

Thanks,
-linda

Re: [PATCH 6/6] xfs_scrub: move everything to /usr/sbin

[Darrick J. Wong]

On Sat, Nov 10, 2018 at 12:43:58AM -0800, L A Walsh wrote:
> 
> 
> On 11/9/2018 4:45 PM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@oracle.com>
> > 
> > Recently, it was pointed out that xfs_scrub{,_all} depend on components
> > and libraries (libicu, python) that live in /usr.  /sbin binaries
> > shouldn't depend on /usr, so let's move the scrub binaries to /usr/sbin.
> ----
> 	As stated in the original comment, only "xfs_scrub_all"
> depends on /usr, as it depends on /usr/bin/python.
> 
> System binaries that might be used to repair problems should, IMO,
> reside on /sbin (or /bin) if they don't depend on /usr.
> 
> 
> xfs_scrub has no non-root dependencies on my version:
> > ldd /sbin/xfs_scrub
>  linux-vdso.so.1 (0x00007ffe275d1000)
>  libc.so.6 => /lib64/libc.so.6 (0x00007fd1c3561000)
>  libpthread.so.0 => /lib64/libpthread.so.0 (0x00007fd1c3342000)
>  /lib64/ld-linux-x86-64.so.2 (0x00007fd1c3b32000)

Ahh, but xfs_scrub depends on /usr on my system:

$ ldd $(which xfs_scrub)
linux-vdso.so.1 (0x00007ffd610d5000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007fd40f1e2000)
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007fd40efc3000)
libicui18n.so.60 => /usr/lib/x86_64-linux-gnu/libicui18n.so.60 (0x00007fd40eb22000)
libicuuc.so.60 => /usr/lib/x86_64-linux-gnu/libicuuc.so.60 (0x00007fd40e76b000)
/lib64/ld-linux-x86-64.so.2 (0x00007fd40f7ea000)
libstdc++.so.6 => /usr/lib/x86_64-linux-gnu/libstdc++.so.6 (0x00007fd40e3e2000)
libm.so.6 => /lib/x86_64-linux-gnu/libm.so.6 (0x00007fd40e044000)
libgcc_s.so.1 => /lib/x86_64-linux-gnu/libgcc_s.so.1 (0x00007fd40de2c000)
libicudata.so.60 => /usr/lib/x86_64-linux-gnu/libicudata.so.60 (0x00007fd40c283000)
libdl.so.2 => /lib/x86_64-linux-gnu/libdl.so.2 (0x00007fd40c07f000)

So that's why it moved too.

(The libraries from /usr are used to notify the sysadmin about
directories with file names that render in a deceptively similar
fashion, if the system locale settings indicate UTF-8 mode.)

> Ideally the same should go for xfs_restore (and dump), xfs.mkfs,
> xfs_repair, xfs_admin, maybe xfs_db (hopefully rare), and xfs_irecover.

mkfs and repair live in /sbin, at least on Debian.  I've variously
wondered if xfs_metadump (and therefore xfs_db) belong somewhere so that
you can metadump or debug any filesystem possible, though so far that's
only taken the form of an initramfs hook...

> Please don't _unnecessarily_ move binaries that might be needed for fs repair
> onto /usr if possible.   I've found that using 'xfs' for a root file system
> eliminates my need for an indirect boot off of a
> ram disk.

Please remember, scrub is the online fsck tool; if you can't get / and
/usr online then the recovery procedure (xfs_repair) for them is the
same as it always has been.

> xfs has been far more stable and easy to use than indirect methods involving
> a separate ramdisk.

Yeah, I noticed that still mostly works even these days. :)

(Granted that was the other day when I was test-booting a browser x86 vm
off a simple xfs image just to see if we could do 'xfstests in your
browser' but aside from the absurdly slow emulation it mostly worked.
:P)

--D

> Thanks,
> -linda
> 

[PATCH 7/6] xfs_db: add missing string name for DBM_COWDATA

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

In db/check.c, typename[] is supposed to have strings for every DBM_
type, but we forgot one.  Add it now.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 db/check.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/db/check.c b/db/check.c
index 352aab3f..f5b5e845 100644
--- a/db/check.c
+++ b/db/check.c
@@ -164,6 +164,7 @@ static const char	*typename[] = {
 	"btrmap",
 	"btrefcnt",
 	"rldata",
+	"cowdata",
 	NULL
 };
 static int		verbose;

Re: [PATCH 7/6] xfs_db: add missing string name for DBM_COWDATA

[Eric Sandeen]

On 11/27/18 5:15 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In db/check.c, typename[] is supposed to have strings for every DBM_
> type, but we forgot one.  Add it now.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
>  db/check.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/db/check.c b/db/check.c
> index 352aab3f..f5b5e845 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -164,6 +164,7 @@ static const char	*typename[] = {
>  	"btrmap",
>  	"btrefcnt",
>  	"rldata",
> +	"cowdata",
>  	NULL
>  };
>  static int		verbose;

Maybe:

/* Every type plus the NULL at the end */
BUILD_BUG_ON(ARRAY_SIZE(typename) != DBM_NDBM + 1);

in init() to avoid this next time?

-Eric

[PATCH v2 7/6] xfs_db: add missing string name for DBM_COWDATA

[Darrick J. Wong]

From: Darrick J. Wong <darrick.wong@oracle.com>

In db/check.c, typename[] is supposed to have strings for every DBM_
type, but we forgot one.  Add it now.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
v2: add build time check
---
 db/check.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/db/check.c b/db/check.c
index 352aab3f..442600be 100644
--- a/db/check.c
+++ b/db/check.c
@@ -164,8 +164,19 @@ static const char	*typename[] = {
 	"btrmap",
 	"btrefcnt",
 	"rldata",
+	"cowdata",
 	NULL
 };
+
+/*
+ * Make sure typename has the same number of elements as there are DBM types.
+ * This function isn't called anywhere; we just use it to trip up the compiler.
+ */
+static inline void check_typename(void)
+{
+	BUILD_BUG_ON(ARRAY_SIZE(typename) != DBM_NDBM + 1);
+}
+
 static int		verbose;
 
 #define	CHECK_BLIST(b)	(blist_size && check_blist(b))

Re: [PATCH v2 7/6] xfs_db: add missing string name for DBM_COWDATA

[Eric Sandeen]

On 11/28/18 6:54 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> In db/check.c, typename[] is supposed to have strings for every DBM_
> type, but we forgot one.  Add it now.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---
> v2: add build time check
> ---
>  db/check.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/db/check.c b/db/check.c
> index 352aab3f..442600be 100644
> --- a/db/check.c
> +++ b/db/check.c
> @@ -164,8 +164,19 @@ static const char	*typename[] = {
>  	"btrmap",
>  	"btrefcnt",
>  	"rldata",
> +	"cowdata",
>  	NULL
>  };
> +
> +/*
> + * Make sure typename has the same number of elements as there are DBM types.
> + * This function isn't called anywhere; we just use it to trip up the compiler.
> + */
> +static inline void check_typename(void)
> +{
> +	BUILD_BUG_ON(ARRAY_SIZE(typename) != DBM_NDBM + 1);
> +}
> +
>  static int		verbose;
>  
>  #define	CHECK_BLIST(b)	(blist_size && check_blist(b))

Seems ok, if it won't cause unused warnings ...

Reviewed-by: Eric Sandeen <sandeen@redhat.com>