[PATCHv2 0/4] btrfs-progs: Add BTRFS_IOC_SNAP_DESTROY_V2 support

[PATCHv2 0/4] btrfs-progs: Add BTRFS_IOC_SNAP_DESTROY_V2 support

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

Second version of the patchset, which pairs with v3 of kernel changes.

Changes from v1:
* Moved subvolid member to the same union containing name and devid (David)
* Dropped patch that was bumping the libbtrfsutils version (David, Wenruo)

Marcos Paulo de Souza (4):
  btrfs-progs: add IOC_SNAP_DESTROY_V2 to ioctl.h
  libbtrfsutil: add IOC_SNAP_DESTROY_V2 to ioctl.h
  libbtrfsutil: Introduce btrfs_util_delete_subvolume_by_id_fd
  cmds: subvolume: Add --subvolid argument to subvol_delete

 Documentation/btrfs-subvolume.asciidoc        |  8 ++-
 cmds/subvolume.c                              | 53 ++++++++++++++++---
 ioctl.h                                       |  8 ++-
 libbtrfsutil/btrfs.h                          |  7 ++-
 libbtrfsutil/btrfsutil.h                      | 11 ++++
 libbtrfsutil/subvolume.c                      | 16 ++++++
 tests/misc-tests/038-delete-subvolume/test.sh | 40 ++++++++++++++
 7 files changed, 134 insertions(+), 9 deletions(-)
 create mode 100755 tests/misc-tests/038-delete-subvolume/test.sh

-- 
2.24.0

[PATCHv2 1/4] btrfs-progs: add IOC_SNAP_DESTROY_V2 to ioctl.h

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This ioctl will make possible to delete a subvolume/snapshot by using
the subvolume id.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---

Changes from v1:
* Moved subvolid member to the same union containing name and devid (David)
* Changed BTRFS_SUBVOL_DELETE_BY_ID to BTRFS_SUBVOL_SPEC_BY_ID (David,
  Christoph)

 ioctl.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/ioctl.h b/ioctl.h
index 1d53c100..1a2882df 100644
--- a/ioctl.h
+++ b/ioctl.h
@@ -53,12 +53,14 @@ BUILD_ASSERT(sizeof(struct btrfs_ioctl_vol_args) == 4096);
 #define BTRFS_SUBVOL_RDONLY		(1ULL << 1)
 #define BTRFS_SUBVOL_QGROUP_INHERIT	(1ULL << 2)
 #define BTRFS_DEVICE_SPEC_BY_ID		(1ULL << 3)
+#define BTRFS_SUBVOL_SPEC_BY_ID		(1ULL << 4)
 
 #define BTRFS_VOL_ARG_V2_FLAGS_SUPPORTED		\
 			(BTRFS_SUBVOL_CREATE_ASYNC |	\
 			BTRFS_SUBVOL_RDONLY |		\
 			BTRFS_SUBVOL_QGROUP_INHERIT |	\
-			BTRFS_DEVICE_SPEC_BY_ID)
+			BTRFS_DEVICE_SPEC_BY_ID |	\
+			BTRFS_SUBVOL_SPEC_BY_ID)
 
 #define BTRFS_FSID_SIZE 16
 #define BTRFS_UUID_SIZE 16
@@ -104,6 +106,7 @@ struct btrfs_ioctl_vol_args_v2 {
 	};
 	union {
 		char name[BTRFS_SUBVOL_NAME_MAX + 1];
+		__u64 subvolid;
 		__u64 devid;
 	};
 };
@@ -940,6 +943,9 @@ static inline char *btrfs_err_str(enum btrfs_err_code err_code)
 				struct btrfs_ioctl_get_subvol_rootref_args)
 #define BTRFS_IOC_INO_LOOKUP_USER _IOWR(BTRFS_IOCTL_MAGIC, 62, \
 				struct btrfs_ioctl_ino_lookup_user_args)
+#define BTRFS_IOC_SNAP_DESTROY_V2 _IOW(BTRFS_IOCTL_MAGIC, 63, \
+				   struct btrfs_ioctl_vol_args_v2)
+
 #ifdef __cplusplus
 }
 #endif
-- 
2.24.0

[PATCHv2 2/4] libbtrfsutil: add IOC_SNAP_DESTROY_V2 to ioctl.h

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This ioctl will make possible to delete a subvolume/snapshot by using
the subvolume id.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---

Changes from v1:
* Moved subvolid member to the same union containing name and devid (David)
* Changed BTRFS_SUBVOL_DELETE_BY_ID to BTRFS_SUBVOL_SPEC_BY_ID (David,
  Christoph)


 libbtrfsutil/btrfs.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/libbtrfsutil/btrfs.h b/libbtrfsutil/btrfs.h
index c9a61b30..d1206410 100644
--- a/libbtrfsutil/btrfs.h
+++ b/libbtrfsutil/btrfs.h
@@ -36,12 +36,14 @@ struct btrfs_ioctl_vol_args {
 #define BTRFS_DEVICE_PATH_NAME_MAX 1024
 
 #define BTRFS_DEVICE_SPEC_BY_ID		(1ULL << 3)
+#define BTRFS_SUBVOL_SPEC_BY_ID	(1ULL << 4)
 
 #define BTRFS_VOL_ARG_V2_FLAGS_SUPPORTED		\
 			(BTRFS_SUBVOL_CREATE_ASYNC |	\
 			BTRFS_SUBVOL_RDONLY |		\
 			BTRFS_SUBVOL_QGROUP_INHERIT |	\
-			BTRFS_DEVICE_SPEC_BY_ID)
+			BTRFS_DEVICE_SPEC_BY_ID |	\
+			BTRFS_SUBVOL_SPEC_BY_ID)
 
 #define BTRFS_FSID_SIZE 16
 #define BTRFS_UUID_SIZE 16
@@ -119,6 +121,7 @@ struct btrfs_ioctl_vol_args_v2 {
 	};
 	union {
 		char name[BTRFS_SUBVOL_NAME_MAX + 1];
+		__u64 subvolid;
 		__u64 devid;
 	};
 };
@@ -942,5 +945,7 @@ enum btrfs_err_code {
 				struct btrfs_ioctl_get_subvol_rootref_args)
 #define BTRFS_IOC_INO_LOOKUP_USER _IOWR(BTRFS_IOCTL_MAGIC, 62, \
 				struct btrfs_ioctl_ino_lookup_user_args)
+#define BTRFS_IOC_SNAP_DESTROY_V2 _IOW(BTRFS_IOCTL_MAGIC, 63, \
+				struct btrfs_ioctl_vol_args_v2)
 
 #endif /* _LINUX_BTRFS_H */
-- 
2.24.0

[PATCHv2 3/4] libbtrfsutil: Introduce btrfs_util_delete_subvolume_by_id_fd

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This new function will use the new BTRFS_IOC_SNAP_DESTROY_V2 to delete a
subvolume using it's id. The parent_fs argument should be a mount point.

Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 libbtrfsutil/btrfsutil.h | 11 +++++++++++
 libbtrfsutil/subvolume.c | 16 ++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
index 0442af6e..c4cab2e0 100644
--- a/libbtrfsutil/btrfsutil.h
+++ b/libbtrfsutil/btrfsutil.h
@@ -488,6 +488,17 @@ enum btrfs_util_error btrfs_util_delete_subvolume_fd(int parent_fd,
 						     const char *name,
 						     int flags);
 
+/**
+ * btrfs_util_delete_subvolume_by_id_fd() - Delete a subvolume or snapshot using
+ * subvolume id.
+ * @path: Path of the subvolume to delete.
+ * @subvolid: Subvolume id of the subvolume or snapshot to be deleted.
+ *
+ * Return: %BTRFS_UTIL_OK on success, non-zero error code on failure.
+ */
+enum btrfs_util_error btrfs_util_delete_subvolume_by_id_fd(int parent_fd,
+							uint64_t subvolid);
+
 struct btrfs_util_subvolume_iterator;
 
 /**
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 3f8343a2..204a837b 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -1290,6 +1290,22 @@ PUBLIC enum btrfs_util_error btrfs_util_delete_subvolume_fd(int parent_fd,
 	return BTRFS_UTIL_OK;
 }
 
+PUBLIC enum btrfs_util_error btrfs_util_delete_subvolume_by_id_fd(int parent_fd,
+							    uint64_t subvolid)
+{
+	struct btrfs_ioctl_vol_args_v2 args = {};
+	int ret;
+
+	args.flags = BTRFS_SUBVOL_SPEC_BY_ID;
+	args.subvolid = subvolid;
+
+	ret = ioctl(parent_fd, BTRFS_IOC_SNAP_DESTROY_V2, &args);
+	if (ret == -1)
+		return BTRFS_UTIL_ERROR_SNAP_DESTROY_FAILED;
+
+	return BTRFS_UTIL_OK;
+}
+
 PUBLIC void btrfs_util_destroy_subvolume_iterator(struct btrfs_util_subvolume_iterator *iter)
 {
 	if (iter) {
-- 
2.24.0

[PATCHv2 4/4] cmds: subvolume: Add --subvolid argument to subvol_delete

[Marcos Paulo de Souza]

From: Marcos Paulo de Souza <mpdesouza@suse.com>

This can be used when a system has a file system mounted from a
subvolume, rather than the root file system, like below:

/
|- @subvol1
|- @subvol2
\- @subvol_default

If only @subvol_default is mounted, we have no path to reach
@subvol1 and @subvol2, thus no way to delete them.
This patch introduces a new argument to specify the subvolume id of the
subvolume/snapshot to be deleted.

If only @subvol_default is mounted, we have no path to reach
@subvol1 and @subvol2, thus no way to delete them. Current subvolume
delete ioctl takes a file handler point as argument, and if
@subvol_default is mounted, we can't reach @subvol1 and @subvol2 from
the same mount point.

This patch introduces a new flag to allow BTRFS_IOC_SNAP_DESTORY_V2
to delete subvolume using subvolid.

Now, we can use this new ioctl specifying the subvolume id and refer to
the same mount point. It's doesn't matter which subvolume was mounted,
since we can reach to the desired one using the subvolume id, and then
delete it.

Now "subvolume delete" receives a new argument --subvolid, which will
contain the subvolume the user wants to delete.

Fixes: #152
Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>
---
 Documentation/btrfs-subvolume.asciidoc        |  8 ++-
 cmds/subvolume.c                              | 53 ++++++++++++++++---
 tests/misc-tests/038-delete-subvolume/test.sh | 40 ++++++++++++++
 3 files changed, 94 insertions(+), 7 deletions(-)
 create mode 100755 tests/misc-tests/038-delete-subvolume/test.sh

diff --git a/Documentation/btrfs-subvolume.asciidoc b/Documentation/btrfs-subvolume.asciidoc
index 6c0af2f8..9ebfcdfe 100644
--- a/Documentation/btrfs-subvolume.asciidoc
+++ b/Documentation/btrfs-subvolume.asciidoc
@@ -59,12 +59,15 @@ directory.
 Add the newly created subvolume to a qgroup. This option can be given multiple
 times.
 
-*delete* [options] <subvolume> [<subvolume>...]::
+*delete* [options] <[<subvolume> [<subvolume>...]] | [-s|--subvolid <subvolid> <path>]>::
 Delete the subvolume(s) from the filesystem.
 +
 If <subvolume> is not a subvolume, btrfs returns an error but continues if
 there are more arguments to process.
 +
+If --subvolid if used, <path> must point to a btrfs filesystem. See `btrfs
+subvolume list` how to get the subvolume id.
++
 The corresponding directory is removed instantly but the data blocks are
 removed later in the background. The command returns immediately. See `btrfs
 subvolume sync` how to wait until the subvolume gets completely removed.
@@ -84,6 +87,9 @@ wait for transaction commit after deleting each subvolume.
 +
 -v|--verbose::::
 verbose output of operations.
++
+-s|--subvolid::::
+subvolume id of the to be removed subvolume from <path>
 
 *find-new* <subvolume> <last_gen>::
 List the recently modified files in a subvolume, after <last_gen> generation.
diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 7a5fd79b..074fc11c 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -222,7 +222,8 @@ static int wait_for_commit(int fd)
 }
 
 static const char * const cmd_subvol_delete_usage[] = {
-	"btrfs subvolume delete [options] <subvolume> [<subvolume>...]",
+	"btrfs subvolume delete [options] <subvolume> [<subvolume>...]\n"
+	"btrfs subvolume delete [options] -s|--subvolid <subvolid> <path>",
 	"Delete subvolume(s)",
 	"Delete subvolumes from the filesystem. The corresponding directory",
 	"is removed instantly but the data blocks are removed later.",
@@ -234,6 +235,7 @@ static const char * const cmd_subvol_delete_usage[] = {
 	"-c|--commit-after      wait for transaction commit at the end of the operation",
 	"-C|--commit-each       wait for transaction commit after deleting each subvolume",
 	"-v|--verbose           verbose output of operations",
+	"-s|--subvolid          subvolume id of the to be removed subvolume",
 	NULL
 };
 
@@ -246,12 +248,14 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 	char	*dname, *vname, *cpath;
 	char	*dupdname = NULL;
 	char	*dupvname = NULL;
-	char	*path;
+	char	*path = NULL;
 	DIR	*dirstream = NULL;
 	int verbose = 0;
 	int commit_mode = 0;
 	u8 fsid[BTRFS_FSID_SIZE];
+	u64 subvolid = 0;
 	char uuidbuf[BTRFS_UUID_UNPARSED_SIZE];
+	char full_volpath[BTRFS_SUBVOL_NAME_MAX];
 	struct seen_fsid *seen_fsid_hash[SEEN_FSID_HASH_SIZE] = { NULL, };
 	enum { COMMIT_AFTER = 1, COMMIT_EACH = 2 };
 	enum btrfs_util_error err;
@@ -262,11 +266,12 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 		static const struct option long_options[] = {
 			{"commit-after", no_argument, NULL, 'c'},
 			{"commit-each", no_argument, NULL, 'C'},
+			{"subvolid", required_argument, NULL, 's'},
 			{"verbose", no_argument, NULL, 'v'},
 			{NULL, 0, NULL, 0}
 		};
 
-		c = getopt_long(argc, argv, "cCv", long_options, NULL);
+		c = getopt_long(argc, argv, "cCvs:", long_options, NULL);
 		if (c < 0)
 			break;
 
@@ -280,6 +285,9 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 		case 'v':
 			verbose++;
 			break;
+		case 's':
+			subvolid = arg_strtou64(optarg);
+			break;
 		default:
 			usage_unknown_option(cmd, argv);
 		}
@@ -288,6 +296,10 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 	if (check_argc_min(argc - optind, 1))
 		return 1;
 
+	/* when using --subvolid, ensure that we have only one argument */
+	if (subvolid > 0 && check_argc_exact(argc - optind, 1))
+		return 1;
+
 	if (verbose > 0) {
 		printf("Transaction commit: %s\n",
 			!commit_mode ? "none (default)" :
@@ -296,6 +308,23 @@ static int cmd_subvol_delete(const struct cmd_struct *cmd,
 
 	cnt = optind;
 
+	/* check the following syntax: subvolume delete --subvolid <subvolid> <path> */
+	if (subvolid > 0) {
+		char *subvol;
+
+		path = argv[cnt];
+		err = btrfs_util_subvolume_path(path, subvolid, &subvol);
+		if (err) {
+			error_btrfs_util(err);
+			ret = 1;
+			goto out;
+		}
+
+		/* build new volpath using the volume name found */
+		sprintf(full_volpath, "%s/%s", path, subvol);
+		free(subvol);
+	}
+
 again:
 	path = argv[cnt];
 
@@ -318,17 +347,29 @@ again:
 	vname = basename(dupvname);
 	free(cpath);
 
+	/* when subvolid is passed, <path> will point to the mount point */
+	if (subvolid > 0)
+		dname = dupvname;
+
 	fd = btrfs_open_dir(dname, &dirstream, 1);
 	if (fd < 0) {
 		ret = 1;
 		goto out;
 	}
 
-	printf("Delete subvolume (%s): '%s/%s'\n",
+	printf("Delete subvolume (%s): ",
 		commit_mode == COMMIT_EACH || (commit_mode == COMMIT_AFTER && cnt + 1 == argc)
-		? "commit" : "no-commit", dname, vname);
+		? "commit" : "no-commit");
+
+	if (subvolid == 0)
+		printf("'%s/%s'\n", dname, vname);
+	else
+		printf("'%s'\n", full_volpath);
 
-	err = btrfs_util_delete_subvolume_fd(fd, vname, 0);
+	if (subvolid == 0)
+		err = btrfs_util_delete_subvolume_fd(fd, vname, 0);
+	else
+		err = btrfs_util_delete_subvolume_by_id_fd(fd, subvolid);
 	if (err) {
 		error_btrfs_util(err);
 		ret = 1;
diff --git a/tests/misc-tests/038-delete-subvolume/test.sh b/tests/misc-tests/038-delete-subvolume/test.sh
new file mode 100755
index 00000000..d81ae120
--- /dev/null
+++ b/tests/misc-tests/038-delete-subvolume/test.sh
@@ -0,0 +1,40 @@
+#!/bin/bash
+# test btrfs subvolume delete --subvolid <volid> <path>
+
+source "$TEST_TOP/common"
+
+check_prereq mkfs.btrfs
+check_prereq btrfs
+
+setup_root_helper
+prepare_test_dev
+
+run_check_mkfs_test_dev
+run_check_mount_test_dev
+
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT"/mysubvol1
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT"/mysubvol2
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume create "$TEST_MNT"/mysubvol3
+
+# subvolid expected failures
+run_mustfail "subvolume delete --subvolid expects an integer" \
+	$SUDO_HELPER "$TOP/btrfs" subvolume delete --subvolid aaa "$TEST_MNT"
+
+run_mustfail "subvolume delete --subvolid with invalid unexisting subvolume" \
+	$SUDO_HELPER "$TOP/btrfs" subvolume delete --subvolid 999 "$TEST_MNT"
+
+run_mustfail "subvolume delete --subvolid expects only one extra argument: the mountpoint" \
+	$SUDO_HELPER "$TOP/btrfs" subvolume delete --subvolid 256 "$TEST_MNT" "$TEST_MNT"
+
+# delete the recently created subvol using the subvolid
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete --subvolid 256 "$TEST_MNT"
+
+run_check_umount_test_dev
+
+run_check_mount_test_dev -o subvol=mysubvol2
+# when mounted the subvolume mysubvol3, mysubvol2 is not reachable by the
+# current mount point, but "subvolume delete --subvolid " should be able to
+# delete it
+run_check $SUDO_HELPER "$TOP/btrfs" subvolume delete --subvolid 258 "$TEST_MNT"
+
+run_check_umount_test_dev
-- 
2.24.0

Re: [PATCHv2 0/4] btrfs-progs: Add BTRFS_IOC_SNAP_DESTROY_V2 support

[David Sterba]

On Fri, Feb 07, 2020 at 10:10:24AM -0300, Marcos Paulo de Souza wrote:
> From: Marcos Paulo de Souza <mpdesouza@suse.com>
> 
> Second version of the patchset, which pairs with v3 of kernel changes.
> 
> Changes from v1:
> * Moved subvolid member to the same union containing name and devid (David)
> * Dropped patch that was bumping the libbtrfsutils version (David, Wenruo)
> 
> Marcos Paulo de Souza (4):
>   btrfs-progs: add IOC_SNAP_DESTROY_V2 to ioctl.h
>   libbtrfsutil: add IOC_SNAP_DESTROY_V2 to ioctl.h
>   libbtrfsutil: Introduce btrfs_util_delete_subvolume_by_id_fd
>   cmds: subvolume: Add --subvolid argument to subvol_delete

I've reworked it a bit, the libbtrfsutil changes can be done in one, and
the test should be separate.

The short option would be probably -i as I have some patches regarding
'sync' after deletion (ie. the 'subvolume sync') and 'i' resembles the
id more. The fstests might need to be updated once the final version is
in progs git but the internal test has a good coverage.

As for release target, this will be in 5.5 once it appears. Thanks.