Re: [PATCH v5 10/13] fs/configfs: Add a callback to determine attribute visibility

[Christophe JAILLET]

Le 05/06/2024 à 17:18, Tom Lendacky a écrit :
> In order to support dynamic decisions as to whether an attribute should be
> created, add a callback that returns a bool to indicate whether the
> attribute should be displayed. If no callback is registered, the attribute
> is displayed by default.
>
> Cc: Joel Becker <jlbec@xxxxxxxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>
> Co-developed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
> ---
>   fs/configfs/dir.c        | 10 ++++++++++
>   include/linux/configfs.h |  3 +++
>   2 files changed, 13 insertions(+)
>
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index 18677cd4e62f..43d6bde1adcc 100644
> --- a/fs/configfs/dir.c
> +++ b/fs/configfs/dir.c
> @@ -580,6 +580,7 @@ static void detach_attrs(struct config_item * item)
>   static int populate_attrs(struct config_item *item)
>   {
>   	const struct config_item_type *t = item->ci_type;
> +	struct configfs_group_operations *ops;
>   	struct configfs_attribute *attr;
>   	struct configfs_bin_attribute *bin_attr;
>   	int error = 0;
> @@ -587,14 +588,23 @@ static int populate_attrs(struct config_item *item)
>   
>   	if (!t)
>   		return -EINVAL;
> +
> +	ops = t->ct_group_ops;
> +
>   	if (t->ct_attrs) {
>   		for (i = 0; (attr = t->ct_attrs[i]) != NULL; i++) {
> +			if (ops && ops->is_visible && !ops->is_visible(item, attr, i))
> +				continue;
> +
>   			if ((error = configfs_create_file(item, attr)))
>   				break;
>   		}
>   	}
>   	if (t->ct_bin_attrs) {
>   		for (i = 0; (bin_attr = t->ct_bin_attrs[i]) != NULL; i++) {
> +			if (ops && ops->is_bin_visible && !ops->is_bin_visible(item, bin_attr, i))
> +				continue;
> +
>   			error = configfs_create_bin_file(item, bin_attr);
>   			if (error)
>   				break;
> diff --git a/include/linux/configfs.h b/include/linux/configfs.h
> index 2606711adb18..c771e9d0d0b9 100644
> --- a/include/linux/configfs.h
> +++ b/include/linux/configfs.h
> @@ -216,6 +216,9 @@ struct configfs_group_operations {
>   	struct config_group *(*make_group)(struct config_group *group, const char *name);
>   	void (*disconnect_notify)(struct config_group *group, struct config_item *item);
>   	void (*drop_item)(struct config_group *group, struct config_item *item);
> +	bool (*is_visible)(struct config_item *item, struct configfs_attribute *attr, int n);
> +	bool (*is_bin_visible)(struct config_item *item, struct configfs_bin_attribute *attr,

Hi,

Should/could this take a *const* struct configfs_bin_attribute * and a 
const struct config_item *?

I'm currently looking if if is feasible to constify struct 
configfs_attribute and co.

In my investigation, I arrived on this new API.
So, if it makes sense to constify the attr argument, it would avoid some 
later work if.

Just my 2c

CJ



> +			       int n);
>   };
>   
>   struct configfs_subsystem {