On Tue, Apr 18, 2023 at 3:02 PM Alexander Larsson <alexl@xxxxxxxxxx> wrote:
>
> On Wed, 2023-04-12 at 16:54 +0300, Amir Goldstein wrote:
> > Introduce the format lowerdir=lower1:lower2::lowerdata1:lowerdata2
> > where the lower layers after the :: separator are not merged into the
> > overlayfs merge dirs.
> >
> > The files in those layers are only meant to be accessible via
> > absolute
> > redirect from metacopy files in lower layers. Following changes will
> > implement lookup in the data layers.
> >
> > This feature was requested for composefs ostree use case, where the
> > lower data layer should only be accessiable via absolute redirects
> > from metacopy inodes.
> >
> > The lower data layers are not required to a have a unique uuid or any
> > uuid at all, because they are never used to compose the overlayfs
> > inode
> > st_ino/st_dev.
> >
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
>
> Reviewed-by: Alexander Larsson <alexl@xxxxxxxxxx>
>
> > ---
> > Documentation/filesystems/overlayfs.rst | 32 +++++++++++++++++
> > fs/overlayfs/namei.c | 4 +--
> > fs/overlayfs/ovl_entry.h | 9 +++++
> > fs/overlayfs/super.c | 46 +++++++++++++++++++++--
> > --
> > 4 files changed, 82 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/filesystems/overlayfs.rst
> > b/Documentation/filesystems/overlayfs.rst
> > index 4c76fda07645..c8e04a4f0e21 100644
> > --- a/Documentation/filesystems/overlayfs.rst
> > +++ b/Documentation/filesystems/overlayfs.rst
> > @@ -371,6 +371,38 @@ conflict with metacopy=on, and will result in an
> > error.
> > [*] redirect_dir=follow only conflicts with metacopy=on if
> > upperdir=... is
> > given.
> >
> > +
> > +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
> > +
> > + 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)
> > +
> > +The "lower data" file can be on any lower layer, except from the top
> > most
> > +lower layer.
> > +
> > +Below the top most lower layer, any number of lower most layers may
> > be defined
> > +as "data-only" lower layers, using the double collon ("::")
> > separator.
>
> "colon", not "collon"
>
> > +
> > +For example:
> > +
> > + mount -t overlay overlay -olowerdir=/lower1::/lower2:/lower3
> > /merged
> > +
> > +The paths of files in the "data-only" lower layers are not visible
> > in the
> > +merged overlayfs directories and the metadata and st_ino/st_dev of
> > files
> > +in the "data-only" lower layers are not visible in overlayfs inodes.
> > +
> > +Only the data of the files in the "data-only" lower layers may be
> > visible
> > +when a "metacopy" file in one of the lower layers above it, has a
> > "redirect"
> > +to the absolute path of the "lower data" file in the "data-only"
> > lower layer.
> > +
> > +
> > Sharing and copying layers
> > --------------------------
> >
> > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
> > index b629261324f1..ff82155b4f7e 100644
> > --- a/fs/overlayfs/namei.c
> > +++ b/fs/overlayfs/namei.c
> > @@ -356,7 +356,7 @@ int ovl_check_origin_fh(struct ovl_fs *ofs,
> > struct ovl_fh *fh, bool connected,
> > struct dentry *origin = NULL;
> > int i;
> >
> > - for (i = 1; i < ofs->numlayer; i++) {
> > + for (i = 1; i <= ovl_numlowerlayer(ofs); i++) {
> > /*
> > * If lower fs uuid is not unique among lower fs we
> > cannot match
> > * fh->uuid to layer.
> > @@ -907,7 +907,7 @@ struct dentry *ovl_lookup(struct inode *dir,
> > struct dentry *dentry,
> >
> > if (!d.stop && ovl_numlower(poe)) {
> > err = -ENOMEM;
> > - stack = ovl_stack_alloc(ofs->numlayer - 1);
> > + stack = ovl_stack_alloc(ovl_numlowerlayer(ofs));
> > if (!stack)
> > goto out_put_upper;
> > }
>
> Again, surely ovl_numlower(poe) is a better size here?
Intentional. that is changed in the following patch.
(to ovl_numlowerlayer(ofs) + 1)
As the commit message says:
"Following changes will implement lookup in the data layers."
>
> > diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
> > index 221f0cbe748e..25fabb3175cf 100644
> > --- a/fs/overlayfs/ovl_entry.h
> > +++ b/fs/overlayfs/ovl_entry.h
> > @@ -62,6 +62,8 @@ struct ovl_fs {
> > unsigned int numlayer;
> > /* Number of unique fs among layers including upper fs */
> > unsigned int numfs;
> > + /* Number of data-only lower layers */
> > + unsigned int numdatalayer;
> > const struct ovl_layer *layers;
> > struct ovl_sb *fs;
> > /* workbasedir is the path at workdir= mount option */
> > @@ -95,6 +97,13 @@ struct ovl_fs {
> > errseq_t errseq;
> > };
> >
> > +
> > +/* Number of lower layers, not including data-only layers */
> > +static inline unsigned int ovl_numlowerlayer(struct ovl_fs *ofs)
> > +{
> > + return ofs->numlayer - ofs->numdatalayer - 1;
> > +}
> > +
> > static inline struct vfsmount *ovl_upper_mnt(struct ovl_fs *ofs)
> > {
> > return ofs->layers[0].mnt;
> > diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> > index 7742aef3f3b3..3484f39a8f27 100644
> > --- a/fs/overlayfs/super.c
> > +++ b/fs/overlayfs/super.c
> > @@ -1579,6 +1579,16 @@ static int ovl_get_fsid(struct ovl_fs *ofs,
> > const struct path *path)
> > return ofs->numfs++;
> > }
> >
> > +/*
> > + * The fsid after the last lower fsid is used for the data layers.
> > + * It is a "null fs" with a null sb, null uuid, and no pseudo dev.
> > + */
> > +static int ovl_get_data_fsid(struct ovl_fs *ofs)
> > +{
> > + return ofs->numfs;
> > +}
> > +
> > +
> > static int ovl_get_layers(struct super_block *sb, struct ovl_fs
> > *ofs,
> > struct path *stack, unsigned int numlower,
> > struct ovl_layer *layers)
> > @@ -1586,11 +1596,14 @@ static int ovl_get_layers(struct super_block
> > *sb, struct ovl_fs *ofs,
> > int err;
> > unsigned int i;
> >
> > - ofs->fs = kcalloc(numlower + 1, sizeof(struct ovl_sb),
> > GFP_KERNEL);
> > + ofs->fs = kcalloc(numlower + 2, sizeof(struct ovl_sb),
> > GFP_KERNEL);
> > if (ofs->fs == NULL)
> > return -ENOMEM;
> >
> > - /* idx/fsid 0 are reserved for upper fs even with lower only
> > overlay */
> > + /*
> > + * idx/fsid 0 are reserved for upper fs even with lower only
> > overlay
> > + * and the last fsid is reserved for "null fs" of the data
> > layers.
> > + */
> > ofs->numfs++;
> >
> > /*
> > @@ -1615,7 +1628,10 @@ static int ovl_get_layers(struct super_block
> > *sb, struct ovl_fs *ofs,
> > struct inode *trap;
> > int fsid;
> >
> > - fsid = ovl_get_fsid(ofs, &stack[i]);
> > + if (i < numlower - ofs->numdatalayer)
> > + fsid = ovl_get_fsid(ofs, &stack[i]);
> > + else
> > + fsid = ovl_get_data_fsid(ofs);
> > if (fsid < 0)
> > return fsid;
> >
> > @@ -1703,6 +1719,7 @@ static int ovl_get_lowerstack(struct
> > super_block *sb, struct ovl_entry *oe,
> > int err;
> > struct path *stack = NULL;
> > struct ovl_path *lowerstack;
> > + unsigned int numlowerdata = 0;
> > unsigned int i;
> >
> > if (!ofs->config.upperdir && numlower == 1) {
> > @@ -1714,13 +1731,27 @@ static int ovl_get_lowerstack(struct
> > super_block *sb, struct ovl_entry *oe,
> > if (!stack)
> > return -ENOMEM;
> >
> > - err = -EINVAL;
> > - for (i = 0; i < numlower; i++) {
> > + for (i = 0; i < numlower;) {
> > err = ovl_lower_dir(lower, &stack[i], ofs, &sb-
> > >s_stack_depth);
> > if (err)
> > goto out_err;
> >
> > lower = strchr(lower, '\0') + 1;
> > +
> > + i++;
> > + err = -EINVAL;
> > + /* :: seperator indicates the start of lower data
> > layers */
> > + if (!*lower && i < numlower && !numlowerdata) {
> > + if (!ofs->config.metacopy) {
> > + pr_err("lower data-only dirs require
> > metacopy support.\n");
> > + goto out_err;
> > + }
> > + lower++;
> > + numlower--;
> > + ofs->numdatalayer = numlowerdata = numlower -
> > i;
> > + pr_info("using the lowest %d of %d lowerdirs
> > as data layers\n",
> > + numlowerdata, numlower);
> > + }
> > }
>
> This will handle a "::" at the end of the string as an error. Maybe it
> would be better to treat it as "zero lower data layera", to make code
> that generates mount options more regular? Not a huge issue though.
>
No reason to do that.
Code that prepares lowerdata layers should know what it is doing.
Thanks,
Amir.
