[PATCH 0/2] btrfstune fs_info cleanups

[PATCH 0/2] btrfstune fs_info cleanups

[Nikolay Borisov]

While inspecting btrfstune source code I came across 2 functions which needlessly
take fs_info argument. This small series rectifies this. No functional changes. 

Nikolay Borisov (2):
  btrfstune: Remove fs_info arg from change_device_uuid
  btrfstune: Rename change_header_uuid to change_buffer_header_uuid

 btrfstune.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

-- 
2.7.4

[PATCH 1/2] btrfstune: Remove fs_info arg from change_device_uuid

[Nikolay Borisov]

This function already takes an extent buffer that contains a reference
to the fs_info. Use that and reduce argument count. No functional
changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 btrfstune.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/btrfstune.c b/btrfstune.c
index 889b931c55d8..723317a00f3a 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -179,10 +179,10 @@ static int change_extents_uuid(struct btrfs_fs_info *fs_info)
 	return ret;
 }
 
-static int change_device_uuid(struct btrfs_fs_info *fs_info, struct extent_buffer *eb,
-			      int slot)
+static int change_device_uuid(struct extent_buffer *eb, int slot)
 {
 	struct btrfs_dev_item *di;
+	struct btrfs_fs_info *fs_info = eb->fs_info;
 	int ret = 0;
 
 	di = btrfs_item_ptr(eb, slot, struct btrfs_dev_item);
@@ -217,7 +217,7 @@ static int change_devices_uuid(struct btrfs_fs_info *fs_info)
 		if (key.type != BTRFS_DEV_ITEM_KEY ||
 		    key.objectid != BTRFS_DEV_ITEMS_OBJECTID)
 			goto next;
-		ret = change_device_uuid(fs_info, path.nodes[0], path.slots[0]);
+		ret = change_device_uuid(path.nodes[0], path.slots[0]);
 		if (ret < 0)
 			goto out;
 next:
-- 
2.7.4

Re: [PATCH 1/2] btrfstune: Remove fs_info arg from change_device_uuid

[Qu Wenruo]

On 2018/8/20 下午9:28, Nikolay Borisov wrote:
> This function already takes an extent buffer that contains a reference
> to the fs_info. Use that and reduce argument count. No functional
> changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  btrfstune.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/btrfstune.c b/btrfstune.c
> index 889b931c55d8..723317a00f3a 100644
> --- a/btrfstune.c
> +++ b/btrfstune.c
> @@ -179,10 +179,10 @@ static int change_extents_uuid(struct btrfs_fs_info *fs_info)
>  	return ret;
>  }
>  
> -static int change_device_uuid(struct btrfs_fs_info *fs_info, struct extent_buffer *eb,
> -			      int slot)
> +static int change_device_uuid(struct extent_buffer *eb, int slot)
>  {
>  	struct btrfs_dev_item *di;
> +	struct btrfs_fs_info *fs_info = eb->fs_info;
>  	int ret = 0;
>  
>  	di = btrfs_item_ptr(eb, slot, struct btrfs_dev_item);
> @@ -217,7 +217,7 @@ static int change_devices_uuid(struct btrfs_fs_info *fs_info)
>  		if (key.type != BTRFS_DEV_ITEM_KEY ||
>  		    key.objectid != BTRFS_DEV_ITEMS_OBJECTID)
>  			goto next;
> -		ret = change_device_uuid(fs_info, path.nodes[0], path.slots[0]);
> +		ret = change_device_uuid(path.nodes[0], path.slots[0]);
>  		if (ret < 0)
>  			goto out;
>  next:
> 

[PATCH 2/2] btrfstune: Rename change_header_uuid to change_buffer_header_uuid

[Nikolay Borisov]

Rename the function so its name falls in line with the rest of btrfs
nomenclature. So when we change the uuid of the header it's in fact
of the buffer header. Also remove the root argument since it's used
solely to take a reference to fs_info which can be done via the
mandatory eb argument. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 btrfstune.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/btrfstune.c b/btrfstune.c
index 723317a00f3a..3745a00c46ae 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -91,9 +91,9 @@ static int set_super_incompat_flags(struct btrfs_root *root, u64 flags)
 	return ret;
 }
 
-static int change_header_uuid(struct btrfs_root *root, struct extent_buffer *eb)
+static int change_buffer_header_uuid(struct extent_buffer *eb)
 {
-	struct btrfs_fs_info *fs_info = root->fs_info;
+	struct btrfs_fs_info *fs_info = eb->fs_info;
 	int same_fsid = 1;
 	int same_chunk_tree_uuid = 1;
 	int ret;
@@ -157,7 +157,7 @@ static int change_extents_uuid(struct btrfs_fs_info *fs_info)
 			ret = PTR_ERR(eb);
 			goto out;
 		}
-		ret = change_header_uuid(root, eb);
+		ret = change_buffer_header_uuid(eb);
 		free_extent_buffer(eb);
 		if (ret < 0) {
 			error("failed to change uuid of tree block: %llu",
-- 
2.7.4

Re: [PATCH 2/2] btrfstune: Rename change_header_uuid to change_buffer_header_uuid

[Qu Wenruo]

On 2018/8/20 下午9:28, Nikolay Borisov wrote:
> Rename the function so its name falls in line with the rest of btrfs
> nomenclature. So when we change the uuid of the header it's in fact
> of the buffer header.

We have setget functions like btrfs_header_nritems() or
btrfs_header_bytenr(), IMHO both ways make sense.

Although I have no objection against current modification.

> Also remove the root argument since it's used
> solely to take a reference to fs_info which can be done via the
> mandatory eb argument. No functional changes.>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  btrfstune.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/btrfstune.c b/btrfstune.c
> index 723317a00f3a..3745a00c46ae 100644
> --- a/btrfstune.c
> +++ b/btrfstune.c
> @@ -91,9 +91,9 @@ static int set_super_incompat_flags(struct btrfs_root *root, u64 flags)
>  	return ret;
>  }
>  
> -static int change_header_uuid(struct btrfs_root *root, struct extent_buffer *eb)
> +static int change_buffer_header_uuid(struct extent_buffer *eb)
>  {
> -	struct btrfs_fs_info *fs_info = root->fs_info;
> +	struct btrfs_fs_info *fs_info = eb->fs_info;
>  	int same_fsid = 1;
>  	int same_chunk_tree_uuid = 1;
>  	int ret;
> @@ -157,7 +157,7 @@ static int change_extents_uuid(struct btrfs_fs_info *fs_info)
>  			ret = PTR_ERR(eb);
>  			goto out;
>  		}
> -		ret = change_header_uuid(root, eb);
> +		ret = change_buffer_header_uuid(eb);
>  		free_extent_buffer(eb);
>  		if (ret < 0) {
>  			error("failed to change uuid of tree block: %llu",
> 

Re: [PATCH 2/2] btrfstune: Rename change_header_uuid to change_buffer_header_uuid

[Nikolay Borisov]

On 20.08.2018 16:34, Qu Wenruo wrote:
> 
> 
> On 2018/8/20 下午9:28, Nikolay Borisov wrote:
>> Rename the function so its name falls in line with the rest of btrfs
>> nomenclature. So when we change the uuid of the header it's in fact
>> of the buffer header.
> 
> We have setget functions like btrfs_header_nritems() or
> btrfs_header_bytenr(), IMHO both ways make sense.

Yes, however, we don't really have such functions for the two fields.
Alternatively they could be introduced and will consist of only :
write_extent_buffer() calls and the cheks for
same_fsid/same_chunk_tree_uuid will still be retained in
change_buffer_header_uuid.

> 
> Although I have no objection against current modification.
> 
>> Also remove the root argument since it's used
>> solely to take a reference to fs_info which can be done via the
>> mandatory eb argument. No functional changes.>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 
> Thanks,
> Qu
> 
>> ---
>>  btrfstune.c | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/btrfstune.c b/btrfstune.c
>> index 723317a00f3a..3745a00c46ae 100644
>> --- a/btrfstune.c
>> +++ b/btrfstune.c
>> @@ -91,9 +91,9 @@ static int set_super_incompat_flags(struct btrfs_root *root, u64 flags)
>>  	return ret;
>>  }
>>  
>> -static int change_header_uuid(struct btrfs_root *root, struct extent_buffer *eb)
>> +static int change_buffer_header_uuid(struct extent_buffer *eb)
>>  {
>> -	struct btrfs_fs_info *fs_info = root->fs_info;
>> +	struct btrfs_fs_info *fs_info = eb->fs_info;
>>  	int same_fsid = 1;
>>  	int same_chunk_tree_uuid = 1;
>>  	int ret;
>> @@ -157,7 +157,7 @@ static int change_extents_uuid(struct btrfs_fs_info *fs_info)
>>  			ret = PTR_ERR(eb);
>>  			goto out;
>>  		}
>> -		ret = change_header_uuid(root, eb);
>> +		ret = change_buffer_header_uuid(eb);
>>  		free_extent_buffer(eb);
>>  		if (ret < 0) {
>>  			error("failed to change uuid of tree block: %llu",
>>
> 

Re: [PATCH 0/2] btrfstune fs_info cleanups

[David Sterba]

On Mon, Aug 20, 2018 at 04:28:03PM +0300, Nikolay Borisov wrote:
> While inspecting btrfstune source code I came across 2 functions which needlessly
> take fs_info argument. This small series rectifies this. No functional changes. 
> 
> Nikolay Borisov (2):
>   btrfstune: Remove fs_info arg from change_device_uuid
>   btrfstune: Rename change_header_uuid to change_buffer_header_uuid

Applied, thanks.