Hi Ben,
On 1/9/25 9:33 AM, Benjamin Coddington wrote:
> On 8 Jan 2025, at 16:36, Anna Schumaker wrote:
>
>> From: Anna Schumaker <anna.schumaker@xxxxxxxxxx>
>>
>> The Linux NFS server added support for returning this information during
>> an EXCHANGE_ID in Linux v6.13. This is something and admin might want to
>> query, so let's add it to sysfs.
>
> I agree that admins will want to know (and they might not yet know they want
> to know), good idea!
>
> A couple comments..
>
>> Signed-off-by: Anna Schumaker <anna.schumaker@xxxxxxxxxx>
>> ---
>> fs/nfs/sysfs.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 79 insertions(+)
>>
>> diff --git a/fs/nfs/sysfs.c b/fs/nfs/sysfs.c
>> index 7b59a40d40c0..6b82c92c45bf 100644
>> --- a/fs/nfs/sysfs.c
>> +++ b/fs/nfs/sysfs.c
>> @@ -272,6 +272,32 @@ shutdown_store(struct kobject *kobj, struct kobj_attribute *attr,
>>
>> static struct kobj_attribute nfs_sysfs_attr_shutdown = __ATTR_RW(shutdown);
>>
>> +#if IS_ENABLED(CONFIG_NFS_V4_1)
>> +static ssize_t
>> +implid_domain_show(struct kobject *kobj, struct kobj_attribute *attr,
>> + char *buf)
>> +{
>> + struct nfs_server *server = container_of(kobj, struct nfs_server, kobj);
>> + struct nfs41_impl_id *impl_id = server->nfs_client->cl_implid;
>> + return sysfs_emit(buf, "%s\n", impl_id->domain);
>> +}
>> +
>> +static struct kobj_attribute nfs_sysfs_attr_implid_domain = __ATTR_RO(implid_domain);
>> +
>> +
>> +static ssize_t
>> +implid_name_show(struct kobject *kobj, struct kobj_attribute *attr,
>> + char *buf)
>> +{
>> + struct nfs_server *server = container_of(kobj, struct nfs_server, kobj);
>> + struct nfs41_impl_id *impl_id = server->nfs_client->cl_implid;
>> + return sysfs_emit(buf, "%s\n", impl_id->name);
>> +}
>> +
>> +static struct kobj_attribute nfs_sysfs_attr_implid_name = __ATTR_RO(implid_name);
>> +
>> +#endif /* IS_ENABLED(CONFIG_NFS_V4_1) */
>> +
>> #define RPC_CLIENT_NAME_SIZE 64
>>
>> void nfs_sysfs_link_rpc_client(struct nfs_server *server,
>> @@ -309,6 +335,33 @@ static struct kobj_type nfs_sb_ktype = {
>> .child_ns_type = nfs_netns_object_child_ns_type,
>> };
>>
>> +#if IS_ENABLED(CONFIG_NFS_V4_1)
>> +static void nfs_sysfs_add_nfsv41_server(struct nfs_server *server)
>> +{
>> + struct nfs_client *clp = server->nfs_client;
>> + int ret;
>> +
>> + if (clp->cl_implid && strlen(clp->cl_implid->domain) > 0) {
>
> Can we create the files and leave them empty if the strings are not set?
> Having an empty file might be a nice prompt for an implementation to start
> sending values. I also have a slight preference for a less dynamic sysfs.
Sure! I've made that change in my branch.
>
>> + ret = sysfs_create_file_ns(&server->kobj, &nfs_sysfs_attr_implid_domain.attr,
>> + nfs_netns_server_namespace(&server->kobj));
>> + if (ret < 0)
>> + pr_warn("NFS: sysfs_create_file_ns for server-%d failed (%d)\n",
>> + server->s_sysfs_id, ret);
>> +
>> + ret = sysfs_create_file_ns(&server->kobj, &nfs_sysfs_attr_implid_name.attr,
>> + nfs_netns_server_namespace(&server->kobj));
>> + if (ret < 0)
>> + pr_warn("NFS: sysfs_create_file_ns for server-%d failed (%d)\n",
>> + server->s_sysfs_id, ret);
>> +
>> + }
>> +}
>> +#else /* CONFIG_NFS_V4_1 */
>> +static inline void nfs_sysfs_add_nfsv41_server(struct nfs_server *server)
>> +{
>> +}
>> +#endif /* CONFIG_NFS_V4_1 */
>> +
>> void nfs_sysfs_add_server(struct nfs_server *server)
>> {
>> int ret;
>> @@ -325,6 +378,32 @@ void nfs_sysfs_add_server(struct nfs_server *server)
>> if (ret < 0)
>> pr_warn("NFS: sysfs_create_file_ns for server-%d failed (%d)\n",
>> server->s_sysfs_id, ret);
>> +
>> + nfs_sysfs_add_nfsv41_server(server);
>
> I think you didn't mean to send the hunk below. :)
Hah! No, I did not. I've removed it for v2.
Thanks for looking at this!
Anna
>
> Ben
>
>> +
>> +/* if (server->nfs_client->cl_serverowner) {
>> + ret = sysfs_create_file_ns(&server->kobj, &nfs_sysfs_attr_serverowner.attr,
>> + nfs_netns_server_namespace(&server->kobj));
>> + if (ret < 0)
>> + pr_warn("NFS: sysfs_create_file_ns for server-%d failed (%d)\n",
>> + server->s_sysfs_id, ret);
>> + }
>> +
>> + if (server->nfs_client->cl_serverscope) {
>> + ret = sysfs_create_file_ns(&server->kobj, &nfs_sysfs_attr_serverscope.attr,
>> + nfs_netns_server_namespace(&server->kobj));
>> + if (ret < 0)
>> + pr_warn("NFS: sysfs_create_file_ns for server-%d failed (%d)\n",
>> + server->s_sysfs_id, ret);
>> + }
>> +
>> + if (server->nfs_client->cl_implid) {
>> + ret = sysfs_create_file_ns(&server->kobj, &nfs_sysfs_attr_implid.attr,
>> + nfs_netns_server_namespace(&server->kobj));
>> + if (ret < 0)
>> + pr_warn("NFS: sysfs_create_file_ns for server-%d failed (%d)\n",
>> + server->s_sysfs_id, ret);
>> + }*/
>> }
>> EXPORT_SYMBOL_GPL(nfs_sysfs_add_server);
>>
>> --
>> 2.47.1
>
