2018-12-26 05:41:26

by Kangjie Lu

[permalink] [raw]
Subject: [PATCH v2] btrfs: add a check for sysfs_create_group

In case sysfs_create_group fails, let's check its return value and
issues an error message.

Signed-off-by: Kangjie Lu <[email protected]>
---
fs/btrfs/sysfs.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
index 3717c864ba23..24ef416e700b 100644
--- a/fs/btrfs/sysfs.c
+++ b/fs/btrfs/sysfs.c
@@ -889,6 +889,8 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
*/
sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
+ if (ret)
+ btrfs_err(fs_info, "failed to create btrfs_feature_attr_group.\n");
}

static int btrfs_init_debugfs(void)
--
2.17.2 (Apple Git-113)



2018-12-26 05:44:42

by Su Yue

[permalink] [raw]
Subject: Re: [PATCH v2] btrfs: add a check for sysfs_create_group



On 12/26/18 1:37 PM, Kangjie Lu wrote:
> In case sysfs_create_group fails, let's check its return value and
> issues an error message.
>
> Signed-off-by: Kangjie Lu <[email protected]>
> ---
> fs/btrfs/sysfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 3717c864ba23..24ef416e700b 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -889,6 +889,8 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> */
> sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
> + if (ret)
> + btrfs_err(fs_info, "failed to create btrfs_feature_attr_group.\n");

NIT: ".\n" is unnecessary.

---
Su
> }
>
> static int btrfs_init_debugfs(void)
>

2018-12-26 05:49:59

by Qu Wenruo

[permalink] [raw]
Subject: Re: [PATCH v2] btrfs: add a check for sysfs_create_group



On 2018/12/26 下午1:37, Kangjie Lu wrote:
> In case sysfs_create_group fails, let's check its return value and
> issues an error message.
>
> Signed-off-by: Kangjie Lu <[email protected]>
> ---
> fs/btrfs/sysfs.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c
> index 3717c864ba23..24ef416e700b 100644
> --- a/fs/btrfs/sysfs.c
> +++ b/fs/btrfs/sysfs.c
> @@ -889,6 +889,8 @@ void btrfs_sysfs_feature_update(struct btrfs_fs_info *fs_info,
> */
> sysfs_remove_group(fsid_kobj, &btrfs_feature_attr_group);
> ret = sysfs_create_group(fsid_kobj, &btrfs_feature_attr_group);
> + if (ret)
> + btrfs_err(fs_info, "failed to create btrfs_feature_attr_group.\n");

Forgot to mention, for btrfs_* infrastructure, no need for the ending '\n'.

Despite that, looks good.

Reviewed-by: Qu Wenruo <[email protected]>

Thanks,
Qu

> }
>
> static int btrfs_init_debugfs(void)
>

Attachments:
signature.asc (499.00 B)
OpenPGP digital signature

2019-01-08 15:53:37

by David Sterba

[permalink] [raw]
Subject: Re: [PATCH v2] btrfs: add a check for sysfs_create_group

On Tue, Dec 25, 2018 at 11:37:44PM -0600, Kangjie Lu wrote:
> In case sysfs_create_group fails, let's check its return value and
> issues an error message.

It's in a function that's not currently used and the sysfs code needs to
be reworked. The return code should be passed to the caller, printing
error message in this case may not be desired but I can't say right now.