[PATCH] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[PATCH] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[Nikolay Borisov]

Support for asynchronous snapshot creation was originally added in
72fd032e9424 ("Btrfs: add SNAP_CREATE_ASYNC ioctl") to cater for
ceph's backend needs. However, since Ceph has deprecated support for
btrfs there is no longer need for that support in btrfs. Additionally,
this was never supported by btrfs-progs, the official userspace tools.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 fs/btrfs/ioctl.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c343f72a84d5..a412bd2036bb 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1837,8 +1837,12 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
 		goto free_args;
 	}
 
-	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC)
+	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC) {
+		pr_warn("BTRFS: async snapshot creation is deprecated and will"
+			" be removed in kernel 5.7\n");
+
 		ptr = &transid;
+	}
 	if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
 		readonly = true;
 	if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
@@ -4214,6 +4218,10 @@ static noinline long btrfs_ioctl_start_sync(struct btrfs_root *root,
 	u64 transid;
 	int ret;
 
+
+	pr_warn("BTRFS: START_SYNC ioctl is deprecated and will be removed in "
+		"kernel 5.7\n");
+
 	trans = btrfs_attach_transaction_barrier(root);
 	if (IS_ERR(trans)) {
 		if (PTR_ERR(trans) != -ENOENT)
@@ -4241,6 +4249,9 @@ static noinline long btrfs_ioctl_wait_sync(struct btrfs_fs_info *fs_info,
 {
 	u64 transid;
 
+	pr_warn("BTRFS: WAIT_SYNC ioctl is deprecated and will be removed in "
+		"kernel 5.7\n");
+
 	if (argp) {
 		if (copy_from_user(&transid, argp, sizeof(transid)))
 			return -EFAULT;
-- 
2.17.1

Re: [PATCH] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[Qu Wenruo]

On 2019/8/26 下午8:37, Nikolay Borisov wrote:
> Support for asynchronous snapshot creation was originally added in
> 72fd032e9424 ("Btrfs: add SNAP_CREATE_ASYNC ioctl") to cater for
> ceph's backend needs. However, since Ceph has deprecated support for
> btrfs

Any reference for that?

From what I have read, btrfs is still supported, although only
recommended for testing/development.
From ceph filesystem recommendations:

We used to recommend btrfs for testing, development, and any
non-critical deployments becuase it has the most promising set of
features. However, we now plan to avoid using a kernel file system
entirely with the new BlueStore backend. btrfs is still supported and
has a comparatively compelling set of features, but be mindful of its
stability and support status in your Linux distribution.

(Anyway, still better than the not recommended ext4).

Thanks,
Qu

> there is no longer need for that support in btrfs. Additionally,
> this was never supported by btrfs-progs, the official userspace tools.
>
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  fs/btrfs/ioctl.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index c343f72a84d5..a412bd2036bb 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -1837,8 +1837,12 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
>  		goto free_args;
>  	}
>
> -	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC)
> +	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC) {
> +		pr_warn("BTRFS: async snapshot creation is deprecated and will"
> +			" be removed in kernel 5.7\n");
> +
>  		ptr = &transid;
> +	}
>  	if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
>  		readonly = true;
>  	if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
> @@ -4214,6 +4218,10 @@ static noinline long btrfs_ioctl_start_sync(struct btrfs_root *root,
>  	u64 transid;
>  	int ret;
>
> +
> +	pr_warn("BTRFS: START_SYNC ioctl is deprecated and will be removed in "
> +		"kernel 5.7\n");
> +
>  	trans = btrfs_attach_transaction_barrier(root);
>  	if (IS_ERR(trans)) {
>  		if (PTR_ERR(trans) != -ENOENT)
> @@ -4241,6 +4249,9 @@ static noinline long btrfs_ioctl_wait_sync(struct btrfs_fs_info *fs_info,
>  {
>  	u64 transid;
>
> +	pr_warn("BTRFS: WAIT_SYNC ioctl is deprecated and will be removed in "
> +		"kernel 5.7\n");
> +
>  	if (argp) {
>  		if (copy_from_user(&transid, argp, sizeof(transid)))
>  			return -EFAULT;
>

Re: [PATCH] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[Nikolay Borisov]

On 26.08.19 г. 16:03 ч., Qu Wenruo wrote:
> 
> 
> On 2019/8/26 下午8:37, Nikolay Borisov wrote:
>> Support for asynchronous snapshot creation was originally added in
>> 72fd032e9424 ("Btrfs: add SNAP_CREATE_ASYNC ioctl") to cater for
>> ceph's backend needs. However, since Ceph has deprecated support for
>> btrfs
> 
> Any reference for that?
> 
> From what I have read, btrfs is still supported, although only
> recommended for testing/development.
> From ceph filesystem recommendations:

The principal author of btrfs (and funnily enough, original implementer
of the async subvolume) says so himself:

http://lists.ceph.com/pipermail/ceph-users-ceph.com/2017-July/019095.html:

On 06/30/2017 06:48 PM, Sage Weil wrote:

> Ah, crap.  This is what happens when devs don't read their own
> documetnation.  I recommend against btrfs every time it ever comes
> up, the downstream distributions all support only xfs, but yes, it
> looks like the docs never got updated... despite the xfs focus being
> 5ish years old now.


> 
> We used to recommend btrfs for testing, development, and any
> non-critical deployments becuase it has the most promising set of
> features. However, we now plan to avoid using a kernel file system
> entirely with the new BlueStore backend. btrfs is still supported and
> has a comparatively compelling set of features, but be mindful of its
> stability and support status in your Linux distribution.
> 
> (Anyway, still better than the not recommended ext4).
> 
> Thanks,
> Qu
> 
>> there is no longer need for that support in btrfs. Additionally,
>> this was never supported by btrfs-progs, the official userspace tools.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  fs/btrfs/ioctl.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>> index c343f72a84d5..a412bd2036bb 100644
>> --- a/fs/btrfs/ioctl.c
>> +++ b/fs/btrfs/ioctl.c
>> @@ -1837,8 +1837,12 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
>>  		goto free_args;
>>  	}
>>
>> -	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC)
>> +	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC) {
>> +		pr_warn("BTRFS: async snapshot creation is deprecated and will"
>> +			" be removed in kernel 5.7\n");
>> +
>>  		ptr = &transid;
>> +	}
>>  	if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
>>  		readonly = true;
>>  	if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
>> @@ -4214,6 +4218,10 @@ static noinline long btrfs_ioctl_start_sync(struct btrfs_root *root,
>>  	u64 transid;
>>  	int ret;
>>
>> +
>> +	pr_warn("BTRFS: START_SYNC ioctl is deprecated and will be removed in "
>> +		"kernel 5.7\n");
>> +
>>  	trans = btrfs_attach_transaction_barrier(root);
>>  	if (IS_ERR(trans)) {
>>  		if (PTR_ERR(trans) != -ENOENT)
>> @@ -4241,6 +4249,9 @@ static noinline long btrfs_ioctl_wait_sync(struct btrfs_fs_info *fs_info,
>>  {
>>  	u64 transid;
>>
>> +	pr_warn("BTRFS: WAIT_SYNC ioctl is deprecated and will be removed in "
>> +		"kernel 5.7\n");
>> +
>>  	if (argp) {
>>  		if (copy_from_user(&transid, argp, sizeof(transid)))
>>  			return -EFAULT;
>>
> 

Re: [PATCH] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[Johannes Thumshirn]

Reviewed-by: Johannes Thumshirn <jthumshirn@suse.de>
-- 
Johannes Thumshirn                            SUSE Labs Filesystems
jthumshirn@suse.de                                +49 911 74053 689
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5
90409 Nürnberg
Germany
(HRB 247165, AG München)
Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850

Re: [PATCH] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[Qu Wenruo]

On 2019/8/26 下午9:08, Nikolay Borisov wrote:
>
>
> On 26.08.19 г. 16:03 ч., Qu Wenruo wrote:
>>
>>
>> On 2019/8/26 下午8:37, Nikolay Borisov wrote:
>>> Support for asynchronous snapshot creation was originally added in
>>> 72fd032e9424 ("Btrfs: add SNAP_CREATE_ASYNC ioctl") to cater for
>>> ceph's backend needs. However, since Ceph has deprecated support for
>>> btrfs
>>
>> Any reference for that?
>>
>> From what I have read, btrfs is still supported, although only
>> recommended for testing/development.
>> From ceph filesystem recommendations:
>
> The principal author of btrfs (and funnily enough, original implementer
> of the async subvolume) says so himself:
>
> http://lists.ceph.com/pipermail/ceph-users-ceph.com/2017-July/019095.html:

OK, still that transid mismatch ghost...

Anyway, the patch itself should be OK, so:

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

Thanks,
Qu
>
> On 06/30/2017 06:48 PM, Sage Weil wrote:
>
>> Ah, crap.  This is what happens when devs don't read their own
>> documetnation.  I recommend against btrfs every time it ever comes
>> up, the downstream distributions all support only xfs, but yes, it
>> looks like the docs never got updated... despite the xfs focus being
>> 5ish years old now.
>
>
>>
>> We used to recommend btrfs for testing, development, and any
>> non-critical deployments becuase it has the most promising set of
>> features. However, we now plan to avoid using a kernel file system
>> entirely with the new BlueStore backend. btrfs is still supported and
>> has a comparatively compelling set of features, but be mindful of its
>> stability and support status in your Linux distribution.
>>
>> (Anyway, still better than the not recommended ext4).
>>
>> Thanks,
>> Qu
>>
>>> there is no longer need for that support in btrfs. Additionally,
>>> this was never supported by btrfs-progs, the official userspace tools.
>>>
>>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>>> ---
>>>  fs/btrfs/ioctl.c | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
>>> index c343f72a84d5..a412bd2036bb 100644
>>> --- a/fs/btrfs/ioctl.c
>>> +++ b/fs/btrfs/ioctl.c
>>> @@ -1837,8 +1837,12 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
>>>  		goto free_args;
>>>  	}
>>>
>>> -	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC)
>>> +	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC) {
>>> +		pr_warn("BTRFS: async snapshot creation is deprecated and will"
>>> +			" be removed in kernel 5.7\n");
>>> +
>>>  		ptr = &transid;
>>> +	}
>>>  	if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
>>>  		readonly = true;
>>>  	if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
>>> @@ -4214,6 +4218,10 @@ static noinline long btrfs_ioctl_start_sync(struct btrfs_root *root,
>>>  	u64 transid;
>>>  	int ret;
>>>
>>> +
>>> +	pr_warn("BTRFS: START_SYNC ioctl is deprecated and will be removed in "
>>> +		"kernel 5.7\n");
>>> +
>>>  	trans = btrfs_attach_transaction_barrier(root);
>>>  	if (IS_ERR(trans)) {
>>>  		if (PTR_ERR(trans) != -ENOENT)
>>> @@ -4241,6 +4249,9 @@ static noinline long btrfs_ioctl_wait_sync(struct btrfs_fs_info *fs_info,
>>>  {
>>>  	u64 transid;
>>>
>>> +	pr_warn("BTRFS: WAIT_SYNC ioctl is deprecated and will be removed in "
>>> +		"kernel 5.7\n");
>>> +
>>>  	if (argp) {
>>>  		if (copy_from_user(&transid, argp, sizeof(transid)))
>>>  			return -EFAULT;
>>>
>>

Re: [PATCH] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[David Sterba]

On Mon, Aug 26, 2019 at 03:37:28PM +0300, Nikolay Borisov wrote:
> Support for asynchronous snapshot creation was originally added in
> 72fd032e9424 ("Btrfs: add SNAP_CREATE_ASYNC ioctl") to cater for
> ceph's backend needs. However, since Ceph has deprecated support for
> btrfs there is no longer need for that support in btrfs. Additionally,
> this was never supported by btrfs-progs, the official userspace tools.

As there were some questions, changelog could link to the mails that
explain why ceph is not intrested anymore.

> @@ -1837,8 +1837,12 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
>  		goto free_args;
>  	}
>  
> -	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC)
> +	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC) {
> +		pr_warn("BTRFS: async snapshot creation is deprecated and will"

Why not btrfs_warn? pr_warn lacks identification of the filesystem. I'd
also write shortened identifiers of the ioctl and flags. 'async
snapshot' sounds cryptic and it might be hard to actually find what's
the actual source line implementing it.

Though we all know there are no users left, the deprecation process
assumes there are and should make it possible to notice and update the
affected code.

btrfs_warn("SNAP_CREATE_V2 ioctl with CREATE_ASYNC is deprecated and will be removed in ...")


> +			" be removed in kernel 5.7\n");

> +	pr_warn("BTRFS: START_SYNC ioctl is deprecated and will be removed in "
> +		"kernel 5.7\n");

> +	pr_warn("BTRFS: WAIT_SYNC ioctl is deprecated and will be removed in "
> +		"kernel 5.7\n");

Yeah, like here the ioctl is sufficiently described.

[PATCH v2] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[Nikolay Borisov]

Support for asynchronous snapshot creation was originally added in
72fd032e9424 ("Btrfs: add SNAP_CREATE_ASYNC ioctl") to cater for
ceph's backend needs. However, since Ceph has deprecated support for
btrfs there is no longer need for that support in btrfs. Additionally,
this was never supported by btrfs-progs, the official userspace tools.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---

V2: 
 * Switch to using btrfs_warn to make it more explicit and easily recognisable 
 it's a btrfs-sourced warning

 * Reword the SNAP_CREATE_V2 deprecation warning to make it easier to spot 
 which ioctl is deprecated. 

 fs/btrfs/ioctl.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index c343f72a84d5..185b757f6cb6 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -1837,8 +1837,14 @@ static noinline int btrfs_ioctl_snap_create_v2(struct file *file,
 		goto free_args;
 	}
 
-	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC)
+	if (vol_args->flags & BTRFS_SUBVOL_CREATE_ASYNC) {
+		struct inode *inode = file_inode(file);
+		struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
+		btrfs_warn(fs_info, "SNAP_CREATE_V2 ioctl with CREATE_ASYNC is deprecated and will"
+			   " be removed in kernel 5.7\n");
+
 		ptr = &transid;
+	}
 	if (vol_args->flags & BTRFS_SUBVOL_RDONLY)
 		readonly = true;
 	if (vol_args->flags & BTRFS_SUBVOL_QGROUP_INHERIT) {
@@ -4214,6 +4220,10 @@ static noinline long btrfs_ioctl_start_sync(struct btrfs_root *root,
 	u64 transid;
 	int ret;
 
+
+	btrfs_warn(root->fs_info, "START_SYNC ioctl is deprecated and will be removed in "
+		   "kernel 5.7\n");
+
 	trans = btrfs_attach_transaction_barrier(root);
 	if (IS_ERR(trans)) {
 		if (PTR_ERR(trans) != -ENOENT)
@@ -4241,6 +4251,9 @@ static noinline long btrfs_ioctl_wait_sync(struct btrfs_fs_info *fs_info,
 {
 	u64 transid;
 
+	btrfs_warn(fs_info, "WAIT_SYNC ioctl is deprecated and will be removed in "
+		   "kernel 5.7\n");
+
 	if (argp) {
 		if (copy_from_user(&transid, argp, sizeof(transid)))
 			return -EFAULT;
-- 
2.17.1

Re: [PATCH v2] btrfs: Deprecate BTRFS_SUBVOL_CREATE_ASYNC flag

[David Sterba]

On Mon, Aug 26, 2019 at 05:34:24PM +0300, Nikolay Borisov wrote:
> Support for asynchronous snapshot creation was originally added in
> 72fd032e9424 ("Btrfs: add SNAP_CREATE_ASYNC ioctl") to cater for
> ceph's backend needs. However, since Ceph has deprecated support for
> btrfs there is no longer need for that support in btrfs. Additionally,
> this was never supported by btrfs-progs, the official userspace tools.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Added to misc-next, thanks.