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 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.