diff mbox

[4/4] btrfs: create sprout should rename fsid on the sysfs as well

Message ID 1401096626-13210-5-git-send-email-anand.jain@oracle.com (mailing list archive)
State Superseded
Headers show

Commit Message

Anand Jain May 26, 2014, 9:30 a.m. UTC
Creating sprout will change the fsid of the mounted root.
do the same on the sysfs as well.

reproducer:
 mount /dev/sdb /btrfs (seed disk)
 btrfs dev add /dev/sdc /btrfs
 mount -o rw,remount /btrfs
 btrfs dev del /dev/sdb /btrfs
 mount /dev/sdb /btrfs

Error:
kobject_add_internal failed for fe350492-dc28-4051-a601-e017b17e6145 with -EEXIST, don't try to register things with the same name in the same directory.

Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
 fs/btrfs/volumes.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

David Sterba May 29, 2014, 12:54 p.m. UTC | #1
On Mon, May 26, 2014 at 05:30:26PM +0800, Anand Jain wrote:
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2084,6 +2084,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
>  	mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
>  
>  	if (seeding_dev) {
> +		char fsid_buf[37];

Is there a symbolic constant available? We have one in userspace, but I
can't find one for kernel, only a few locally defined.

>  		ret = init_first_rw_device(trans, root, device);
>  		if (ret) {
>  			btrfs_abort_transaction(trans, root, ret);
> @@ -2094,6 +2095,13 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
>  			btrfs_abort_transaction(trans, root, ret);
>  			goto error_trans;
>  		}
> +
> +		/* Sprouting would change fsid of the mounted root,
> +		 * so rename the fsid on the sysfs
> +		 */
> +		sprintf(fsid_buf, "%pU", root->fs_info->fsid);

Would be better do use snprintf explicitly.

> +		if (kobject_rename(&root->fs_info->super_kobj, fsid_buf))
> +			goto error_trans;
>  	} else {

Otherwise ok.
--
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
Anand Jain June 2, 2014, 8:22 a.m. UTC | #2
On 29/05/14 20:54, David Sterba wrote:
> On Mon, May 26, 2014 at 05:30:26PM +0800, Anand Jain wrote:
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -2084,6 +2084,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
>>   	mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
>>
>>   	if (seeding_dev) {
>> +		char fsid_buf[37];
>
> Is there a symbolic constant available? We have one in userspace, but I
> can't find one for kernel, only a few locally defined.

  now defined the same (as in progs) in kernel as well.

>>   		ret = init_first_rw_device(trans, root, device);
>>   		if (ret) {
>>   			btrfs_abort_transaction(trans, root, ret);
>> @@ -2094,6 +2095,13 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
>>   			btrfs_abort_transaction(trans, root, ret);
>>   			goto error_trans;
>>   		}
>> +
>> +		/* Sprouting would change fsid of the mounted root,
>> +		 * so rename the fsid on the sysfs
>> +		 */
>> +		sprintf(fsid_buf, "%pU", root->fs_info->fsid);
>
> Would be better do use snprintf explicitly.

  added. Thanks for commenting.

Anand




>> +		if (kobject_rename(&root->fs_info->super_kobj, fsid_buf))
>> +			goto error_trans;
>>   	} else {
>
> Otherwise ok.
> --
> 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
David Sterba June 2, 2014, 3:39 p.m. UTC | #3
On Mon, Jun 02, 2014 at 04:22:20PM +0800, Anand Jain wrote:
> >>--- a/fs/btrfs/volumes.c
> >>+++ b/fs/btrfs/volumes.c
> >>@@ -2084,6 +2084,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
> >>  	mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
> >>
> >>  	if (seeding_dev) {
> >>+		char fsid_buf[37];
> >
> >Is there a symbolic constant available? We have one in userspace, but I
> >can't find one for kernel, only a few locally defined.
> 
>  now defined the same (as in progs) in kernel as well.

In progs it's in utils.h

 40 #define BTRFS_UUID_UNPARSED_SIZE        37

but I don't see where it's defined in kernel. Can you please give me a
pointer?

On the other hand, progs have a local define, we can do the same, I
don't see any other potential users.
--
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
Anand Jain June 3, 2014, 12:27 a.m. UTC | #4
On 02/06/2014 23:39, David Sterba wrote:
> On Mon, Jun 02, 2014 at 04:22:20PM +0800, Anand Jain wrote:
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -2084,6 +2084,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
>>>>   	mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
>>>>
>>>>   	if (seeding_dev) {
>>>> +		char fsid_buf[37];
>>>
>>> Is there a symbolic constant available? We have one in userspace, but I
>>> can't find one for kernel, only a few locally defined.
>>
>>   now defined the same (as in progs) in kernel as well.
>
> In progs it's in utils.h
>
>   40 #define BTRFS_UUID_UNPARSED_SIZE        37
>
> but I don't see where it's defined in kernel. Can you please give me a
> pointer?
>
> On the other hand, progs have a local define, we can do the same, I
> don't see any other potential users.

  David,
  What I have as of now (in my workspace) is a local define in volume.h.

  but in the 2nd thought I am thinking if it is better to be at

./include/uapi/linux/btrfs.h
::
#define BTRFS_FSID_SIZE 16
#define BTRFS_UUID_SIZE 16
#define BTRFS_UUID_UNPARSED_SIZE 37 <--

  Eventually in the long run, when we clean up btrfs-progs it could
  just include ./include/uapi/linux/btrfs.h

  Also there is this driver, who has defined it but its local

#define MAXUUIDLEN  37
./drivers/staging/tidspbridge/include/dspbridge/uuidutil.h

  How do you like the idea of define at include/uapi/linux/btrfs.h
  let me know.

Thxs, Anand

> --
> 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
diff mbox

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 5a577ab..e6cf3a7 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2084,6 +2084,7 @@  int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
 	mutex_unlock(&root->fs_info->fs_devices->device_list_mutex);
 
 	if (seeding_dev) {
+		char fsid_buf[37];
 		ret = init_first_rw_device(trans, root, device);
 		if (ret) {
 			btrfs_abort_transaction(trans, root, ret);
@@ -2094,6 +2095,13 @@  int btrfs_init_new_device(struct btrfs_root *root, char *device_path)
 			btrfs_abort_transaction(trans, root, ret);
 			goto error_trans;
 		}
+
+		/* Sprouting would change fsid of the mounted root,
+		 * so rename the fsid on the sysfs
+		 */
+		sprintf(fsid_buf, "%pU", root->fs_info->fsid);
+		if (kobject_rename(&root->fs_info->super_kobj, fsid_buf))
+			goto error_trans;
 	} else {
 		ret = btrfs_add_device(trans, root, device);
 		if (ret) {