Re: [PATCH v2] btrfs-progs: Remove support for BTRFS_SUBVOL_CREATE_ASYNC

[Omar Sandoval <osandov@osandov.com>]

On Mon, Mar 16, 2020 at 11:28:36AM +0200, Nikolay Borisov wrote:
> Kernel has removed support for this feature in 5.7 so let's remove
> support from progs as well.

Built and passed tests on my machine. A few very trivial nits below,
otherwise:

Reviewed-by: Omar Sandoval <osandov@fb.com>

Thanks!

> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
> Changelog v2:
>  * Removed async mentions in README.md
>  * Changed docs in libbtrfsutil/btrfsutil.h to mention async is unused.
>  * Removed tests using async_
>  * Changed python module's doc to mention the async_ parameter is unused.
> 
>  ioctl.h                                     |  4 +--
>  libbtrfsutil/README.md                      | 14 ++------
>  libbtrfsutil/btrfs.h                        |  4 +--
>  libbtrfsutil/btrfsutil.h                    | 18 +++++-----
>  libbtrfsutil/python/module.c                |  6 ++--
>  libbtrfsutil/python/tests/test_subvolume.py | 12 ++-----
>  libbtrfsutil/subvolume.c                    | 38 ++++++---------------
>  7 files changed, 29 insertions(+), 67 deletions(-)

[snip]

> diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
> index 0442af6ed67f..47d4cf1e5fe9 100644
> --- a/libbtrfsutil/btrfsutil.h
> +++ b/libbtrfsutil/btrfsutil.h
> @@ -366,7 +366,7 @@ struct btrfs_util_qgroup_inherit;
>   * btrfs_util_create_subvolume() - Create a new subvolume.
>   * @path: Where to create the subvolume.
>   * @flags: Must be zero.
> - * @async_transid: If not NULL, create the subvolume asynchronously (i.e.,
> + * @unused: No longer used
>   * without waiting for it to commit it to disk) and return the transaction ID
>   * that it was created in. This transaction ID can be waited on with
>   * btrfs_util_wait_sync().

These three lines need to be deleted, too.

> @@ -375,7 +375,7 @@ struct btrfs_util_qgroup_inherit;
>   * Return: %BTRFS_UTIL_OK on success, non-zero error code on failure.
>   */
>  enum btrfs_util_error btrfs_util_create_subvolume(const char *path, int flags,
> -						  uint64_t *async_transid,
> +						  uint64_t *unused,
>  						  struct btrfs_util_qgroup_inherit *qgroup_inherit);
> 
>  /**
> @@ -385,7 +385,7 @@ enum btrfs_util_error btrfs_util_create_subvolume(const char *path, int flags,
>   * should be created.
>   * @name: Name of the subvolume to create.
>   * @flags: See btrfs_util_create_subvolume().
> - * @async_transid: See btrfs_util_create_subvolume().
> + * @unused: See btrfs_util_create_subvolume().
>   * @qgroup_inherit: See btrfs_util_create_subvolume().
>   *
>   * Return: %BTRFS_UTIL_OK on success, non-zero error code on failure.
> @@ -393,7 +393,7 @@ enum btrfs_util_error btrfs_util_create_subvolume(const char *path, int flags,
>  enum btrfs_util_error btrfs_util_create_subvolume_fd(int parent_fd,
>  						     const char *name,
>  						     int flags,
> -						     uint64_t *async_transid,
> +						     uint64_t *unused,
>  						     struct btrfs_util_qgroup_inherit *qgroup_inherit);
> 
>  /**
> @@ -418,7 +418,7 @@ enum btrfs_util_error btrfs_util_create_subvolume_fd(int parent_fd,
>   * @source: Path of the existing subvolume to snapshot.
>   * @path: Where to create the snapshot.
>   * @flags: Bitmask of BTRFS_UTIL_CREATE_SNAPSHOT_* flags.
> - * @async_transid: See btrfs_util_create_subvolume(). If
> + * @unused: See btrfs_util_create_subvolume(). If
>   * %BTRFS_UTIL_CREATE_SNAPSHOT_RECURSIVE was in @flags, then this will contain
>   * the largest transaction ID of all created subvolumes.

The "If ..." description needs to go away, too.

>   * @qgroup_inherit: See btrfs_util_create_subvolume().
> @@ -427,7 +427,7 @@ enum btrfs_util_error btrfs_util_create_subvolume_fd(int parent_fd,
>   */
>  enum btrfs_util_error btrfs_util_create_snapshot(const char *source,
>  						 const char *path, int flags,
> -						 uint64_t *async_transid,
> +						 uint64_t *unused,
>  						 struct btrfs_util_qgroup_inherit *qgroup_inherit);
> 
>  /**
> @@ -435,7 +435,7 @@ enum btrfs_util_error btrfs_util_create_snapshot(const char *source,
>   */
>  enum btrfs_util_error btrfs_util_create_snapshot_fd(int fd, const char *path,
>  						    int flags,
> -						    uint64_t *async_transid,
> +						    uint64_t *unused,
>  						    struct btrfs_util_qgroup_inherit *qgroup_inherit);
> 
>  /**
> @@ -446,13 +446,13 @@ enum btrfs_util_error btrfs_util_create_snapshot_fd(int fd, const char *path,
>   * be created.
>   * @name: Name of the snapshot to create.
>   * @flags: See btrfs_util_create_snapshot().
> - * @async_transid: See btrfs_util_create_snapshot().
> + * @unused: See btrfs_util_create_snapshot().
>   * @qgroup_inherit: See btrfs_util_create_snapshot().
>   */
>  enum btrfs_util_error btrfs_util_create_snapshot_fd2(int fd, int parent_fd,
>  						     const char *name,
>  						     int flags,
> -						     uint64_t *async_transid,
> +						     uint64_t *unused,
>  						     struct btrfs_util_qgroup_inherit *qgroup_inherit);
> 
>  /**
> diff --git a/libbtrfsutil/python/module.c b/libbtrfsutil/python/module.c
> index f8260c84ec76..a8aa50bdd7ed 100644
> --- a/libbtrfsutil/python/module.c
> +++ b/libbtrfsutil/python/module.c
> @@ -237,8 +237,7 @@ static PyMethodDef btrfsutil_methods[] = {
>  	 "Create a new subvolume.\n\n"
>  	 "Arguments:\n"
>  	 "path -- string, bytes, or path-like object\n"
> -	 "async_ -- create the subvolume without waiting for it to commit to\n"
> -	 "disk and return the transaction ID\n"
> +	 "async_ -- No longer used\n"

Lowercase "no" for consistency with the other docstrings.

>  	 "qgroup_inherit -- optional QgroupInherit object of qgroups to\n"
>  	 "inherit from"},
>  	{"create_snapshot", (PyCFunction)create_snapshot,
> @@ -251,8 +250,7 @@ static PyMethodDef btrfsutil_methods[] = {
>  	 "path -- string, bytes, or path-like object\n"
>  	 "recursive -- also snapshot child subvolumes\n"
>  	 "read_only -- create a read-only snapshot\n"
> -	 "async_ -- create the subvolume without waiting for it to commit to\n"
> -	 "disk and return the transaction ID\n"
> +	 "async_ -- No longer used\n"

Ditto