Re: [PATCH v4] ovl: mark xwhiteouts directory with overlay.opaque='x'

[Amir Goldstein <amir73il@xxxxxxxxx>]

On Tue, Jan 23, 2024 at 11:54 AM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Mon, 22 Jan 2024 at 20:51, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> > +void ovl_layer_set_xwhiteouts(struct ovl_fs *ofs,
> > +                             const struct ovl_layer *layer)
> > +{
> > +       if (layer->has_xwhiteouts)
> > +               return;
> > +
> > +       /* Write once to read-mostly layer properties */
> > +       ((struct ovl_layer *)layer)->has_xwhiteouts = true;
>
> The cast is wrong.  After this change *ofs->layers is no longer const,
> so it should not be marked as such.
>

Yap, I was considering this:

diff --git a/fs/overlayfs/ovl_entry.h b/fs/overlayfs/ovl_entry.h
index b26d1824bf87..cb449ab310a7 100644
--- a/fs/overlayfs/ovl_entry.h
+++ b/fs/overlayfs/ovl_entry.h
@@ -61,7 +61,7 @@ struct ovl_fs {
        unsigned int numfs;
        /* Number of data-only lower layers */
        unsigned int numdatalayer;
-       const struct ovl_layer *layers;
+       struct ovl_layer *layers;
        struct ovl_sb *fs;
        /* workbasedir is the path at workdir= mount option */
        struct dentry *workbasedir;
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index 0551ba4e3e6a..a8e17f14d7a2 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -485,7 +485,7 @@ void ovl_layer_set_xwhiteouts(struct ovl_fs *ofs,
                return;

        /* Write once to read-mostly layer properties */
-       ((struct ovl_layer *)layer)->has_xwhiteouts = true;
+       ofs->layers[layer->idx].has_xwhiteouts = true;
 }

--

It's still a bit ugly that @layer argument to this function is const,
but the first likely check is really const and I do not want to make
ovl_path::layer non-const.

Do you agree with the above change?

Thanks,
Amir.