diff mbox series

btrfs-progs: add async option to btrfs property set

Message ID 7bef0f743bf7321000d2dbc1b6a4c8f71b78c98b.1598652426.git.boris@bur.io (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: add async option to btrfs property set | expand

Commit Message

Boris Burkov Aug. 31, 2020, 4:47 p.m. UTC 
When setting read-only, we want to avoid unnecessary sync-ing. However,
users may have come to rely on the implicit sync, so keep it in user
space while removing it from the kernel. For those who don't want the
likely unnecessary sync, add '-a' for async to skip the sync.

Since property is a fairly generic command, it is a little tricky to
properly apply -a in all cases. Options as far as I can see it are:
- fail on everything but ro set
- accept but ignore on everything but ro set
- accept and implement for all sets, fail/ignore on get

I opted for the first, least accepting option, though that may be
annoying.

Implementing for other sets is not straightforward, as the underlying
kernel interface may implicitly sync. For example, setting the label
does a transaction commit.

Signed-off-by: Boris Burkov <boris@bur.io>
---
 cmds/property.c          | 37 +++++++++++++++++++++++++------------
 libbtrfsutil/btrfsutil.h |  7 +++++--
 libbtrfsutil/subvolume.c | 11 ++++++++---
 props.c                  | 22 ++++++++++++++++++----
 props.h                  |  5 ++++-
 5 files changed, 60 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/cmds/property.c b/cmds/property.c
index 62cadf71..c5f930dd 100644
--- a/cmds/property.c
+++ b/cmds/property.c
@@ -29,7 +29,7 @@ 
 #include "common/help.h"
 
 static const char * const property_cmd_group_usage[] = {
-	"btrfs property get/set/list [-t <type>] <object> [<name>] [value]",
+	"btrfs property get/set/list [-t <type>] [-a] <object> [<name>] [value]",
 	NULL
 };
 
@@ -182,7 +182,7 @@  static int dump_prop(const struct prop_handler *prop,
 
 	if ((types & type) && (prop->types & type)) {
 		if (!name_and_help)
-			ret = prop->handler(type, object, prop->name, NULL);
+			ret = prop->handler(type, object, prop->name, NULL, true);
 		else
 			printf("%-20s%s\n", prop->name, prop->desc);
 	}
@@ -214,7 +214,7 @@  out:
 }
 
 static int setget_prop(int types, const char *object,
-		       const char *name, const char *value)
+		       const char *name, const char *value, bool sync)
 {
 	int ret;
 	const struct prop_handler *prop = NULL;
@@ -242,7 +242,7 @@  static int setget_prop(int types, const char *object,
 		return 1;
 	}
 
-	ret = prop->handler(types, object, name, value);
+	ret = prop->handler(types, object, name, value, sync);
 
 	if (ret < 0)
 		ret = 1;
@@ -255,15 +255,18 @@  static int setget_prop(int types, const char *object,
 
 static int parse_args(const struct cmd_struct *cmd, int argc, char **argv,
 		       int *types, char **object,
-		       char **name, char **value, int min_nonopt_args)
+		       char **name, char **value,
+		       bool *sync, int min_nonopt_args)
 {
 	int ret;
 	char *type_str = NULL;
 	int max_nonopt_args = 1;
 
+	if (sync)
+		*sync = true;
 	optind = 1;
 	while (1) {
-		int c = getopt(argc, argv, "t:");
+		int c = getopt(argc, argv, "t:a");
 		if (c < 0)
 			break;
 
@@ -271,6 +274,14 @@  static int parse_args(const struct cmd_struct *cmd, int argc, char **argv,
 		case 't':
 			type_str = optarg;
 			break;
+		case 'a':
+			if (!sync) {
+				errno = -EINVAL;
+				error("async not supported for command");
+				return 1;
+			}
+			*sync = false;
+			break;
 		default:
 			usage_unknown_option(cmd, argv);
 		}
@@ -352,11 +363,11 @@  static int cmd_property_get(const struct cmd_struct *cmd,
 	char *name = NULL;
 	int types = 0;
 
-	if (parse_args(cmd, argc, argv, &types, &object, &name, NULL, 1))
+	if (parse_args(cmd, argc, argv, &types, &object, &name, NULL, NULL, 1))
 		return 1;
 
 	if (name)
-		ret = setget_prop(types, object, name, NULL);
+		ret = setget_prop(types, object, name, NULL, false);
 	else
 		ret = dump_props(types, object, 0);
 
@@ -365,13 +376,14 @@  static int cmd_property_get(const struct cmd_struct *cmd,
 static DEFINE_SIMPLE_COMMAND(property_get, "get");
 
 static const char * const cmd_property_set_usage[] = {
-	"btrfs property set [-t <type>] <object> <name> <value>",
+	"btrfs property set [-t <type>] [-a] <object> <name> <value>",
 	"Set a property on a btrfs object",
 	"Set a property on a btrfs object where object is a path to file or",
 	"directory and can also represent the filesystem or device based on the type",
 	"",
 	"-t <TYPE>       list properties for the given object type (inode, subvol,",
 	"                filesystem, device)",
+	"-a              async: skip sync after setting the property",
 	NULL
 };
 
@@ -383,11 +395,12 @@  static int cmd_property_set(const struct cmd_struct *cmd,
 	char *name = NULL;
 	char *value = NULL;
 	int types = 0;
+	bool sync;
 
-	if (parse_args(cmd, argc, argv, &types, &object, &name, &value, 3))
+	if (parse_args(cmd, argc, argv, &types, &object, &name, &value, &sync, 3))
 		return 1;
 
-	ret = setget_prop(types, object, name, value);
+	ret = setget_prop(types, object, name, value, sync);
 
 	return ret;
 }
@@ -412,7 +425,7 @@  static int cmd_property_list(const struct cmd_struct *cmd,
 	char *object = NULL;
 	int types = 0;
 
-	if (parse_args(cmd, argc, argv, &types, &object, NULL, NULL, 1))
+	if (parse_args(cmd, argc, argv, &types, &object, NULL, NULL, NULL, 1))
 		return 1;
 
 	ret = dump_props(types, object, 1);
diff --git a/libbtrfsutil/btrfsutil.h b/libbtrfsutil/btrfsutil.h
index b7894e14..cb07c48e 100644
--- a/libbtrfsutil/btrfsutil.h
+++ b/libbtrfsutil/btrfsutil.h
@@ -304,20 +304,23 @@  enum btrfs_util_error btrfs_util_get_subvolume_read_only_fd(int fd, bool *ret);
  * btrfs_util_set_subvolume_read_only() - Set whether a subvolume is read-only.
  * @path: Subvolume path.
  * @read_only: New value of read-only flag.
+ * @sync: Whether or not to block on sync-ing the subvolume after.
  *
  * This requires appropriate privilege (CAP_SYS_ADMIN).
  *
  * Return: %BTRFS_UTIL_OK on success, non-zero error code on failure.
  */
 enum btrfs_util_error btrfs_util_set_subvolume_read_only(const char *path,
-							 bool read_only);
+							 bool read_only,
+							 bool sync);
 
 /**
  * btrfs_util_set_subvolume_read_only_fd() - See
  * btrfs_util_set_subvolume_read_only().
  */
 enum btrfs_util_error btrfs_util_set_subvolume_read_only_fd(int fd,
-							    bool read_only);
+							    bool read_only,
+							    bool sync);
 
 /**
  * btrfs_util_get_default_subvolume() - Get the default subvolume for a
diff --git a/libbtrfsutil/subvolume.c b/libbtrfsutil/subvolume.c
index 204a837b..44647276 100644
--- a/libbtrfsutil/subvolume.c
+++ b/libbtrfsutil/subvolume.c
@@ -482,7 +482,8 @@  PUBLIC enum btrfs_util_error btrfs_util_get_subvolume_read_only(const char *path
 }
 
 PUBLIC enum btrfs_util_error btrfs_util_set_subvolume_read_only(const char *path,
-								bool read_only)
+								bool read_only,
+								bool sync)
 {
 	enum btrfs_util_error err;
 	int fd;
@@ -491,13 +492,14 @@  PUBLIC enum btrfs_util_error btrfs_util_set_subvolume_read_only(const char *path
 	if (fd == -1)
 		return BTRFS_UTIL_ERROR_OPEN_FAILED;
 
-	err = btrfs_util_set_subvolume_read_only_fd(fd, read_only);
+	err = btrfs_util_set_subvolume_read_only_fd(fd, read_only, sync);
 	SAVE_ERRNO_AND_CLOSE(fd);
 	return err;
 }
 
 PUBLIC enum btrfs_util_error btrfs_util_set_subvolume_read_only_fd(int fd,
-								   bool read_only)
+								   bool read_only,
+								   bool sync)
 {
 	uint64_t flags;
 	int ret;
@@ -515,6 +517,9 @@  PUBLIC enum btrfs_util_error btrfs_util_set_subvolume_read_only_fd(int fd,
 	if (ret == -1)
 		return BTRFS_UTIL_ERROR_SUBVOL_SETFLAGS_FAILED;
 
+	if (sync)
+		return btrfs_util_sync_fd(fd);
+
 	return BTRFS_UTIL_OK;
 }
 
diff --git a/props.c b/props.c
index 0cfc358d..0313f22b 100644
--- a/props.c
+++ b/props.c
@@ -41,7 +41,8 @@ 
 static int prop_read_only(enum prop_object_type type,
 			  const char *object,
 			  const char *name,
-			  const char *value)
+			  const char *value,
+			  bool sync)
 {
 	enum btrfs_util_error err;
 	bool read_only;
@@ -56,7 +57,7 @@  static int prop_read_only(enum prop_object_type type,
 			return -EINVAL;
 		}
 
-		err = btrfs_util_set_subvolume_read_only(object, read_only);
+		err = btrfs_util_set_subvolume_read_only(object, read_only, sync);
 		if (err) {
 			error_btrfs_util(err);
 			return -errno;
@@ -77,10 +78,16 @@  static int prop_read_only(enum prop_object_type type,
 static int prop_label(enum prop_object_type type,
 		      const char *object,
 		      const char *name,
-		      const char *value)
+		      const char *value,
+		      bool sync)
 {
 	int ret;
 
+	if (!sync) {
+		error("async not supported for label");
+		return -EINVAL;
+	}
+
 	if (value) {
 		ret = set_label((char *) object, (char *) value);
 	} else {
@@ -97,7 +104,8 @@  static int prop_label(enum prop_object_type type,
 static int prop_compression(enum prop_object_type type,
 			    const char *object,
 			    const char *name,
-			    const char *value)
+			    const char *value,
+			    bool sync)
 {
 	int ret;
 	ssize_t sret;
@@ -107,6 +115,12 @@  static int prop_compression(enum prop_object_type type,
 	char *xattr_name = NULL;
 	int open_flags = value ? O_RDWR : O_RDONLY;
 
+	if (!sync) {
+		ret = -EINVAL;
+		error("async not supported for compression");
+		goto out;
+	}
+
 	fd = open_file_or_dir3(object, &dirstream, open_flags);
 	if (fd == -1) {
 		ret = -errno;
diff --git a/props.h b/props.h
index a43cb253..970185f0 100644
--- a/props.h
+++ b/props.h
@@ -17,6 +17,8 @@ 
 #ifndef __BTRFS_PROPS_H__
 #define __BTRFS_PROPS_H__
 
+#include <stdbool.h>
+
 enum prop_object_type {
 	prop_object_dev		= (1 << 0),
 	prop_object_root	= (1 << 1),
@@ -28,7 +30,8 @@  enum prop_object_type {
 typedef int (*prop_handler_t)(enum prop_object_type type,
 			      const char *object,
 			      const char *name,
-			      const char *value);
+			      const char *value,
+			      bool sync);
 
 struct prop_handler {
 	const char *name;