[PATCH] Btrfs-progs: convert: do not clear header rev

[PATCH] Btrfs-progs: convert: do not clear header rev

[Liu Bo]

So btrfs_set_header_flags() vs btrfs_set_header_flag, the difference is sort of
similar to "=" vs "|=", when creating and initialising a new extent buffer,
convert uses the former one which clears header_rev by accident.

Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
---
 convert/common.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/convert/common.c b/convert/common.c
index 40bf32c..f0dd2cf 100644
--- a/convert/common.c
+++ b/convert/common.c
@@ -167,7 +167,7 @@ static int setup_temp_extent_buffer(struct extent_buffer *buf,
 	btrfs_set_header_generation(buf, 1);
 	btrfs_set_header_backref_rev(buf, BTRFS_MIXED_BACKREF_REV);
 	btrfs_set_header_owner(buf, owner);
-	btrfs_set_header_flags(buf, BTRFS_HEADER_FLAG_WRITTEN);
+	btrfs_set_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN);
 	write_extent_buffer(buf, chunk_uuid, btrfs_header_chunk_tree_uuid(buf),
 			    BTRFS_UUID_SIZE);
 	write_extent_buffer(buf, fsid, btrfs_header_fsid(), BTRFS_FSID_SIZE);
-- 
2.5.0

Re: [PATCH] Btrfs-progs: convert: do not clear header rev

[Qu Wenruo]

At 06/27/2017 07:55 AM, Liu Bo wrote:
> So btrfs_set_header_flags() vs btrfs_set_header_flag, the difference is sort of
> similar to "=" vs "|=", when creating and initialising a new extent buffer,
> convert uses the former one which clears header_rev by accident.

Thanks for catching this one.

Reviewed-by: Qu Wenruo <quwenruo@cn.fujitsu.com>

Thanks,
Qu
> 
> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
>   convert/common.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/convert/common.c b/convert/common.c
> index 40bf32c..f0dd2cf 100644
> --- a/convert/common.c
> +++ b/convert/common.c
> @@ -167,7 +167,7 @@ static int setup_temp_extent_buffer(struct extent_buffer *buf,
>   	btrfs_set_header_generation(buf, 1);
>   	btrfs_set_header_backref_rev(buf, BTRFS_MIXED_BACKREF_REV);
>   	btrfs_set_header_owner(buf, owner);
> -	btrfs_set_header_flags(buf, BTRFS_HEADER_FLAG_WRITTEN);
> +	btrfs_set_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN);
>   	write_extent_buffer(buf, chunk_uuid, btrfs_header_chunk_tree_uuid(buf),
>   			    BTRFS_UUID_SIZE);
>   	write_extent_buffer(buf, fsid, btrfs_header_fsid(), BTRFS_FSID_SIZE);
> 

Re: [PATCH] Btrfs-progs: convert: do not clear header rev

[Liu Bo]

On Mon, Jun 26, 2017 at 05:55:02PM -0600, Liu Bo wrote:
> So btrfs_set_header_flags() vs btrfs_set_header_flag, the difference is sort of
> similar to "=" vs "|=", when creating and initialising a new extent buffer,
> convert uses the former one which clears header_rev by accident.
>

Here is the regression test script,

mkfs.ext4 -F /disk
btrfs-convert -d /disk
mount /disk /mnt
xfs_io -f -c "pwrite 0 4K" /mnt/foobar
umount /disk

---------------
It'd end up with 'unable to find ref blabla" shown in dmesg log.

thanks,
-liubo

> Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> ---
>  convert/common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/convert/common.c b/convert/common.c
> index 40bf32c..f0dd2cf 100644
> --- a/convert/common.c
> +++ b/convert/common.c
> @@ -167,7 +167,7 @@ static int setup_temp_extent_buffer(struct extent_buffer *buf,
>  	btrfs_set_header_generation(buf, 1);
>  	btrfs_set_header_backref_rev(buf, BTRFS_MIXED_BACKREF_REV);
>  	btrfs_set_header_owner(buf, owner);
> -	btrfs_set_header_flags(buf, BTRFS_HEADER_FLAG_WRITTEN);
> +	btrfs_set_header_flag(buf, BTRFS_HEADER_FLAG_WRITTEN);
>  	write_extent_buffer(buf, chunk_uuid, btrfs_header_chunk_tree_uuid(buf),
>  			    BTRFS_UUID_SIZE);
>  	write_extent_buffer(buf, fsid, btrfs_header_fsid(), BTRFS_FSID_SIZE);
> -- 
> 2.5.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

Re: [PATCH] Btrfs-progs: convert: do not clear header rev

[David Sterba]

On Tue, Jun 27, 2017 at 12:07:13PM -0700, Liu Bo wrote:
> On Mon, Jun 26, 2017 at 05:55:02PM -0600, Liu Bo wrote:
> > So btrfs_set_header_flags() vs btrfs_set_header_flag, the difference is sort of
> > similar to "=" vs "|=", when creating and initialising a new extent buffer,
> > convert uses the former one which clears header_rev by accident.
> >
> 
> Here is the regression test script,
> 
> mkfs.ext4 -F /disk
> btrfs-convert -d /disk
> mount /disk /mnt
> xfs_io -f -c "pwrite 0 4K" /mnt/foobar
> umount /disk
> 
> ---------------
> It'd end up with 'unable to find ref blabla" shown in dmesg log.

I can't reproduce it, is there something else needed? Tested kernel is
4.11.6. Check does not complain either.

Re: [PATCH] Btrfs-progs: convert: do not clear header rev

[Lu Fengqi]

On Fri, Jun 30, 2017 at 04:52:54PM +0200, David Sterba wrote:
>On Tue, Jun 27, 2017 at 12:07:13PM -0700, Liu Bo wrote:
>> On Mon, Jun 26, 2017 at 05:55:02PM -0600, Liu Bo wrote:
>> > So btrfs_set_header_flags() vs btrfs_set_header_flag, the difference is sort of
>> > similar to "=" vs "|=", when creating and initialising a new extent buffer,
>> > convert uses the former one which clears header_rev by accident.
>> >
>> 
>> Here is the regression test script,
>> 
>> mkfs.ext4 -F /disk
>> btrfs-convert -d /disk
>> mount /disk /mnt
>> xfs_io -f -c "pwrite 0 4K" /mnt/foobar
>> umount /disk
>> 
>> ---------------
>> It'd end up with 'unable to find ref blabla" shown in dmesg log.
>
>I can't reproduce it, is there something else needed? Tested kernel is
>4.11.6. Check does not complain either.

Excuse me, is your btrfs-progs already applied this patch?

I'm sure that I can reproduce it in my test environment.
1. kernel 4.11.6
2. btrfs-progs kdave/devel(drop this patch)

-- 
Thanks,
Lu