Skip to contentSkip to conversation

Fix the inconsistency between the code and the manual page of mkfs.xfs.

PATCH In review [email protected] 2 participants 4 messages Jun 13, 2019 – Jul 11, 2019

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 Fix the inconsistency between the code and the manual page of mkfs.xfs.

From: Alvin Zheng <[email protected]>

Signed-off-by Alvin Zheng <[email protected]>
---
 man/man8/mkfs.xfs.8 | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
man/man8/mkfs.xfs.8 +14 -9
index 4b8c78c..bf2ad54 100644
@@ -115,9 +115,12 @@ When specifying parameters in units of sectors or filesystem blocks, the
 .B \-s
 option or the
 .B \-b
-option first needs to be added to the command line.
-Failure to specify the size of the units will result in illegal value errors
-when parameters are quantified in those units.
+option can be used to specify the size of the sector or block. The 
+.B \-s
+option and the
+.B \-b
+should be placed before any options in units of sectors or blocks. If the size of the block
+or sector is not specified, the default size (block: 4KiB, sector: 512B) will be used.
 .PP
 Many feature options allow an optional argument of 0 or 1, to explicitly
 disable or enable the functionality.

@@ -136,9 +139,10 @@ The filesystem block size is specified with a
 in bytes. The default value is 4096 bytes (4 KiB), the minimum is 512, and the
 maximum is 65536 (64 KiB).
 .IP
-To specify any options on the command line in units of filesystem blocks, this
-option must be specified first so that the filesystem block size is
-applied consistently to all options.
+If a non-default filesystem block size is specified, the option
+must be specified before any options that use filesystem block size
+units so that the non-default filesystem block size is applied
+consistently to all options.
 .IP
 Although
 .B mkfs.xfs

@@ -895,9 +899,10 @@ is 512 bytes. The minimum value for sector size is
 must be a power of 2 size and cannot be made larger than the
 filesystem block size.
 .IP
-To specify any options on the command line in units of sectors, this
-option must be specified first so that the sector size is
-applied consistently to all options.
+If a non-default sector size is specified, the option
+must be specified before any options that use sector size
+units so that the non-default sector size is applied
+consistently to all options.
 .RE
 .TP
 .BI \-L " label"
Signature · 1 line
1.8.3.1
#1 lore raw
Quoted 3 lines · On 6/13/19 5:26 AM, [email protected] wrote:

From: Alvin Zheng <[email protected]>

Signed-off-by Alvin Zheng <[email protected]>

Sorry for getting to this so late. First of all, we need a descriptive changelog, something like:

mkfs.xfs.8: Fix an inconsistency between the code and the man page

The man page currently states that block and sector size units cannot be used for other option values unless they are explicitly specified, when in fact the default sizes will be used in that case.

Change the man page to clarify this.

Quoted 9 lines
---
 man/man8/mkfs.xfs.8 | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)
man/man8/mkfs.xfs.8 +0 -0
index 4b8c78c..bf2ad54 100644

man/man8/mkfs.xfs.8 When specifying parameters in units of sectors or filesystem blocks, the · line 118

Quoted 7 lines
man/man8/mkfs.xfs.8 +1 -3
@@ -115,9 +115,12 @@ When specifying parameters in units of sectors or filesystem blocks, the
 .B \-s
 option or the
 .B \-b
-option first needs to be added to the command line.
-Failure to specify the size of the units will result in illegal value errors
-when parameters are quantified in those units.
+option can be used to specify the size of the sector or block. The

trailing whitespace there

Quoted 7 lines
man/man8/mkfs.xfs.8 +5 -0
+.B \-s
+option and the
+.B \-b
+should be placed before any options in units of sectors or blocks. If the size of the block
+or sector is not specified, the default size (block: 4KiB, sector: 512B) will be used.

80 col lines.

But also, it seems that it's not actually necessary to state them first, as this works as expected:

# mkfs/mkfs.xfs -d size=65536b,file,name=fsfile -b size=2k
                        ^^^^^^                     ^^^^^^^
meta-data=fsfile                 isize=512    agcount=4, agsize=16384 blks
         =                       sectsz=512   attr=2, projid32bit=1
         =                       crc=1        finobt=1, sparse=1, rmapbt=0
         =                       reflink=1
data     =                       bsize=2048   blocks=65536, imaxpct=25
                                       ^^^^          ^^^^^
         =                       sunit=0      swidth=0 blks

...

# mkfs/mkfs.xfs -d size=65536s,file,name=fsfile -s size=2k
                        ^^^^^^                     ^^^^^^^
meta-data=fsfile                 isize=512    agcount=4, agsize=8192 blks
         =                       sectsz=2048  attr=2, projid32bit=1
                                        ^^^^
         =                       crc=1        finobt=1, sparse=1, rmapbt=0
         =                       reflink=1
data     =                       bsize=4096   blocks=32768, imaxpct=25
                                                     ^^^^^
         =                       sunit=0      swidth=0 blks

I think this is because we only parse the string to start:

data_opts_parser()

        case D_SIZE:
                cli->dsize = getstr(value, opts, subopt);
                break;

and in fact this is true for all the things that can take this type of unit:

        /* parameters that depend on sector/block size being validated. */
        char    *dsize;
        char    *agsize;
        char    *dsu;
        char    *dirblocksize;
        char    *logsize;
        char    *lsu;
        char    *rtextsize;
        char    *rtsize;

So we validate blocksize & sector size and move them from cli or defaults into cfg, as needed:

        validate_blocksize(&cfg, &cli, &dft);
        validate_sectorsize(&cfg, &cli, &dft, &ft, dfile, dry_run,
                            force_overwrite);

and from then on we can start converting other sizes using those units:

        /*
         * we've now completed basic validation of the features, sector and
         * block sizes, so from this point onwards we use the values found in
         * the cfg structure for them, not the command line structure.
         */ 
        validate_dirblocksize(&cfg, &cli);
        validate_inodesize(&cfg, &cli);

...

so I think the changes which indicate that -s size and -b size must be stated first are not actually correct, as it was intentional to handle them being stated in any order. (I know Dave said otherwise, but I think he was wrong, and he forgot how he wrote this code) ;)

-Eric

Quoted 32 lines

.PP
Many feature options allow an optional argument of 0 or 1, to explicitly disable or enable the functionality.

man/man8/mkfs.xfs.8 +8 -6
@@ -136,9 +139,10 @@ The filesystem block size is specified with a
 in bytes. The default value is 4096 bytes (4 KiB), the minimum is 512, and the
 maximum is 65536 (64 KiB).
 .IP
-To specify any options on the command line in units of filesystem blocks, this
-option must be specified first so that the filesystem block size is
-applied consistently to all options.
+If a non-default filesystem block size is specified, the option
+must be specified before any options that use filesystem block size
+units so that the non-default filesystem block size is applied
+consistently to all options.
 .IP
 Although
 .B mkfs.xfs

@@ -895,9 +899,10 @@ is 512 bytes. The minimum value for sector size is
 must be a power of 2 size and cannot be made larger than the
 filesystem block size.
 .IP
-To specify any options on the command line in units of sectors, this
-option must be specified first so that the sector size is
-applied consistently to all options.
+If a non-default sector size is specified, the option
+must be specified before any options that use sector size
+units so that the non-default sector size is applied
+consistently to all options.
 .RE
 .TP
 .BI \-L " label"
Reviewed-by
#2 lore raw

To save time, I'll merge this with my edits if you agree:

mkfs.xfs.8: Fix an inconsistency between the code and the man page.

From: Alvin Zheng <[email protected]>

The man page currently states that block and sector size units cannot be used for other option values unless they are explicitly specified, when in fact the default sizes will be used in that case.

Change the man page to clarify this.

Signed-off-by Alvin Zheng <[email protected]>
[sandeen: sector/block values do not need to be specified first]
Reviewed-by Eric Sandeen <[email protected]>
Signed-off-by Eric Sandeen <[email protected]>

---

man/man8/mkfs.xfs.8 +3 -11
index 78b15015..9d762a43 100644
@@ -115,9 +115,9 @@ When specifying parameters in units of sectors or filesystem blocks, the
 .B \-s
 option or the
 .B \-b
-option first needs to be added to the command line.
-Failure to specify the size of the units will result in illegal value errors
-when parameters are quantified in those units.
+option may be used to specify the size of the sector or block.
+If the size of the block or sector is not specified, the default sizes
+(block: 4KiB, sector: 512B) will be used.
 .PP
 Many feature options allow an optional argument of 0 or 1, to explicitly
 disable or enable the functionality.

@@ -136,10 +136,6 @@ The filesystem block size is specified with a
 in bytes. The default value is 4096 bytes (4 KiB), the minimum is 512, and the
 maximum is 65536 (64 KiB).
 .IP
-To specify any options on the command line in units of filesystem blocks, this
-option must be specified first so that the filesystem block size is
-applied consistently to all options.
-.IP
 Although
 .B mkfs.xfs
 will accept any of these values and create a valid filesystem,

@@ -901,10 +897,6 @@ is 512 bytes. The minimum value for sector size is
 .I sector_size
 must be a power of 2 size and cannot be made larger than the
 filesystem block size.
-.IP
-To specify any options on the command line in units of sectors, this
-option must be specified first so that the sector size is
-applied consistently to all options.
 .RE
 .TP
 .BI \-L " label"
author
#3 lore raw
Quoted 1 line · Eric Sandeen

To save time, I'll merge this with my edits if you agree:

Please merge it with your edits, thanks.

Best regards,

Alvin

Quoted 57 lines

mkfs.xfs.8: Fix an inconsistency between the code and the man page.

From: Alvin Zheng <[email protected]>

The man page currently states that block and sector size units cannot be used for other option values unless they are explicitly specified, when in fact the default sizes will be used in that case.

Change the man page to clarify this.

Signed-off-by Alvin Zheng <[email protected]>
[sandeen: sector/block values do not need to be specified first]
Reviewed-by Eric Sandeen <[email protected]>
Signed-off-by Eric Sandeen <[email protected]>

---

man/man8/mkfs.xfs.8 +3 -11
index 78b15015..9d762a43 100644
@@ -115,9 +115,9 @@ When specifying parameters in units of sectors or filesystem blocks, the
  .B \-s
  option or the
  .B \-b
-option first needs to be added to the command line.
-Failure to specify the size of the units will result in illegal value errors
-when parameters are quantified in those units.
+option may be used to specify the size of the sector or block.
+If the size of the block or sector is not specified, the default sizes
+(block: 4KiB, sector: 512B) will be used.
  .PP
  Many feature options allow an optional argument of 0 or 1, to explicitly
  disable or enable the functionality.

@@ -136,10 +136,6 @@ The filesystem block size is specified with a
  in bytes. The default value is 4096 bytes (4 KiB), the minimum is 512, and the
  maximum is 65536 (64 KiB).
  .IP
-To specify any options on the command line in units of filesystem blocks, this
-option must be specified first so that the filesystem block size is
-applied consistently to all options.
-.IP
  Although
  .B mkfs.xfs
  will accept any of these values and create a valid filesystem,

@@ -901,10 +897,6 @@ is 512 bytes. The minimum value for sector size is
  .I sector_size
  must be a power of 2 size and cannot be made larger than the
  filesystem block size.
-.IP
-To specify any options on the command line in units of sectors, this
-option must be specified first so that the sector size is
-applied consistently to all options.
  .RE
  .TP
  .BI \-L " label"