[PATCH v2 0/2] Fixes to overlayfs documentation

[PATCH v2 0/2] Fixes to overlayfs documentation

[Amir Goldstein]

Hi all,

Some minor fixes to overlayfs.rst that I plan to queue for next merge
window.

Some of the fixes are workarounds for oddities of github when parsing
ReST format [1].

Amir.

[1] https://github.com/torvalds/linux/blob/master/Documentation/filesystems/overlayfs.rst#permission-model

Amir Goldstein (2):
  overlayfs.rst: use consistent feature names
  overlayfs.rst: fix ReST formatting

 Documentation/filesystems/overlayfs.rst | 90 +++++++++++++------------
 1 file changed, 47 insertions(+), 43 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] overlayfs.rst: use consistent feature names

[Amir Goldstein]

Use the feature names "metacopy" and "index" consistently throughout
the document.

Covert the numbered list of features "redirect_dir", "index", "xino"
to section headings, so that those features could be referenced in the
document by their name.

Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/overlayfs.rst | 27 ++++++++++++++-----------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index 0407f361f32a..926396fdc5eb 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -39,7 +39,7 @@ objects in the original filesystem.
 On 64bit systems, even if all overlay layers are not on the same
 underlying filesystem, the same compliant behavior could be achieved
 with the "xino" feature.  The "xino" feature composes a unique object
-identifier from the real object st_ino and an underlying fsid index.
+identifier from the real object st_ino and an underlying fsid number.
 The "xino" feature uses the high inode number bits for fsid, because the
 underlying filesystems rarely use the high inode number bits.  In case
 the underlying inode number does overflow into the high xino bits, overlay
@@ -356,7 +356,7 @@ as an octal characters (\072) when displayed in /proc/self/mountinfo.
 Metadata only copy up
 ---------------------
 
-When metadata only copy up feature is enabled, overlayfs will only copy
+When the "metacopy" feature is enabled, overlayfs will only copy
 up metadata (as opposed to whole file), when a metadata specific operation
 like chown/chmod is performed. Full file will be copied up later when
 file is opened for WRITE operation.
@@ -492,27 +492,27 @@ though it will not result in a crash or deadlock.
 
 Mounting an overlay using an upper layer path, where the upper layer path
 was previously used by another mounted overlay in combination with a
-different lower layer path, is allowed, unless the "inodes index" feature
-or "metadata only copy up" feature is enabled.
+different lower layer path, is allowed, unless the "index" or "metacopy"
+features are enabled.
 
-With the "inodes index" feature, on the first time mount, an NFS file
+With the "index" feature, on the first time mount, an NFS file
 handle of the lower layer root directory, along with the UUID of the lower
 filesystem, are encoded and stored in the "trusted.overlay.origin" extended
 attribute on the upper layer root directory.  On subsequent mount attempts,
 the lower root directory file handle and lower filesystem UUID are compared
 to the stored origin in upper root directory.  On failure to verify the
 lower root origin, mount will fail with ESTALE.  An overlayfs mount with
-"inodes index" enabled will fail with EOPNOTSUPP if the lower filesystem
+"index" enabled will fail with EOPNOTSUPP if the lower filesystem
 does not support NFS export, lower filesystem does not have a valid UUID or
 if the upper filesystem does not support extended attributes.
 
-For "metadata only copy up" feature there is no verification mechanism at
+For the "metacopy" feature, there is no verification mechanism at
 mount time. So if same upper is mounted with different set of lower, mount
 probably will succeed but expect the unexpected later on. So don't do it.
 
 It is quite a common practice to copy overlay layers to a different
 directory tree on the same or different underlying filesystem, and even
-to a different machine.  With the "inodes index" feature, trying to mount
+to a different machine.  With the "index" feature, trying to mount
 the copied layers will fail the verification of the lower root file handle.
 
 Nesting overlayfs mounts
@@ -560,7 +560,8 @@ file for write or truncating the file will not be denied with ETXTBSY.
 The following options allow overlayfs to act more like a standards
 compliant filesystem:
 
-1) "redirect_dir"
+redirect_dir
+````````````
 
 Enabled with the mount option or module option: "redirect_dir=on" or with
 the kernel config option CONFIG_OVERLAY_FS_REDIRECT_DIR=y.
@@ -568,7 +569,8 @@ the kernel config option CONFIG_OVERLAY_FS_REDIRECT_DIR=y.
 If this feature is disabled, then rename(2) on a lower or merged directory
 will fail with EXDEV ("Invalid cross-device link").
 
-2) "inode index"
+index
+`````
 
 Enabled with the mount option or module option "index=on" or with the
 kernel config option CONFIG_OVERLAY_FS_INDEX=y.
@@ -577,7 +579,8 @@ If this feature is disabled and a file with multiple hard links is copied
 up, then this will "break" the link.  Changes will not be propagated to
 other names referring to the same inode.
 
-3) "xino"
+xino
+````
 
 Enabled with the mount option "xino=auto" or "xino=on", with the module
 option "xino_auto=on" or with the kernel config option
@@ -604,7 +607,7 @@ a crash or deadlock.
 
 Offline changes, when the overlay is not mounted, are allowed to the
 upper tree.  Offline changes to the lower tree are only allowed if the
-"metadata only copy up", "inode index", "xino" and "redirect_dir" features
+"metacopy", "index", "xino" and "redirect_dir" features
 have not been used.  If the lower tree is modified and any of these
 features has been used, the behavior of the overlay is undefined,
 though it will not result in a crash or deadlock.
-- 
2.34.1

[PATCH v2 2/2] overlayfs.rst: fix ReST formatting

[Amir Goldstein]

Fix some indentation issues and fix missing newlines in quoted text
by converting quoted text to code blocks.

Unindent a) b) enumerated list to workaround github displaying it
as numbered list.

Reported-by: Christian Brauner <brauner@kernel.org>
Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
---
 Documentation/filesystems/overlayfs.rst | 63 +++++++++++++------------
 1 file changed, 32 insertions(+), 31 deletions(-)

diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
index 926396fdc5eb..a36f3a2a2d4b 100644
--- a/Documentation/filesystems/overlayfs.rst
+++ b/Documentation/filesystems/overlayfs.rst
@@ -118,7 +118,7 @@ Where both upper and lower objects are directories, a merged directory
 is formed.
 
 At mount time, the two directories given as mount options "lowerdir" and
-"upperdir" are combined into a merged directory:
+"upperdir" are combined into a merged directory::
 
   mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
   workdir=/work /merged
@@ -174,10 +174,10 @@ programs.
 seek offsets are assigned sequentially when the directories are read.
 Thus if
 
-  - read part of a directory
-  - remember an offset, and close the directory
-  - re-open the directory some time later
-  - seek to the remembered offset
+- read part of a directory
+- remember an offset, and close the directory
+- re-open the directory some time later
+- seek to the remembered offset
 
 there may be little correlation between the old and new locations in
 the list of filenames, particularly if anything has changed in the
@@ -285,21 +285,21 @@ Permission model
 
 Permission checking in the overlay filesystem follows these principles:
 
- 1) permission check SHOULD return the same result before and after copy up
+1) permission check SHOULD return the same result before and after copy up
 
- 2) task creating the overlay mount MUST NOT gain additional privileges
+2) task creating the overlay mount MUST NOT gain additional privileges
 
- 3) non-mounting task MAY gain additional privileges through the overlay,
- compared to direct access on underlying lower or upper filesystems
+3) non-mounting task MAY gain additional privileges through the overlay,
+   compared to direct access on underlying lower or upper filesystems
 
-This is achieved by performing two permission checks on each access
+This is achieved by performing two permission checks on each access:
 
- a) check if current task is allowed access based on local DAC (owner,
-    group, mode and posix acl), as well as MAC checks
+a) check if current task is allowed access based on local DAC (owner,
+group, mode and posix acl), as well as MAC checks
 
- b) check if mounting task would be allowed real operation on lower or
-    upper layer based on underlying filesystem permissions, again including
-    MAC checks
+b) check if mounting task would be allowed real operation on lower or
+upper layer based on underlying filesystem permissions, again including
+MAC checks
 
 Check (a) ensures consistency (1) since owner, group, mode and posix acls
 are copied up.  On the other hand it can result in server enforced
@@ -311,11 +311,11 @@ to create setups where the consistency rule (1) does not hold; normally,
 however, the mounting task will have sufficient privileges to perform all
 operations.
 
-Another way to demonstrate this model is drawing parallels between
+Another way to demonstrate this model is drawing parallels between::
 
   mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,... /merged
 
-and
+and::
 
   cp -a /lower /upper
   mount --bind /upper /merged
@@ -328,7 +328,7 @@ Multiple lower layers
 ---------------------
 
 Multiple lower layers can now be given using the colon (":") as a
-separator character between the directory names.  For example:
+separator character between the directory names.  For example::
 
   mount -t overlay overlay -olowerdir=/lower1:/lower2:/lower3 /merged
 
@@ -340,13 +340,13 @@ rightmost one and going left.  In the above example lower1 will be the
 top, lower2 the middle and lower3 the bottom layer.
 
 Note: directory names containing colons can be provided as lower layer by
-escaping the colons with a single backslash.  For example:
+escaping the colons with a single backslash.  For example::
 
   mount -t overlay overlay -olowerdir=/a\:lower\:\:dir /merged
 
 Since kernel version v6.8, directory names containing colons can also
 be configured as lower layer using the "lowerdir+" mount options and the
-fsconfig syscall from new mount api.  For example:
+fsconfig syscall from new mount api.  For example::
 
   fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/a:lower::dir", 0);
 
@@ -390,11 +390,11 @@ Data-only lower layers
 With "metacopy" feature enabled, an overlayfs regular file may be a composition
 of information from up to three different layers:
 
- 1) metadata from a file in the upper layer
+1) metadata from a file in the upper layer
 
- 2) st_ino and st_dev object identifier from a file in a lower layer
+2) st_ino and st_dev object identifier from a file in a lower layer
 
- 3) data from a file in another lower layer (further below)
+3) data from a file in another lower layer (further below)
 
 The "lower data" file can be on any lower layer, except from the top most
 lower layer.
@@ -405,7 +405,7 @@ A normal lower layer is not allowed to be below a data-only layer, so single
 colon separators are not allowed to the right of double colon ("::") separators.
 
 
-For example:
+For example::
 
   mount -t overlay overlay -olowerdir=/l1:/l2:/l3::/do1::/do2 /merged
 
@@ -419,7 +419,7 @@ to the absolute path of the "lower data" file in the "data-only" lower layer.
 
 Since kernel version v6.8, "data-only" lower layers can also be added using
 the "datadir+" mount options and the fsconfig syscall from new mount api.
-For example:
+For example::
 
   fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l1", 0);
   fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l2", 0);
@@ -429,7 +429,7 @@ For example:
 
 
 fs-verity support
-----------------------
+-----------------
 
 During metadata copy up of a lower file, if the source file has
 fs-verity enabled and overlay verity support is enabled, then the
@@ -653,9 +653,10 @@ following rules apply:
    encode an upper file handle from upper inode
 
 The encoded overlay file handle includes:
- - Header including path type information (e.g. lower/upper)
- - UUID of the underlying filesystem
- - Underlying filesystem encoding of underlying inode
+
+- Header including path type information (e.g. lower/upper)
+- UUID of the underlying filesystem
+- Underlying filesystem encoding of underlying inode
 
 This encoding format is identical to the encoding format file handles that
 are stored in extended attribute "trusted.overlay.origin".
@@ -773,9 +774,9 @@ Testsuite
 There's a testsuite originally developed by David Howells and currently
 maintained by Amir Goldstein at:
 
-  https://github.com/amir73il/unionmount-testsuite.git
+https://github.com/amir73il/unionmount-testsuite.git
 
-Run as root:
+Run as root::
 
   # cd unionmount-testsuite
   # ./run --ov --verify
-- 
2.34.1

Re: [PATCH v2 1/2] overlayfs.rst: use consistent feature names

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 5858 bytes --]

On Wed, Dec 13, 2023 at 02:34:21PM +0200, Amir Goldstein wrote:
> Use the feature names "metacopy" and "index" consistently throughout
> the document.
> 
> Covert the numbered list of features "redirect_dir", "index", "xino"
> to section headings, so that those features could be referenced in the
> document by their name.
> 
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  Documentation/filesystems/overlayfs.rst | 27 ++++++++++++++-----------
>  1 file changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 0407f361f32a..926396fdc5eb 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -39,7 +39,7 @@ objects in the original filesystem.
>  On 64bit systems, even if all overlay layers are not on the same
>  underlying filesystem, the same compliant behavior could be achieved
>  with the "xino" feature.  The "xino" feature composes a unique object
> -identifier from the real object st_ino and an underlying fsid index.
> +identifier from the real object st_ino and an underlying fsid number.
>  The "xino" feature uses the high inode number bits for fsid, because the
>  underlying filesystems rarely use the high inode number bits.  In case
>  the underlying inode number does overflow into the high xino bits, overlay
> @@ -356,7 +356,7 @@ as an octal characters (\072) when displayed in /proc/self/mountinfo.
>  Metadata only copy up
>  ---------------------
>  
> -When metadata only copy up feature is enabled, overlayfs will only copy
> +When the "metacopy" feature is enabled, overlayfs will only copy
>  up metadata (as opposed to whole file), when a metadata specific operation
>  like chown/chmod is performed. Full file will be copied up later when
>  file is opened for WRITE operation.
> @@ -492,27 +492,27 @@ though it will not result in a crash or deadlock.
>  
>  Mounting an overlay using an upper layer path, where the upper layer path
>  was previously used by another mounted overlay in combination with a
> -different lower layer path, is allowed, unless the "inodes index" feature
> -or "metadata only copy up" feature is enabled.
> +different lower layer path, is allowed, unless the "index" or "metacopy"
> +features are enabled.
>  
> -With the "inodes index" feature, on the first time mount, an NFS file
> +With the "index" feature, on the first time mount, an NFS file
>  handle of the lower layer root directory, along with the UUID of the lower
>  filesystem, are encoded and stored in the "trusted.overlay.origin" extended
>  attribute on the upper layer root directory.  On subsequent mount attempts,
>  the lower root directory file handle and lower filesystem UUID are compared
>  to the stored origin in upper root directory.  On failure to verify the
>  lower root origin, mount will fail with ESTALE.  An overlayfs mount with
> -"inodes index" enabled will fail with EOPNOTSUPP if the lower filesystem
> +"index" enabled will fail with EOPNOTSUPP if the lower filesystem
>  does not support NFS export, lower filesystem does not have a valid UUID or
>  if the upper filesystem does not support extended attributes.
>  
> -For "metadata only copy up" feature there is no verification mechanism at
> +For the "metacopy" feature, there is no verification mechanism at
>  mount time. So if same upper is mounted with different set of lower, mount
>  probably will succeed but expect the unexpected later on. So don't do it.
>  
>  It is quite a common practice to copy overlay layers to a different
>  directory tree on the same or different underlying filesystem, and even
> -to a different machine.  With the "inodes index" feature, trying to mount
> +to a different machine.  With the "index" feature, trying to mount
>  the copied layers will fail the verification of the lower root file handle.
>  
>  Nesting overlayfs mounts
> @@ -560,7 +560,8 @@ file for write or truncating the file will not be denied with ETXTBSY.
>  The following options allow overlayfs to act more like a standards
>  compliant filesystem:
>  
> -1) "redirect_dir"
> +redirect_dir
> +````````````
>  
>  Enabled with the mount option or module option: "redirect_dir=on" or with
>  the kernel config option CONFIG_OVERLAY_FS_REDIRECT_DIR=y.
> @@ -568,7 +569,8 @@ the kernel config option CONFIG_OVERLAY_FS_REDIRECT_DIR=y.
>  If this feature is disabled, then rename(2) on a lower or merged directory
>  will fail with EXDEV ("Invalid cross-device link").
>  
> -2) "inode index"
> +index
> +`````
>  
>  Enabled with the mount option or module option "index=on" or with the
>  kernel config option CONFIG_OVERLAY_FS_INDEX=y.
> @@ -577,7 +579,8 @@ If this feature is disabled and a file with multiple hard links is copied
>  up, then this will "break" the link.  Changes will not be propagated to
>  other names referring to the same inode.
>  
> -3) "xino"
> +xino
> +````
>  
>  Enabled with the mount option "xino=auto" or "xino=on", with the module
>  option "xino_auto=on" or with the kernel config option
> @@ -604,7 +607,7 @@ a crash or deadlock.
>  
>  Offline changes, when the overlay is not mounted, are allowed to the
>  upper tree.  Offline changes to the lower tree are only allowed if the
> -"metadata only copy up", "inode index", "xino" and "redirect_dir" features
> +"metacopy", "index", "xino" and "redirect_dir" features
>  have not been used.  If the lower tree is modified and any of these
>  features has been used, the behavior of the overlay is undefined,
>  though it will not result in a crash or deadlock.

LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] overlayfs.rst: fix ReST formatting

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 7962 bytes --]

On Wed, Dec 13, 2023 at 02:34:22PM +0200, Amir Goldstein wrote:
> Fix some indentation issues and fix missing newlines in quoted text
> by converting quoted text to code blocks.
> 
> Unindent a) b) enumerated list to workaround github displaying it
> as numbered list.
> 
> Reported-by: Christian Brauner <brauner@kernel.org>
> Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  Documentation/filesystems/overlayfs.rst | 63 +++++++++++++------------
>  1 file changed, 32 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 926396fdc5eb..a36f3a2a2d4b 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -118,7 +118,7 @@ Where both upper and lower objects are directories, a merged directory
>  is formed.
>  
>  At mount time, the two directories given as mount options "lowerdir" and
> -"upperdir" are combined into a merged directory:
> +"upperdir" are combined into a merged directory::
>  
>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
>    workdir=/work /merged
> @@ -174,10 +174,10 @@ programs.
>  seek offsets are assigned sequentially when the directories are read.
>  Thus if
>  
> -  - read part of a directory
> -  - remember an offset, and close the directory
> -  - re-open the directory some time later
> -  - seek to the remembered offset
> +- read part of a directory
> +- remember an offset, and close the directory
> +- re-open the directory some time later
> +- seek to the remembered offset
>  
>  there may be little correlation between the old and new locations in
>  the list of filenames, particularly if anything has changed in the
> @@ -285,21 +285,21 @@ Permission model
>  
>  Permission checking in the overlay filesystem follows these principles:
>  
> - 1) permission check SHOULD return the same result before and after copy up
> +1) permission check SHOULD return the same result before and after copy up
>  
> - 2) task creating the overlay mount MUST NOT gain additional privileges
> +2) task creating the overlay mount MUST NOT gain additional privileges
>  
> - 3) non-mounting task MAY gain additional privileges through the overlay,
> - compared to direct access on underlying lower or upper filesystems
> +3) non-mounting task MAY gain additional privileges through the overlay,
> +   compared to direct access on underlying lower or upper filesystems
>  
> -This is achieved by performing two permission checks on each access
> +This is achieved by performing two permission checks on each access:
>  
> - a) check if current task is allowed access based on local DAC (owner,
> -    group, mode and posix acl), as well as MAC checks
> +a) check if current task is allowed access based on local DAC (owner,
> +group, mode and posix acl), as well as MAC checks
>  
> - b) check if mounting task would be allowed real operation on lower or
> -    upper layer based on underlying filesystem permissions, again including
> -    MAC checks
> +b) check if mounting task would be allowed real operation on lower or
> +upper layer based on underlying filesystem permissions, again including
> +MAC checks
>  
>  Check (a) ensures consistency (1) since owner, group, mode and posix acls
>  are copied up.  On the other hand it can result in server enforced
> @@ -311,11 +311,11 @@ to create setups where the consistency rule (1) does not hold; normally,
>  however, the mounting task will have sufficient privileges to perform all
>  operations.
>  
> -Another way to demonstrate this model is drawing parallels between
> +Another way to demonstrate this model is drawing parallels between::
>  
>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,... /merged
>  
> -and
> +and::
>  
>    cp -a /lower /upper
>    mount --bind /upper /merged
> @@ -328,7 +328,7 @@ Multiple lower layers
>  ---------------------
>  
>  Multiple lower layers can now be given using the colon (":") as a
> -separator character between the directory names.  For example:
> +separator character between the directory names.  For example::
>  
>    mount -t overlay overlay -olowerdir=/lower1:/lower2:/lower3 /merged
>  
> @@ -340,13 +340,13 @@ rightmost one and going left.  In the above example lower1 will be the
>  top, lower2 the middle and lower3 the bottom layer.
>  
>  Note: directory names containing colons can be provided as lower layer by
> -escaping the colons with a single backslash.  For example:
> +escaping the colons with a single backslash.  For example::
>  
>    mount -t overlay overlay -olowerdir=/a\:lower\:\:dir /merged
>  
>  Since kernel version v6.8, directory names containing colons can also
>  be configured as lower layer using the "lowerdir+" mount options and the
> -fsconfig syscall from new mount api.  For example:
> +fsconfig syscall from new mount api.  For example::
>  
>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/a:lower::dir", 0);
>  
> @@ -390,11 +390,11 @@ Data-only lower layers
>  With "metacopy" feature enabled, an overlayfs regular file may be a composition
>  of information from up to three different layers:
>  
> - 1) metadata from a file in the upper layer
> +1) metadata from a file in the upper layer
>  
> - 2) st_ino and st_dev object identifier from a file in a lower layer
> +2) st_ino and st_dev object identifier from a file in a lower layer
>  
> - 3) data from a file in another lower layer (further below)
> +3) data from a file in another lower layer (further below)
>  
>  The "lower data" file can be on any lower layer, except from the top most
>  lower layer.
> @@ -405,7 +405,7 @@ A normal lower layer is not allowed to be below a data-only layer, so single
>  colon separators are not allowed to the right of double colon ("::") separators.
>  
>  
> -For example:
> +For example::
>  
>    mount -t overlay overlay -olowerdir=/l1:/l2:/l3::/do1::/do2 /merged
>  
> @@ -419,7 +419,7 @@ to the absolute path of the "lower data" file in the "data-only" lower layer.
>  
>  Since kernel version v6.8, "data-only" lower layers can also be added using
>  the "datadir+" mount options and the fsconfig syscall from new mount api.
> -For example:
> +For example::
>  
>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l1", 0);
>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l2", 0);
> @@ -429,7 +429,7 @@ For example:
>  
>  
>  fs-verity support
> -----------------------
> +-----------------
>  
>  During metadata copy up of a lower file, if the source file has
>  fs-verity enabled and overlay verity support is enabled, then the
> @@ -653,9 +653,10 @@ following rules apply:
>     encode an upper file handle from upper inode
>  
>  The encoded overlay file handle includes:
> - - Header including path type information (e.g. lower/upper)
> - - UUID of the underlying filesystem
> - - Underlying filesystem encoding of underlying inode
> +
> +- Header including path type information (e.g. lower/upper)
> +- UUID of the underlying filesystem
> +- Underlying filesystem encoding of underlying inode
>  
>  This encoding format is identical to the encoding format file handles that
>  are stored in extended attribute "trusted.overlay.origin".
> @@ -773,9 +774,9 @@ Testsuite
>  There's a testsuite originally developed by David Howells and currently
>  maintained by Amir Goldstein at:
>  
> -  https://github.com/amir73il/unionmount-testsuite.git
> +https://github.com/amir73il/unionmount-testsuite.git
>  
> -Run as root:
> +Run as root::
>  
>    # cd unionmount-testsuite
>    # ./run --ov --verify

LGTM, thanks!

Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/2] overlayfs.rst: fix ReST formatting

[Akira Yokosawa]

Hi,

On Wed, 13 Dec 2023 14:34:22 +0200, Amir Goldstein wrote:
> Fix some indentation issues and fix missing newlines in quoted text
> by converting quoted text to code blocks.
> 
> Unindent a) b) enumerated list to workaround github displaying it
> as numbered list.

I don't think we need to work around github's weird behavior around
enumerated lists.  What matters for us is what Sphinx (+ our own
extensions) ends up generating.

The corresponding html page rendered by Sphinx is at:
https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#permission-model

It does not look perfect, but at least it preserves enumeration by
number and alphabet.

I'd suggest reporting github about the minor breakage of their
rst renderer.

Further comments below:

> 
> Reported-by: Christian Brauner <brauner@kernel.org>
> Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> ---
>  Documentation/filesystems/overlayfs.rst | 63 +++++++++++++------------
>  1 file changed, 32 insertions(+), 31 deletions(-)
> 
> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> index 926396fdc5eb..a36f3a2a2d4b 100644
> --- a/Documentation/filesystems/overlayfs.rst
> +++ b/Documentation/filesystems/overlayfs.rst
> @@ -118,7 +118,7 @@ Where both upper and lower objects are directories, a merged directory
>  is formed.
>  
>  At mount time, the two directories given as mount options "lowerdir" and
> -"upperdir" are combined into a merged directory:
> +"upperdir" are combined into a merged directory::
>  
>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
>    workdir=/work /merged
> @@ -174,10 +174,10 @@ programs.
>  seek offsets are assigned sequentially when the directories are read.
>  Thus if
>  
> -  - read part of a directory
> -  - remember an offset, and close the directory
> -  - re-open the directory some time later
> -  - seek to the remembered offset
> +- read part of a directory
> +- remember an offset, and close the directory
> +- re-open the directory some time later
> +- seek to the remembered offset

To my eyes, unindent spoils the readability of this file as pure
plain text.  Please don't do this.

>  
>  there may be little correlation between the old and new locations in
>  the list of filenames, particularly if anything has changed in the
> @@ -285,21 +285,21 @@ Permission model
>  
>  Permission checking in the overlay filesystem follows these principles:
>  
> - 1) permission check SHOULD return the same result before and after copy up
> +1) permission check SHOULD return the same result before and after copy up
>  
> - 2) task creating the overlay mount MUST NOT gain additional privileges
> +2) task creating the overlay mount MUST NOT gain additional privileges
>  
> - 3) non-mounting task MAY gain additional privileges through the overlay,
> - compared to direct access on underlying lower or upper filesystems
> +3) non-mounting task MAY gain additional privileges through the overlay,
> +   compared to direct access on underlying lower or upper filesystems

All you need to fix is this adjustment of indent.
Don't do other unindents please

>  
> -This is achieved by performing two permission checks on each access
> +This is achieved by performing two permission checks on each access:
>  
> - a) check if current task is allowed access based on local DAC (owner,
> -    group, mode and posix acl), as well as MAC checks
> +a) check if current task is allowed access based on local DAC (owner,
> +group, mode and posix acl), as well as MAC checks
>  
> - b) check if mounting task would be allowed real operation on lower or
> -    upper layer based on underlying filesystem permissions, again including
> -    MAC checks
> +b) check if mounting task would be allowed real operation on lower or
> +upper layer based on underlying filesystem permissions, again including
> +MAC checks

Your workaround harms the readability very badly.
Don't break the construct of enumerated (or numbered) list in rst.

For the specification of enumerated list, please see:

https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#enumerated-lists

If there is a rst parser who fails to recognize some of the defined
list structure, fix such a parser please!

>  
>  Check (a) ensures consistency (1) since owner, group, mode and posix acls
>  are copied up.  On the other hand it can result in server enforced
> @@ -311,11 +311,11 @@ to create setups where the consistency rule (1) does not hold; normally,
>  however, the mounting task will have sufficient privileges to perform all
>  operations.
>  
> -Another way to demonstrate this model is drawing parallels between
> +Another way to demonstrate this model is drawing parallels between::
>  
>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,... /merged
>  
> -and
> +and::
>  
>    cp -a /lower /upper
>    mount --bind /upper /merged
> @@ -328,7 +328,7 @@ Multiple lower layers
>  ---------------------
>  
>  Multiple lower layers can now be given using the colon (":") as a
> -separator character between the directory names.  For example:
> +separator character between the directory names.  For example::
>  
>    mount -t overlay overlay -olowerdir=/lower1:/lower2:/lower3 /merged
>  
> @@ -340,13 +340,13 @@ rightmost one and going left.  In the above example lower1 will be the
>  top, lower2 the middle and lower3 the bottom layer.
>  
>  Note: directory names containing colons can be provided as lower layer by
> -escaping the colons with a single backslash.  For example:
> +escaping the colons with a single backslash.  For example::
>  
>    mount -t overlay overlay -olowerdir=/a\:lower\:\:dir /merged
>  
>  Since kernel version v6.8, directory names containing colons can also
>  be configured as lower layer using the "lowerdir+" mount options and the
> -fsconfig syscall from new mount api.  For example:
> +fsconfig syscall from new mount api.  For example::
>  
>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/a:lower::dir", 0);
>  
> @@ -390,11 +390,11 @@ Data-only lower layers
>  With "metacopy" feature enabled, an overlayfs regular file may be a composition
>  of information from up to three different layers:
>  
> - 1) metadata from a file in the upper layer
> +1) metadata from a file in the upper layer
>  
> - 2) st_ino and st_dev object identifier from a file in a lower layer
> +2) st_ino and st_dev object identifier from a file in a lower layer
>  
> - 3) data from a file in another lower layer (further below)
> +3) data from a file in another lower layer (further below)

Ditto.

>  
>  The "lower data" file can be on any lower layer, except from the top most
>  lower layer.
> @@ -405,7 +405,7 @@ A normal lower layer is not allowed to be below a data-only layer, so single
>  colon separators are not allowed to the right of double colon ("::") separators.
>  
>  
> -For example:
> +For example::
>  
>    mount -t overlay overlay -olowerdir=/l1:/l2:/l3::/do1::/do2 /merged
>  
> @@ -419,7 +419,7 @@ to the absolute path of the "lower data" file in the "data-only" lower layer.
>  
>  Since kernel version v6.8, "data-only" lower layers can also be added using
>  the "datadir+" mount options and the fsconfig syscall from new mount api.
> -For example:
> +For example::
>  
>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l1", 0);
>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l2", 0);
> @@ -429,7 +429,7 @@ For example:
>  
>  
>  fs-verity support
> -----------------------
> +-----------------
>  
>  During metadata copy up of a lower file, if the source file has
>  fs-verity enabled and overlay verity support is enabled, then the
> @@ -653,9 +653,10 @@ following rules apply:
>     encode an upper file handle from upper inode
>  
>  The encoded overlay file handle includes:
> - - Header including path type information (e.g. lower/upper)
> - - UUID of the underlying filesystem
> - - Underlying filesystem encoding of underlying inode
> +
> +- Header including path type information (e.g. lower/upper)
> +- UUID of the underlying filesystem
> +- Underlying filesystem encoding of underlying inode

Ditto.

>  
>  This encoding format is identical to the encoding format file handles that
>  are stored in extended attribute "trusted.overlay.origin".
> @@ -773,9 +774,9 @@ Testsuite
>  There's a testsuite originally developed by David Howells and currently
>  maintained by Amir Goldstein at:
>  
> -  https://github.com/amir73il/unionmount-testsuite.git
> +https://github.com/amir73il/unionmount-testsuite.git
>  
> -Run as root:
> +Run as root::
>  
>    # cd unionmount-testsuite
>    # ./run --ov --verify
> -- 
> 2.34.1

BR,
Akira

Re: [PATCH v2 2/2] overlayfs.rst: fix ReST formatting

[Amir Goldstein]

On Fri, Dec 15, 2023 at 4:07 AM Akira Yokosawa <akiyks@gmail.com> wrote:
>
> Hi,
>
> On Wed, 13 Dec 2023 14:34:22 +0200, Amir Goldstein wrote:
> > Fix some indentation issues and fix missing newlines in quoted text
> > by converting quoted text to code blocks.
> >
> > Unindent a) b) enumerated list to workaround github displaying it
> > as numbered list.
>
> I don't think we need to work around github's weird behavior around
> enumerated lists.  What matters for us is what Sphinx (+ our own
> extensions) ends up generating.
>
> The corresponding html page rendered by Sphinx is at:
> https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#permission-model
>
> It does not look perfect, but at least it preserves enumeration by
> number and alphabet.
>

ok.

> I'd suggest reporting github about the minor breakage of their
> rst renderer.
>
> Further comments below:
>
> >
> > Reported-by: Christian Brauner <brauner@kernel.org>
> > Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> > Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> > ---
> >  Documentation/filesystems/overlayfs.rst | 63 +++++++++++++------------
> >  1 file changed, 32 insertions(+), 31 deletions(-)
> >
> > diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> > index 926396fdc5eb..a36f3a2a2d4b 100644
> > --- a/Documentation/filesystems/overlayfs.rst
> > +++ b/Documentation/filesystems/overlayfs.rst
> > @@ -118,7 +118,7 @@ Where both upper and lower objects are directories, a merged directory
> >  is formed.
> >
> >  At mount time, the two directories given as mount options "lowerdir" and
> > -"upperdir" are combined into a merged directory:
> > +"upperdir" are combined into a merged directory::
> >
> >    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
> >    workdir=/work /merged
> > @@ -174,10 +174,10 @@ programs.
> >  seek offsets are assigned sequentially when the directories are read.
> >  Thus if
> >
> > -  - read part of a directory
> > -  - remember an offset, and close the directory
> > -  - re-open the directory some time later
> > -  - seek to the remembered offset
> > +- read part of a directory
> > +- remember an offset, and close the directory
> > +- re-open the directory some time later
> > +- seek to the remembered offset
>
> To my eyes, unindent spoils the readability of this file as pure
> plain text.  Please don't do this.
>

Ok. I see what you mean.
I restored a single space indent.
I don't see why double space is called for and it is inconsistent
with indentation in the rest of the doc.

> >
> >  there may be little correlation between the old and new locations in
> >  the list of filenames, particularly if anything has changed in the
> > @@ -285,21 +285,21 @@ Permission model
> >
> >  Permission checking in the overlay filesystem follows these principles:
> >
> > - 1) permission check SHOULD return the same result before and after copy up
> > +1) permission check SHOULD return the same result before and after copy up
> >
> > - 2) task creating the overlay mount MUST NOT gain additional privileges
> > +2) task creating the overlay mount MUST NOT gain additional privileges
> >
> > - 3) non-mounting task MAY gain additional privileges through the overlay,
> > - compared to direct access on underlying lower or upper filesystems
> > +3) non-mounting task MAY gain additional privileges through the overlay,
> > +   compared to direct access on underlying lower or upper filesystems
>
> All you need to fix is this adjustment of indent.
> Don't do other unindents please
>

OK. I also fixed the same indents in "Non-standard behavior".

> >
> > -This is achieved by performing two permission checks on each access
> > +This is achieved by performing two permission checks on each access:
> >
> > - a) check if current task is allowed access based on local DAC (owner,
> > -    group, mode and posix acl), as well as MAC checks
> > +a) check if current task is allowed access based on local DAC (owner,
> > +group, mode and posix acl), as well as MAC checks
> >
> > - b) check if mounting task would be allowed real operation on lower or
> > -    upper layer based on underlying filesystem permissions, again including
> > -    MAC checks
> > +b) check if mounting task would be allowed real operation on lower or
> > +upper layer based on underlying filesystem permissions, again including
> > +MAC checks
>
> Your workaround harms the readability very badly.
> Don't break the construct of enumerated (or numbered) list in rst.
>

ok.

> For the specification of enumerated list, please see:
>
> https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#enumerated-lists
>
> If there is a rst parser who fails to recognize some of the defined
> list structure, fix such a parser please!
>
> >
> >  Check (a) ensures consistency (1) since owner, group, mode and posix acls
> >  are copied up.  On the other hand it can result in server enforced
> > @@ -311,11 +311,11 @@ to create setups where the consistency rule (1) does not hold; normally,
> >  however, the mounting task will have sufficient privileges to perform all
> >  operations.
> >
> > -Another way to demonstrate this model is drawing parallels between
> > +Another way to demonstrate this model is drawing parallels between::
> >
> >    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,... /merged
> >
> > -and
> > +and::
> >
> >    cp -a /lower /upper
> >    mount --bind /upper /merged
> > @@ -328,7 +328,7 @@ Multiple lower layers
> >  ---------------------
> >
> >  Multiple lower layers can now be given using the colon (":") as a
> > -separator character between the directory names.  For example:
> > +separator character between the directory names.  For example::
> >
> >    mount -t overlay overlay -olowerdir=/lower1:/lower2:/lower3 /merged
> >
> > @@ -340,13 +340,13 @@ rightmost one and going left.  In the above example lower1 will be the
> >  top, lower2 the middle and lower3 the bottom layer.
> >
> >  Note: directory names containing colons can be provided as lower layer by
> > -escaping the colons with a single backslash.  For example:
> > +escaping the colons with a single backslash.  For example::
> >
> >    mount -t overlay overlay -olowerdir=/a\:lower\:\:dir /merged
> >
> >  Since kernel version v6.8, directory names containing colons can also
> >  be configured as lower layer using the "lowerdir+" mount options and the
> > -fsconfig syscall from new mount api.  For example:
> > +fsconfig syscall from new mount api.  For example::
> >
> >    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/a:lower::dir", 0);
> >
> > @@ -390,11 +390,11 @@ Data-only lower layers
> >  With "metacopy" feature enabled, an overlayfs regular file may be a composition
> >  of information from up to three different layers:
> >
> > - 1) metadata from a file in the upper layer
> > +1) metadata from a file in the upper layer
> >
> > - 2) st_ino and st_dev object identifier from a file in a lower layer
> > +2) st_ino and st_dev object identifier from a file in a lower layer
> >
> > - 3) data from a file in another lower layer (further below)
> > +3) data from a file in another lower layer (further below)
>
> Ditto.
>
> >
> >  The "lower data" file can be on any lower layer, except from the top most
> >  lower layer.
> > @@ -405,7 +405,7 @@ A normal lower layer is not allowed to be below a data-only layer, so single
> >  colon separators are not allowed to the right of double colon ("::") separators.
> >
> >
> > -For example:
> > +For example::
> >
> >    mount -t overlay overlay -olowerdir=/l1:/l2:/l3::/do1::/do2 /merged
> >
> > @@ -419,7 +419,7 @@ to the absolute path of the "lower data" file in the "data-only" lower layer.
> >
> >  Since kernel version v6.8, "data-only" lower layers can also be added using
> >  the "datadir+" mount options and the fsconfig syscall from new mount api.
> > -For example:
> > +For example::
> >
> >    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l1", 0);
> >    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l2", 0);
> > @@ -429,7 +429,7 @@ For example:
> >
> >
> >  fs-verity support
> > -----------------------
> > +-----------------
> >
> >  During metadata copy up of a lower file, if the source file has
> >  fs-verity enabled and overlay verity support is enabled, then the
> > @@ -653,9 +653,10 @@ following rules apply:
> >     encode an upper file handle from upper inode
> >
> >  The encoded overlay file handle includes:
> > - - Header including path type information (e.g. lower/upper)
> > - - UUID of the underlying filesystem
> > - - Underlying filesystem encoding of underlying inode
> > +
> > +- Header including path type information (e.g. lower/upper)
> > +- UUID of the underlying filesystem
> > +- Underlying filesystem encoding of underlying inode
>
> Ditto.
>

ok, but inconsistent indentation between numbered and bullet list is
also not nice:
https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#nfs-export

so I kept this indent and I also indented the non-indented numbered lists
in this section to conform to the rest of the numbered lists in this doc.

I've pushed the fixes to overlayfs-next.
Kept RVB from Bagas, because your comment about the unindent is
aligned with Bagas' initial review comment.

Thanks,
Amir.

Re: [PATCH v2 2/2] overlayfs.rst: fix ReST formatting

[Akira Yokosawa]

Hi Amir,

On 2023/12/15 17:00, Amir Goldstein wrote:
> On Fri, Dec 15, 2023 at 4:07 AM Akira Yokosawa <akiyks@gmail.com> wrote:
>>
>> Hi,
>>
>> On Wed, 13 Dec 2023 14:34:22 +0200, Amir Goldstein wrote:
>>> Fix some indentation issues and fix missing newlines in quoted text
>>> by converting quoted text to code blocks.
>>>
>>> Unindent a) b) enumerated list to workaround github displaying it
>>> as numbered list.
>>
>> I don't think we need to work around github's weird behavior around
>> enumerated lists.  What matters for us is what Sphinx (+ our own
>> extensions) ends up generating.
>>
>> The corresponding html page rendered by Sphinx is at:
>> https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#permission-model
>>
>> It does not look perfect, but at least it preserves enumeration by
>> number and alphabet.
>>
> 
> ok.
> 
>> I'd suggest reporting github about the minor breakage of their
>> rst renderer.
>>
>> Further comments below:
>>
>>>
>>> Reported-by: Christian Brauner <brauner@kernel.org>
>>> Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
>>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>>> ---
>>>  Documentation/filesystems/overlayfs.rst | 63 +++++++++++++------------
>>>  1 file changed, 32 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
>>> index 926396fdc5eb..a36f3a2a2d4b 100644
>>> --- a/Documentation/filesystems/overlayfs.rst
>>> +++ b/Documentation/filesystems/overlayfs.rst
>>> @@ -118,7 +118,7 @@ Where both upper and lower objects are directories, a merged directory
>>>  is formed.
>>>
>>>  At mount time, the two directories given as mount options "lowerdir" and
>>> -"upperdir" are combined into a merged directory:
>>> +"upperdir" are combined into a merged directory::
>>>
>>>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
>>>    workdir=/work /merged
>>> @@ -174,10 +174,10 @@ programs.
>>>  seek offsets are assigned sequentially when the directories are read.
>>>  Thus if
>>>
>>> -  - read part of a directory
>>> -  - remember an offset, and close the directory
>>> -  - re-open the directory some time later
>>> -  - seek to the remembered offset
>>> +- read part of a directory
>>> +- remember an offset, and close the directory
>>> +- re-open the directory some time later
>>> +- seek to the remembered offset
>>
>> To my eyes, unindent spoils the readability of this file as pure
>> plain text.  Please don't do this.
>>
> 
> Ok. I see what you mean.
> I restored a single space indent.
> I don't see why double space is called for and it is inconsistent
> with indentation in the rest of the doc.
> 
>>>
>>>  there may be little correlation between the old and new locations in
>>>  the list of filenames, particularly if anything has changed in the
>>> @@ -285,21 +285,21 @@ Permission model
>>>
>>>  Permission checking in the overlay filesystem follows these principles:
>>>
>>> - 1) permission check SHOULD return the same result before and after copy up
>>> +1) permission check SHOULD return the same result before and after copy up
>>>
>>> - 2) task creating the overlay mount MUST NOT gain additional privileges
>>> +2) task creating the overlay mount MUST NOT gain additional privileges
>>>
>>> - 3) non-mounting task MAY gain additional privileges through the overlay,
>>> - compared to direct access on underlying lower or upper filesystems
>>> +3) non-mounting task MAY gain additional privileges through the overlay,
>>> +   compared to direct access on underlying lower or upper filesystems
>>
>> All you need to fix is this adjustment of indent.
>> Don't do other unindents please
>>
> 
> OK. I also fixed the same indents in "Non-standard behavior".
> 
>>>
>>> -This is achieved by performing two permission checks on each access
>>> +This is achieved by performing two permission checks on each access:
>>>
>>> - a) check if current task is allowed access based on local DAC (owner,
>>> -    group, mode and posix acl), as well as MAC checks
>>> +a) check if current task is allowed access based on local DAC (owner,
>>> +group, mode and posix acl), as well as MAC checks
>>>
>>> - b) check if mounting task would be allowed real operation on lower or
>>> -    upper layer based on underlying filesystem permissions, again including
>>> -    MAC checks
>>> +b) check if mounting task would be allowed real operation on lower or
>>> +upper layer based on underlying filesystem permissions, again including
>>> +MAC checks
>>
>> Your workaround harms the readability very badly.
>> Don't break the construct of enumerated (or numbered) list in rst.
>>
> 
> ok.
> 
>> For the specification of enumerated list, please see:
>>
>> https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#enumerated-lists
>>
>> If there is a rst parser who fails to recognize some of the defined
>> list structure, fix such a parser please!
>>
>>>
>>>  Check (a) ensures consistency (1) since owner, group, mode and posix acls
>>>  are copied up.  On the other hand it can result in server enforced
>>> @@ -311,11 +311,11 @@ to create setups where the consistency rule (1) does not hold; normally,
>>>  however, the mounting task will have sufficient privileges to perform all
>>>  operations.
>>>
>>> -Another way to demonstrate this model is drawing parallels between
>>> +Another way to demonstrate this model is drawing parallels between::
>>>
>>>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,... /merged
>>>
>>> -and
>>> +and::
>>>
>>>    cp -a /lower /upper
>>>    mount --bind /upper /merged
>>> @@ -328,7 +328,7 @@ Multiple lower layers
>>>  ---------------------
>>>
>>>  Multiple lower layers can now be given using the colon (":") as a
>>> -separator character between the directory names.  For example:
>>> +separator character between the directory names.  For example::
>>>
>>>    mount -t overlay overlay -olowerdir=/lower1:/lower2:/lower3 /merged
>>>
>>> @@ -340,13 +340,13 @@ rightmost one and going left.  In the above example lower1 will be the
>>>  top, lower2 the middle and lower3 the bottom layer.
>>>
>>>  Note: directory names containing colons can be provided as lower layer by
>>> -escaping the colons with a single backslash.  For example:
>>> +escaping the colons with a single backslash.  For example::
>>>
>>>    mount -t overlay overlay -olowerdir=/a\:lower\:\:dir /merged
>>>
>>>  Since kernel version v6.8, directory names containing colons can also
>>>  be configured as lower layer using the "lowerdir+" mount options and the
>>> -fsconfig syscall from new mount api.  For example:
>>> +fsconfig syscall from new mount api.  For example::
>>>
>>>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/a:lower::dir", 0);
>>>
>>> @@ -390,11 +390,11 @@ Data-only lower layers
>>>  With "metacopy" feature enabled, an overlayfs regular file may be a composition
>>>  of information from up to three different layers:
>>>
>>> - 1) metadata from a file in the upper layer
>>> +1) metadata from a file in the upper layer
>>>
>>> - 2) st_ino and st_dev object identifier from a file in a lower layer
>>> +2) st_ino and st_dev object identifier from a file in a lower layer
>>>
>>> - 3) data from a file in another lower layer (further below)
>>> +3) data from a file in another lower layer (further below)
>>
>> Ditto.
>>
>>>
>>>  The "lower data" file can be on any lower layer, except from the top most
>>>  lower layer.
>>> @@ -405,7 +405,7 @@ A normal lower layer is not allowed to be below a data-only layer, so single
>>>  colon separators are not allowed to the right of double colon ("::") separators.
>>>
>>>
>>> -For example:
>>> +For example::
>>>
>>>    mount -t overlay overlay -olowerdir=/l1:/l2:/l3::/do1::/do2 /merged
>>>
>>> @@ -419,7 +419,7 @@ to the absolute path of the "lower data" file in the "data-only" lower layer.
>>>
>>>  Since kernel version v6.8, "data-only" lower layers can also be added using
>>>  the "datadir+" mount options and the fsconfig syscall from new mount api.
>>> -For example:
>>> +For example::
>>>
>>>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l1", 0);
>>>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l2", 0);
>>> @@ -429,7 +429,7 @@ For example:
>>>
>>>
>>>  fs-verity support
>>> -----------------------
>>> +-----------------
>>>
>>>  During metadata copy up of a lower file, if the source file has
>>>  fs-verity enabled and overlay verity support is enabled, then the
>>> @@ -653,9 +653,10 @@ following rules apply:
>>>     encode an upper file handle from upper inode
>>>
>>>  The encoded overlay file handle includes:
>>> - - Header including path type information (e.g. lower/upper)
>>> - - UUID of the underlying filesystem
>>> - - Underlying filesystem encoding of underlying inode
>>> +
>>> +- Header including path type information (e.g. lower/upper)
>>> +- UUID of the underlying filesystem
>>> +- Underlying filesystem encoding of underlying inode
>>
>> Ditto.
>>
> 
> ok, but inconsistent indentation between numbered and bullet list is
> also not nice:
> https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#nfs-export

I agree.

> 
> so I kept this indent and I also indented the non-indented numbered lists
> in this section to conform to the rest of the numbered lists in this doc.
> 
> I've pushed the fixes to overlayfs-next.

OK. I'm looking at commit 4552f4b1be08 ("overlayfs.rst: fix ReST formatting").

It looks reasonable to me.
If you'd like, feel free to add

Reviewed-by: Akira Yokosawa <akiyks@gmail.com>

    Regards, Akira

> Kept RVB from Bagas, because your comment about the unindent is
> aligned with Bagas' initial review comment.
> 
> Thanks,
> Amir.

Re: [PATCH v2 2/2] overlayfs.rst: fix ReST formatting

[Amir Goldstein]

On Fri, Dec 15, 2023 at 11:31 AM Akira Yokosawa <akiyks@gmail.com> wrote:
>
> Hi Amir,
>
> On 2023/12/15 17:00, Amir Goldstein wrote:
> > On Fri, Dec 15, 2023 at 4:07 AM Akira Yokosawa <akiyks@gmail.com> wrote:
> >>
> >> Hi,
> >>
> >> On Wed, 13 Dec 2023 14:34:22 +0200, Amir Goldstein wrote:
> >>> Fix some indentation issues and fix missing newlines in quoted text
> >>> by converting quoted text to code blocks.
> >>>
> >>> Unindent a) b) enumerated list to workaround github displaying it
> >>> as numbered list.
> >>
> >> I don't think we need to work around github's weird behavior around
> >> enumerated lists.  What matters for us is what Sphinx (+ our own
> >> extensions) ends up generating.
> >>
> >> The corresponding html page rendered by Sphinx is at:
> >> https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#permission-model
> >>
> >> It does not look perfect, but at least it preserves enumeration by
> >> number and alphabet.
> >>
> >
> > ok.
> >
> >> I'd suggest reporting github about the minor breakage of their
> >> rst renderer.
> >>
> >> Further comments below:
> >>
> >>>
> >>> Reported-by: Christian Brauner <brauner@kernel.org>
> >>> Suggested-by: Bagas Sanjaya <bagasdotme@gmail.com>
> >>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
> >>> ---
> >>>  Documentation/filesystems/overlayfs.rst | 63 +++++++++++++------------
> >>>  1 file changed, 32 insertions(+), 31 deletions(-)
> >>>
> >>> diff --git a/Documentation/filesystems/overlayfs.rst b/Documentation/filesystems/overlayfs.rst
> >>> index 926396fdc5eb..a36f3a2a2d4b 100644
> >>> --- a/Documentation/filesystems/overlayfs.rst
> >>> +++ b/Documentation/filesystems/overlayfs.rst
> >>> @@ -118,7 +118,7 @@ Where both upper and lower objects are directories, a merged directory
> >>>  is formed.
> >>>
> >>>  At mount time, the two directories given as mount options "lowerdir" and
> >>> -"upperdir" are combined into a merged directory:
> >>> +"upperdir" are combined into a merged directory::
> >>>
> >>>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,\
> >>>    workdir=/work /merged
> >>> @@ -174,10 +174,10 @@ programs.
> >>>  seek offsets are assigned sequentially when the directories are read.
> >>>  Thus if
> >>>
> >>> -  - read part of a directory
> >>> -  - remember an offset, and close the directory
> >>> -  - re-open the directory some time later
> >>> -  - seek to the remembered offset
> >>> +- read part of a directory
> >>> +- remember an offset, and close the directory
> >>> +- re-open the directory some time later
> >>> +- seek to the remembered offset
> >>
> >> To my eyes, unindent spoils the readability of this file as pure
> >> plain text.  Please don't do this.
> >>
> >
> > Ok. I see what you mean.
> > I restored a single space indent.
> > I don't see why double space is called for and it is inconsistent
> > with indentation in the rest of the doc.
> >
> >>>
> >>>  there may be little correlation between the old and new locations in
> >>>  the list of filenames, particularly if anything has changed in the
> >>> @@ -285,21 +285,21 @@ Permission model
> >>>
> >>>  Permission checking in the overlay filesystem follows these principles:
> >>>
> >>> - 1) permission check SHOULD return the same result before and after copy up
> >>> +1) permission check SHOULD return the same result before and after copy up
> >>>
> >>> - 2) task creating the overlay mount MUST NOT gain additional privileges
> >>> +2) task creating the overlay mount MUST NOT gain additional privileges
> >>>
> >>> - 3) non-mounting task MAY gain additional privileges through the overlay,
> >>> - compared to direct access on underlying lower or upper filesystems
> >>> +3) non-mounting task MAY gain additional privileges through the overlay,
> >>> +   compared to direct access on underlying lower or upper filesystems
> >>
> >> All you need to fix is this adjustment of indent.
> >> Don't do other unindents please
> >>
> >
> > OK. I also fixed the same indents in "Non-standard behavior".
> >
> >>>
> >>> -This is achieved by performing two permission checks on each access
> >>> +This is achieved by performing two permission checks on each access:
> >>>
> >>> - a) check if current task is allowed access based on local DAC (owner,
> >>> -    group, mode and posix acl), as well as MAC checks
> >>> +a) check if current task is allowed access based on local DAC (owner,
> >>> +group, mode and posix acl), as well as MAC checks
> >>>
> >>> - b) check if mounting task would be allowed real operation on lower or
> >>> -    upper layer based on underlying filesystem permissions, again including
> >>> -    MAC checks
> >>> +b) check if mounting task would be allowed real operation on lower or
> >>> +upper layer based on underlying filesystem permissions, again including
> >>> +MAC checks
> >>
> >> Your workaround harms the readability very badly.
> >> Don't break the construct of enumerated (or numbered) list in rst.
> >>
> >
> > ok.
> >
> >> For the specification of enumerated list, please see:
> >>
> >> https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#enumerated-lists
> >>
> >> If there is a rst parser who fails to recognize some of the defined
> >> list structure, fix such a parser please!
> >>
> >>>
> >>>  Check (a) ensures consistency (1) since owner, group, mode and posix acls
> >>>  are copied up.  On the other hand it can result in server enforced
> >>> @@ -311,11 +311,11 @@ to create setups where the consistency rule (1) does not hold; normally,
> >>>  however, the mounting task will have sufficient privileges to perform all
> >>>  operations.
> >>>
> >>> -Another way to demonstrate this model is drawing parallels between
> >>> +Another way to demonstrate this model is drawing parallels between::
> >>>
> >>>    mount -t overlay overlay -olowerdir=/lower,upperdir=/upper,... /merged
> >>>
> >>> -and
> >>> +and::
> >>>
> >>>    cp -a /lower /upper
> >>>    mount --bind /upper /merged
> >>> @@ -328,7 +328,7 @@ Multiple lower layers
> >>>  ---------------------
> >>>
> >>>  Multiple lower layers can now be given using the colon (":") as a
> >>> -separator character between the directory names.  For example:
> >>> +separator character between the directory names.  For example::
> >>>
> >>>    mount -t overlay overlay -olowerdir=/lower1:/lower2:/lower3 /merged
> >>>
> >>> @@ -340,13 +340,13 @@ rightmost one and going left.  In the above example lower1 will be the
> >>>  top, lower2 the middle and lower3 the bottom layer.
> >>>
> >>>  Note: directory names containing colons can be provided as lower layer by
> >>> -escaping the colons with a single backslash.  For example:
> >>> +escaping the colons with a single backslash.  For example::
> >>>
> >>>    mount -t overlay overlay -olowerdir=/a\:lower\:\:dir /merged
> >>>
> >>>  Since kernel version v6.8, directory names containing colons can also
> >>>  be configured as lower layer using the "lowerdir+" mount options and the
> >>> -fsconfig syscall from new mount api.  For example:
> >>> +fsconfig syscall from new mount api.  For example::
> >>>
> >>>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/a:lower::dir", 0);
> >>>
> >>> @@ -390,11 +390,11 @@ Data-only lower layers
> >>>  With "metacopy" feature enabled, an overlayfs regular file may be a composition
> >>>  of information from up to three different layers:
> >>>
> >>> - 1) metadata from a file in the upper layer
> >>> +1) metadata from a file in the upper layer
> >>>
> >>> - 2) st_ino and st_dev object identifier from a file in a lower layer
> >>> +2) st_ino and st_dev object identifier from a file in a lower layer
> >>>
> >>> - 3) data from a file in another lower layer (further below)
> >>> +3) data from a file in another lower layer (further below)
> >>
> >> Ditto.
> >>
> >>>
> >>>  The "lower data" file can be on any lower layer, except from the top most
> >>>  lower layer.
> >>> @@ -405,7 +405,7 @@ A normal lower layer is not allowed to be below a data-only layer, so single
> >>>  colon separators are not allowed to the right of double colon ("::") separators.
> >>>
> >>>
> >>> -For example:
> >>> +For example::
> >>>
> >>>    mount -t overlay overlay -olowerdir=/l1:/l2:/l3::/do1::/do2 /merged
> >>>
> >>> @@ -419,7 +419,7 @@ to the absolute path of the "lower data" file in the "data-only" lower layer.
> >>>
> >>>  Since kernel version v6.8, "data-only" lower layers can also be added using
> >>>  the "datadir+" mount options and the fsconfig syscall from new mount api.
> >>> -For example:
> >>> +For example::
> >>>
> >>>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l1", 0);
> >>>    fsconfig(fs_fd, FSCONFIG_SET_STRING, "lowerdir+", "/l2", 0);
> >>> @@ -429,7 +429,7 @@ For example:
> >>>
> >>>
> >>>  fs-verity support
> >>> -----------------------
> >>> +-----------------
> >>>
> >>>  During metadata copy up of a lower file, if the source file has
> >>>  fs-verity enabled and overlay verity support is enabled, then the
> >>> @@ -653,9 +653,10 @@ following rules apply:
> >>>     encode an upper file handle from upper inode
> >>>
> >>>  The encoded overlay file handle includes:
> >>> - - Header including path type information (e.g. lower/upper)
> >>> - - UUID of the underlying filesystem
> >>> - - Underlying filesystem encoding of underlying inode
> >>> +
> >>> +- Header including path type information (e.g. lower/upper)
> >>> +- UUID of the underlying filesystem
> >>> +- Underlying filesystem encoding of underlying inode
> >>
> >> Ditto.
> >>
> >
> > ok, but inconsistent indentation between numbered and bullet list is
> > also not nice:
> > https://www.kernel.org/doc/html/latest/filesystems/overlayfs.html#nfs-export
>
> I agree.
>
> >
> > so I kept this indent and I also indented the non-indented numbered lists
> > in this section to conform to the rest of the numbered lists in this doc.
> >
> > I've pushed the fixes to overlayfs-next.
>
> OK. I'm looking at commit 4552f4b1be08 ("overlayfs.rst: fix ReST formatting").
>
> It looks reasonable to me.
> If you'd like, feel free to add
>
> Reviewed-by: Akira Yokosawa <akiyks@gmail.com>
>

Done.

Thanks!
Amir.