| Message ID | 1401096626-13210-2-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, May 26, 2014 at 05:30:23PM +0800, Anand Jain wrote: > --- a/fs/btrfs/sysfs.c > +++ b/fs/btrfs/sysfs.c > @@ -572,6 +572,26 @@ static void init_feature_attrs(void) > } > } > > +int rm_device_membership(struct btrfs_fs_info *fs_info, > + struct btrfs_device *one_device) The name is too generic for a non-static function, so it would be good to add at least the btrfs_ prefix. Same applies to the change of 'add_device_membership' in the next patch. As this is only a trivial change, consider this Reviewed-by: David Sterba <dsterba@suse.cz> > +{ > + struct hd_struct *disk; > + struct kobject *disk_kobj; > + > + if (!fs_info->device_dir_kobj) > + return -EINVAL; > + > + if (one_device) { > + disk = one_device->bdev->bd_part; > + disk_kobj = &part_to_dev(disk)->kobj; > + > + sysfs_remove_link(fs_info->device_dir_kobj, > + disk_kobj->name); > + } > + > + return 0; > +} > + > static int add_device_membership(struct btrfs_fs_info *fs_info) > { > int error = 0; -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Thanks for the review David. On 29/05/14 21:04, David Sterba wrote: > On Mon, May 26, 2014 at 05:30:23PM +0800, Anand Jain wrote: >> --- a/fs/btrfs/sysfs.c >> +++ b/fs/btrfs/sysfs.c >> @@ -572,6 +572,26 @@ static void init_feature_attrs(void) >> } >> } >> >> +int rm_device_membership(struct btrfs_fs_info *fs_info, >> + struct btrfs_device *one_device) > > The name is too generic for a non-static function, so it would be good > to add at least the btrfs_ prefix. Same applies to the change of > 'add_device_membership' in the next patch. Yes indeed. I wanted it to change as well. But the diff became unfriendly to read. I shall do that in a separate patch. > As this is only a trivial change, consider this > Reviewed-by: David Sterba <dsterba@suse.cz> > >> +{ >> + struct hd_struct *disk; >> + struct kobject *disk_kobj; >> + >> + if (!fs_info->device_dir_kobj) >> + return -EINVAL; >> + >> + if (one_device) { >> + disk = one_device->bdev->bd_part; >> + disk_kobj = &part_to_dev(disk)->kobj; >> + >> + sysfs_remove_link(fs_info->device_dir_kobj, >> + disk_kobj->name); >> + } >> + >> + return 0; >> +} >> + >> static int add_device_membership(struct btrfs_fs_info *fs_info) >> { >> int error = 0; > -- > To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, May 30, 2014 at 02:10:28PM +0800, Anand Jain wrote: > >>@@ -572,6 +572,26 @@ static void init_feature_attrs(void) > >> } > >> } > >> > >>+int rm_device_membership(struct btrfs_fs_info *fs_info, > >>+ struct btrfs_device *one_device) > > > >The name is too generic for a non-static function, so it would be good > >to add at least the btrfs_ prefix. Same applies to the change of > >'add_device_membership' in the next patch. > > Yes indeed. I wanted it to change as well. But the diff became > unfriendly to read. I shall do that in a separate patch. No problem with doing the rename changes in a separate patch, but here you introduce a new function, that should get the right name from the beginning. If this would look inconsitent with the add_ counterpart, then do the rename of add_ in advance. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/btrfs/sysfs.c b/fs/btrfs/sysfs.c index 522d023..4fe3f0b 100644 --- a/fs/btrfs/sysfs.c +++ b/fs/btrfs/sysfs.c @@ -572,6 +572,26 @@ static void init_feature_attrs(void) } } +int rm_device_membership(struct btrfs_fs_info *fs_info, + struct btrfs_device *one_device) +{ + struct hd_struct *disk; + struct kobject *disk_kobj; + + if (!fs_info->device_dir_kobj) + return -EINVAL; + + if (one_device) { + disk = one_device->bdev->bd_part; + disk_kobj = &part_to_dev(disk)->kobj; + + sysfs_remove_link(fs_info->device_dir_kobj, + disk_kobj->name); + } + + return 0; +} + static int add_device_membership(struct btrfs_fs_info *fs_info) { int error = 0; diff --git a/fs/btrfs/sysfs.h b/fs/btrfs/sysfs.h index 9ab5763..eaed810 100644 --- a/fs/btrfs/sysfs.h +++ b/fs/btrfs/sysfs.h @@ -66,4 +66,6 @@ char *btrfs_printable_features(enum btrfs_feature_set set, u64 flags); extern const char * const btrfs_feature_set_names[3]; extern struct kobj_type space_info_ktype; extern struct kobj_type btrfs_raid_ktype; +int rm_device_membership(struct btrfs_fs_info *fs_info, + struct btrfs_device *one_device); #endif /* _BTRFS_SYSFS_H_ */ diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 15a8934..3ceb28c 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -40,6 +40,7 @@ #include "rcu-string.h" #include "math.h" #include "dev-replace.h" +#include "sysfs.h" static int init_first_rw_device(struct btrfs_trans_handle *trans, struct btrfs_root *root, @@ -1651,6 +1652,9 @@ int btrfs_rm_device(struct btrfs_root *root, char *device_path) if (device->bdev) device->fs_devices->open_devices--; + /* remove sysfs entry */ + rm_device_membership(root->fs_info, device); + call_rcu(&device->rcu, free_device); num_devices = btrfs_super_num_devices(root->fs_info->super_copy) - 1;