On Wed, Nov 29, 2017 at 10:54:47AM -0500, Vivek Goyal wrote:
> If file is metacopy only, it is possible that lower is encrypted while
> other is not. In that case, report file as encrypted (despite the fact
> that only data is encrypted while metadata is not).
>
> Similarly if lower is compressed, report that in stat for a metacopy
> only file.
>
Hi Amir,
Thinking more about this patch. Taking union of lower and upper attributes
(encryption and compressed) does not feel right.
Right now if lower is compressed and upper is not (metacopy), then we will
report file as compressed. But if lower is not compressed while upper is,
still we will report file as compressed. So there is an anomaly here.
I think, compression and encryption primarily represent the data state.
So always report the state from inode which has file data (lower). And
don't take union with upper metacopy only inode. Because state in
upper inode does not matter till data is copied up.
This will also simplify my implementation when supporting metacopy in
middle layer. That way I don't have to do getattr() on all intermediate
metacopy inodes in the chain and take a union. Instead I can go to lowest
most data inode and take encryption and compression attributes from there.
I am not sure why did we decide to take union of lower and upper attrs.
Right now it does not seem to make sense to me.
Thanks
Vivek
> Reviewed-by: Amir Goldstein <amir73il@xxxxxxxxx>
> Signed-off-by: Vivek Goyal <vgoyal@xxxxxxxxxx>
> ---
> fs/overlayfs/inode.c | 22 +++++++++++++++++++++-
> 1 file changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
> index 605308a9d60f..0b83efaac9e0 100644
> --- a/fs/overlayfs/inode.c
> +++ b/fs/overlayfs/inode.c
> @@ -66,6 +66,24 @@ int ovl_setattr(struct dentry *dentry, struct iattr *attr)
> return err;
> }
>
> +#define OVL_STATX_ATTR_MASK (STATX_ATTR_ENCRYPTED | STATX_ATTR_COMPRESSED)
> +
> +static void ovl_stat_fix_attributes(struct kstat *ustat, struct kstat *lstat)
> +{
> + unsigned int attr_mask, attr;
> +
> + attr_mask = lstat->attributes_mask & OVL_STATX_ATTR_MASK;
> + if (!attr_mask)
> + return;
> +
> + attr = lstat->attributes & attr_mask;
> + if (!attr)
> + return;
> +
> + ustat->attributes_mask |= attr_mask;
> + ustat->attributes |= attr;
> +}
> +
> int ovl_getattr(const struct path *path, struct kstat *stat,
> u32 request_mask, unsigned int flags)
> {
> @@ -123,8 +141,10 @@ int ovl_getattr(const struct path *path, struct kstat *stat,
> else
> stat->dev = ovl_get_pseudo_dev(dentry);
>
> - if (metacopy)
> + if (metacopy) {
> stat->blocks = lowerstat.blocks;
> + ovl_stat_fix_attributes(stat, &lowerstat);
> + }
> }
> if (samefs) {
> /*
> --
> 2.13.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
