diff mbox series

[v2,2/3] btrfs-progs: use libbtrfsutil for btrfs subvolume snapshot

Message ID 20240802112730.3575159-3-maharmstone@fb.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: use libbtrfsutil for subvolume creation | expand

Commit Message

Mark Harmstone Aug. 2, 2024, 11:27 a.m. UTC 
From: Mark Harmstone <maharmstone@meta.com>

Call btrfs_util_subvolume_snapshot in cmd_subvolume_snapshot rather than
calling the ioctl directly.

Signed-off-by: Mark Harmstone <maharmstone@fb.com>
Co-authored-by: Omar Sandoval <osandov@fb.com>
---
 cmds/subvolume.c | 94 +++++++++++++++++-------------------------------
 1 file changed, 33 insertions(+), 61 deletions(-)

Comments

Qu Wenruo Aug. 4, 2024, 7:54 a.m. UTC | #1 
在 2024/8/2 20:57, Mark Harmstone 写道:
> From: Mark Harmstone <maharmstone@meta.com>
>
> Call btrfs_util_subvolume_snapshot in cmd_subvolume_snapshot rather than
> calling the ioctl directly.
>
> Signed-off-by: Mark Harmstone <maharmstone@fb.com>
> Co-authored-by: Omar Sandoval <osandov@fb.com>

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

Thanks,
Qu
> ---
>   cmds/subvolume.c | 94 +++++++++++++++++-------------------------------
>   1 file changed, 33 insertions(+), 61 deletions(-)
>
> diff --git a/cmds/subvolume.c b/cmds/subvolume.c
> index 2a635fa2..a9664039 100644
> --- a/cmds/subvolume.c
> +++ b/cmds/subvolume.c
> @@ -635,18 +635,11 @@ static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char *
>   {
>   	char	*subvol, *dst;
>   	int	res, retval;
> -	int	fd = -1, fddst = -1;
> -	int	len;
> -	bool readonly = false;
> -	char	*dupname = NULL;
> -	char	*dupdir = NULL;
> -	const char *newname;
> -	char	*dstdir;
> +	char	*dstdir = NULL;
>   	enum btrfs_util_error err;
> -	struct btrfs_ioctl_vol_args_v2	args;
> -	struct btrfs_qgroup_inherit *inherit = NULL;
> +	struct btrfs_util_qgroup_inherit *inherit = NULL;
> +	int flags = 0;
>
> -	memset(&args, 0, sizeof(args));
>   	optind = 0;
>   	while (1) {
>   		int c = getopt(argc, argv, "i:r");
> @@ -655,14 +648,14 @@ static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char *
>
>   		switch (c) {
>   		case 'i':
> -			res = btrfs_qgroup_inherit_add_group(&inherit, optarg);
> +			res = qgroup_inherit_add_group(&inherit, optarg);
>   			if (res) {
>   				retval = res;
>   				goto out;
>   			}
>   			break;
>   		case 'r':
> -			readonly = true;
> +			flags |= BTRFS_UTIL_CREATE_SNAPSHOT_READ_ONLY;
>   			break;
>   		default:
>   			usage_unknown_option(cmd, argv);
> @@ -696,72 +689,51 @@ static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char *
>   	}
>
>   	if (res > 0) {
> +		char *dupname;
> +		const char *newname;
> +
>   		dupname = strdup(subvol);
>   		newname = path_basename(dupname);
> -		dstdir = dst;
> -	} else {
> -		dupname = strdup(dst);
> -		newname = path_basename(dupname);
> -		dupdir = strdup(dst);
> -		dstdir = path_dirname(dupdir);
> -	}
> -
> -	if (!test_issubvolname(newname)) {
> -		error("invalid snapshot name '%s'", newname);
> -		goto out;
> -	}
> -
> -	len = strlen(newname);
> -	if (len > BTRFS_VOL_NAME_MAX) {
> -		error("snapshot name too long '%s'", newname);
> -		goto out;
> -	}
>
> -	fddst = btrfs_open_dir(dstdir);
> -	if (fddst < 0)
> -		goto out;
> -
> -	fd = btrfs_open_dir(subvol);
> -	if (fd < 0)
> -		goto out;
> +		dstdir = malloc(strlen(dst) + 1 + strlen(newname) + 1);
> +		if (!dstdir) {
> +			error("out of memory");
> +			free(dupname);
> +			goto out;
> +		}
>
> -	if (readonly)
> -		args.flags |= BTRFS_SUBVOL_RDONLY;
> +		dstdir[0] = 0;
> +		strcpy(dstdir, dst);
> +		strcat(dstdir, "/");
> +		strcat(dstdir, newname);
>
> -	args.fd = fd;
> -	if (inherit) {
> -		args.flags |= BTRFS_SUBVOL_QGROUP_INHERIT;
> -		args.size = btrfs_qgroup_inherit_size(inherit);
> -		args.qgroup_inherit = inherit;
> +		free(dupname);
> +	} else {
> +		dstdir = strdup(dst);
>   	}
> -	strncpy_null(args.name, newname, sizeof(args.name));
>
> -	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args);
> -	if (res < 0) {
> -		if (errno == ETXTBSY)
> -			error("cannot snapshot '%s': source subvolume contains an active swapfile (%m)", subvol);
> -		else
> -			error("cannot snapshot '%s': %m", subvol);
> +	err = btrfs_util_subvolume_snapshot(subvol, dstdir, flags, NULL, inherit);
> +	if (err) {
> +		error_btrfs_util(err);
>   		goto out;
>   	}
>
>   	retval = 0;	/* success */
>
> -	if (readonly)
> +	if (flags & BTRFS_UTIL_CREATE_SNAPSHOT_READ_ONLY)
>   		pr_verbose(LOG_DEFAULT,
> -			   "Create readonly snapshot of '%s' in '%s/%s'\n",
> -			   subvol, dstdir, newname);
> +			   "Create readonly snapshot of '%s' in '%s'\n",
> +			   subvol, dstdir);
>   	else
>   		pr_verbose(LOG_DEFAULT,
> -			   "Create snapshot of '%s' in '%s/%s'\n",
> -			   subvol, dstdir, newname);
> +			   "Create snapshot of '%s' in '%s'\n",
> +			   subvol, dstdir);
>
>   out:
> -	close(fddst);
> -	close(fd);
> -	free(inherit);
> -	free(dupname);
> -	free(dupdir);
> +	free(dstdir);
> +
> +	if (inherit)
> +		btrfs_util_qgroup_inherit_destroy(inherit);
>
>   	return retval;
>   }
diff mbox series

Patch

diff --git a/cmds/subvolume.c b/cmds/subvolume.c
index 2a635fa2..a9664039 100644
--- a/cmds/subvolume.c
+++ b/cmds/subvolume.c
@@ -635,18 +635,11 @@  static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char *
 {
 	char	*subvol, *dst;
 	int	res, retval;
-	int	fd = -1, fddst = -1;
-	int	len;
-	bool readonly = false;
-	char	*dupname = NULL;
-	char	*dupdir = NULL;
-	const char *newname;
-	char	*dstdir;
+	char	*dstdir = NULL;
 	enum btrfs_util_error err;
-	struct btrfs_ioctl_vol_args_v2	args;
-	struct btrfs_qgroup_inherit *inherit = NULL;
+	struct btrfs_util_qgroup_inherit *inherit = NULL;
+	int flags = 0;
 
-	memset(&args, 0, sizeof(args));
 	optind = 0;
 	while (1) {
 		int c = getopt(argc, argv, "i:r");
@@ -655,14 +648,14 @@  static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char *
 
 		switch (c) {
 		case 'i':
-			res = btrfs_qgroup_inherit_add_group(&inherit, optarg);
+			res = qgroup_inherit_add_group(&inherit, optarg);
 			if (res) {
 				retval = res;
 				goto out;
 			}
 			break;
 		case 'r':
-			readonly = true;
+			flags |= BTRFS_UTIL_CREATE_SNAPSHOT_READ_ONLY;
 			break;
 		default:
 			usage_unknown_option(cmd, argv);
@@ -696,72 +689,51 @@  static int cmd_subvolume_snapshot(const struct cmd_struct *cmd, int argc, char *
 	}
 
 	if (res > 0) {
+		char *dupname;
+		const char *newname;
+
 		dupname = strdup(subvol);
 		newname = path_basename(dupname);
-		dstdir = dst;
-	} else {
-		dupname = strdup(dst);
-		newname = path_basename(dupname);
-		dupdir = strdup(dst);
-		dstdir = path_dirname(dupdir);
-	}
-
-	if (!test_issubvolname(newname)) {
-		error("invalid snapshot name '%s'", newname);
-		goto out;
-	}
-
-	len = strlen(newname);
-	if (len > BTRFS_VOL_NAME_MAX) {
-		error("snapshot name too long '%s'", newname);
-		goto out;
-	}
 
-	fddst = btrfs_open_dir(dstdir);
-	if (fddst < 0)
-		goto out;
-
-	fd = btrfs_open_dir(subvol);
-	if (fd < 0)
-		goto out;
+		dstdir = malloc(strlen(dst) + 1 + strlen(newname) + 1);
+		if (!dstdir) {
+			error("out of memory");
+			free(dupname);
+			goto out;
+		}
 
-	if (readonly)
-		args.flags |= BTRFS_SUBVOL_RDONLY;
+		dstdir[0] = 0;
+		strcpy(dstdir, dst);
+		strcat(dstdir, "/");
+		strcat(dstdir, newname);
 
-	args.fd = fd;
-	if (inherit) {
-		args.flags |= BTRFS_SUBVOL_QGROUP_INHERIT;
-		args.size = btrfs_qgroup_inherit_size(inherit);
-		args.qgroup_inherit = inherit;
+		free(dupname);
+	} else {
+		dstdir = strdup(dst);
 	}
-	strncpy_null(args.name, newname, sizeof(args.name));
 
-	res = ioctl(fddst, BTRFS_IOC_SNAP_CREATE_V2, &args);
-	if (res < 0) {
-		if (errno == ETXTBSY)
-			error("cannot snapshot '%s': source subvolume contains an active swapfile (%m)", subvol);
-		else
-			error("cannot snapshot '%s': %m", subvol);
+	err = btrfs_util_subvolume_snapshot(subvol, dstdir, flags, NULL, inherit);
+	if (err) {
+		error_btrfs_util(err);
 		goto out;
 	}
 
 	retval = 0;	/* success */
 
-	if (readonly)
+	if (flags & BTRFS_UTIL_CREATE_SNAPSHOT_READ_ONLY)
 		pr_verbose(LOG_DEFAULT,
-			   "Create readonly snapshot of '%s' in '%s/%s'\n",
-			   subvol, dstdir, newname);
+			   "Create readonly snapshot of '%s' in '%s'\n",
+			   subvol, dstdir);
 	else
 		pr_verbose(LOG_DEFAULT,
-			   "Create snapshot of '%s' in '%s/%s'\n",
-			   subvol, dstdir, newname);
+			   "Create snapshot of '%s' in '%s'\n",
+			   subvol, dstdir);
 
 out:
-	close(fddst);
-	close(fd);
-	free(inherit);
-	free(dupname);
-	free(dupdir);
+	free(dstdir);
+
+	if (inherit)
+		btrfs_util_qgroup_inherit_destroy(inherit);
 
 	return retval;
 }