Re: [PATCH 03/18] ovl: use wrappers to all vfs_*xattr() calls

From: Miklos Szeredi <miklos@szeredi.hu>
To: Christian Brauner <brauner@kernel.org>
Cc: "Amir Goldstein" <amir73il@gmail.com>,
	"Miklos Szeredi" <mszeredi@redhat.com>,
	"Christoph Hellwig" <hch@lst.de>,
	overlayfs <linux-unionfs@vger.kernel.org>,
	"Aleksa Sarai" <cyphar@cyphar.com>,
	"Giuseppe Scrivano" <gscrivan@redhat.com>,
	"Rodrigo Campos Catelin" <rodrigoca@microsoft.com>,
	"Seth Forshee" <sforshee@digitalocean.com>,
	"Luca Bocassi" <luca.boccassi@microsoft.com>,
	"Lennart Poettering" <mzxreary@0pointer.de>,
	"Stéphane Graber" <stgraber@ubuntu.com>
Subject: Re: [PATCH 03/18] ovl: use wrappers to all vfs_*xattr() calls
Date: Tue, 29 Mar 2022 13:18:45 +0200	[thread overview]
Message-ID: <CAJfpegvH32i1xiSTo_Z3XQhTirPjNmExshVsZCR=MjPoeoDQfA@mail.gmail.com> (raw)
In-Reply-To: <20220329103526.1207086-4-brauner@kernel.org>

On Tue, 29 Mar 2022 at 12:36, Christian Brauner <brauner@kernel.org> wrote:
>

> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index 2cd5741c873b..6a53ca0d2c96 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -183,10 +183,9 @@ static inline int ovl_do_symlink(struct inode *dir, struct dentry *dentry,
>  }
>
>  static inline ssize_t ovl_do_getxattr(struct ovl_fs *ofs, struct dentry *dentry,
> -                                     enum ovl_xattr ox, void *value,
> +                                     const char *name, void *value,
>                                       size_t size)
>  {
> -       const char *name = ovl_xattr(ofs, ox);
>         int err = vfs_getxattr(&init_user_ns, dentry, name, value, size);
>         int len = (value && err > 0) ? err : 0;

Previously direct calls to vfs_*xattr() didn't print debug info.  This
was deliberate as debugging normal operations would drown out the
interesting calls.  But perhaps it doesn't matter nowadays, since
overlayfs is stable and nobody enables debugging anymore...

Anyway, I feel that this should be a separate change, or at least
documented in the patch header.

Thanks,
Miklos