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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 15, 2023 at 11:31 AM Akira Yokosawa <akiyks@xxxxxxxxx> wrote:
>
> Hi Amir,
>
> On 2023/12/15 17:00, Amir Goldstein wrote:
> > On Fri, Dec 15, 2023 at 4:07 AM Akira Yokosawa <akiyks@xxxxxxxxx> 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@xxxxxxxxxx>
> >>> Suggested-by: Bagas Sanjaya <bagasdotme@xxxxxxxxx>
> >>> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> >>> ---
> >>>  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@xxxxxxxxx>
>

Done.

Thanks!
Amir.





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux