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 <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Co-developed-by: Dan Williams <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
Signed-off-by: Tom Lendacky <[email protected]>
---
fs/configfs/dir.c | 20 ++++++++++++++++++++
include/linux/configfs.h | 3 +++
2 files changed, 23 insertions(+)
diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
index 18677cd4e62f..463e66258507 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,33 @@ static int populate_attrs(struct config_item *item)
if (!t)
return -EINVAL;
+
+ ops = t->ct_group_ops;
+ if (!ops) {
+ struct config_group *g = item->ci_group;
+
+ /*
+ * No item specific group operations, check if the item's group
+ * has group operations.
+ */
+ if (g && g->cg_item.ci_type)
+ ops = g->cg_item.ci_type->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,
+ int n);
};
struct configfs_subsystem {
--
2.43.2
Tom Lendacky wrote:
> 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 <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Co-developed-by: Dan Williams <[email protected]>
> Signed-off-by: Dan Williams <[email protected]>
> Signed-off-by: Tom Lendacky <[email protected]>
> ---
> fs/configfs/dir.c | 20 ++++++++++++++++++++
> include/linux/configfs.h | 3 +++
> 2 files changed, 23 insertions(+)
>
> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
> index 18677cd4e62f..463e66258507 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,33 @@ static int populate_attrs(struct config_item *item)
>
> if (!t)
> return -EINVAL;
> +
> + ops = t->ct_group_ops;
> + if (!ops) {
> + struct config_group *g = item->ci_group;
> +
> + /*
> + * No item specific group operations, check if the item's group
> + * has group operations.
> + */
> + if (g && g->cg_item.ci_type)
> + ops = g->cg_item.ci_type->ct_group_ops;
Oh, I would not have expected to need to consider any alternate group
ops for attribute visibility beyond t->ct_group_ops. However in my RFC
example I made this mistake:
static struct configfs_group_operations tsm_report_group_ops = {
.make_item = tsm_report_make_item,
+ .is_visible = tsm_report_attr_visible,
+ .is_bin_visible = tsm_report_bin_attr_visible,
};
Which in retrospect is the wrong level, and I suspect only reachable if
you do the the above awkward indirection ("ops =
g->cg_item.ci_type->ct_group_ops"). Instead, I was expecting symmetry
with sysfs where the object that carries ->attrs also carries
->is_visible, so something like this:
+ static struct configfs_group_operations tsm_report_attr_group_ops = {
+ .is_visible = tsm_report_attr_visible,
+ .is_bin_visible = tsm_report_bin_attr_visible,
+ };
const struct config_item_type tsm_report_type = {
.ct_owner = THIS_MODULE,
.ct_bin_attrs = tsm_report_bin_attrs,
.ct_attrs = tsm_report_attrs,
.ct_item_ops = &tsm_report_item_ops,
+ .ct_group_ops = &tsm_report_attr_group_ops
};
EXPORT_SYMBOL_GPL(tsm_report_default_type);
..because is_visible() at the g->cg_item.ci_type->ct_group_ops level
would seem to mean parent directory visibility which is mismatched.
However as I stare at this a bit more it sinks in that configfs "group"
!= sysfs "group". So I am open to the suggestion that ci_item_ops is the
right place to house item attribute visibility callbacks, or even a new
"ci_attr_ops" expressly for this purpose. Either way my expectation is
that config_item_type can get to the visibilty callbacks for its
attributes without needing to traverse any other groups or items.
On 4/26/24 16:48, Dan Williams wrote:
> Tom Lendacky wrote:
>> 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 <[email protected]>
>> Cc: Christoph Hellwig <[email protected]>
>> Co-developed-by: Dan Williams <[email protected]>
>> Signed-off-by: Dan Williams <[email protected]>
>> Signed-off-by: Tom Lendacky <[email protected]>
>> ---
>> fs/configfs/dir.c | 20 ++++++++++++++++++++
>> include/linux/configfs.h | 3 +++
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/fs/configfs/dir.c b/fs/configfs/dir.c
>> index 18677cd4e62f..463e66258507 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,33 @@ static int populate_attrs(struct config_item *item)
>>
>> if (!t)
>> return -EINVAL;
>> +
>> + ops = t->ct_group_ops;
>> + if (!ops) {
>> + struct config_group *g = item->ci_group;
>> +
>> + /*
>> + * No item specific group operations, check if the item's group
>> + * has group operations.
>> + */
>> + if (g && g->cg_item.ci_type)
>> + ops = g->cg_item.ci_type->ct_group_ops;
>
> Oh, I would not have expected to need to consider any alternate group
> ops for attribute visibility beyond t->ct_group_ops. However in my RFC
> example I made this mistake:
Right, that's the question. Do you use the default group ops that are
created for TSM report or do you require the ops be part of the
attributes. It is definitely easy to do the latter.
>
> static struct configfs_group_operations tsm_report_group_ops = {
> .make_item = tsm_report_make_item,
> + .is_visible = tsm_report_attr_visible,
> + .is_bin_visible = tsm_report_bin_attr_visible,
> };
>
> Which in retrospect is the wrong level, and I suspect only reachable if
> you do the the above awkward indirection ("ops =
> g->cg_item.ci_type->ct_group_ops"). Instead, I was expecting symmetry
> with sysfs where the object that carries ->attrs also carries
> ->is_visible, so something like this:
>
> + static struct configfs_group_operations tsm_report_attr_group_ops = {
> + .is_visible = tsm_report_attr_visible,
> + .is_bin_visible = tsm_report_bin_attr_visible,
> + };
>
> const struct config_item_type tsm_report_type = {
> .ct_owner = THIS_MODULE,
> .ct_bin_attrs = tsm_report_bin_attrs,
> .ct_attrs = tsm_report_attrs,
> .ct_item_ops = &tsm_report_item_ops,
> + .ct_group_ops = &tsm_report_attr_group_ops
> };
> EXPORT_SYMBOL_GPL(tsm_report_default_type);
>
> ..because is_visible() at the g->cg_item.ci_type->ct_group_ops level
> would seem to mean parent directory visibility which is mismatched.
>
> However as I stare at this a bit more it sinks in that configfs "group"
> != sysfs "group". So I am open to the suggestion that ci_item_ops is the
> right place to house item attribute visibility callbacks, or even a new
> "ci_attr_ops" expressly for this purpose. Either way my expectation is
> that config_item_type can get to the visibilty callbacks for its
> attributes without needing to traverse any other groups or items.
I'll update the patchset to do that.
Thanks,
Tom
>