Re: [PATCH v2] btrfs-progs: property: introduce drop_subtree_threshold property

[Roman Mamedov <rm@romanrm.net>]

On Sun, 11 Aug 2024 12:24:15 +0000
Sidong Yang <realwakka@gmail.com> wrote:

> This patch introduces new property drop_subtree_threshold. This property
> could be set/get easily by root dir without find sysfs path.
> 
> Fixes: https://github.com/kdave/btrfs-progs/issues/795
> 
> Issue: #795
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
> ---
> v2: error msg for -ENOENT, fix desc for prop
> ---
>  cmds/property.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/cmds/property.c b/cmds/property.c
> index a36b5ab2..88344001 100644
> --- a/cmds/property.c
> +++ b/cmds/property.c
> @@ -35,6 +35,7 @@
>  #include "common/utils.h"
>  #include "common/help.h"
>  #include "common/filesystem-utils.h"
> +#include "common/sysfs-utils.h"
>  #include "cmds/commands.h"
>  #include "cmds/props.h"
>  
> @@ -236,6 +237,50 @@ out:
>  
>  	return ret;
>  }
> +static int prop_drop_subtree_threshold(enum prop_object_type type,
> +				       const char *object,
> +				       const char *name,
> +				       const char *value,
> +				       bool force) {
> +	int ret;
> +	int fd;
> +	int sysfs_fd;
> +	char buf[255];
> +
> +        fd = btrfs_open_path(object, value, false);

This line is indented with spaces instead of TABs like the rest and as it
should. I did not check if this is the only occurrence.

Checkpatch should catch this: https://docs.kernel.org/dev-tools/checkpatch.html

> +	if (fd < 0)
> +		return -errno;
> +
> +	sysfs_fd = sysfs_open_fsid_file(fd, "qgroups/drop_subtree_threshold");
> +	if (sysfs_fd < 0) {
> +		if (sysfs_fd == -ENOENT) {
> +			error("failed to access qgroups/drop_subtree_threshold for %s,"
> +			      " quota should be enabled for this property: %m",
> +			      object);
> +		}
> +		close(fd);
> +		return -errno;
> +	}
> +
> +	if (value) {
> +		ret = write(sysfs_fd, value, strlen(value));
> +	} else {
> +		ret = read(sysfs_fd, buf, 255);
> +		if (ret > 0) {
> +			buf[ret] = 0;
> +			pr_verbose(LOG_DEFAULT, "drop_subtree_threshold=%s", buf);
> +		}
> +	}
> +	if (ret < 0) {
> +		ret = -errno;
> +	} else {
> +		ret = 0;
> +	}
> +
> +	close(sysfs_fd);
> +	close(fd);
> +	return ret;
> +}
>  
>  const struct prop_handler prop_handlers[] = {
>  	{
> @@ -259,6 +304,14 @@ const struct prop_handler prop_handlers[] = {
>  		.types = prop_object_inode,
>  		.handler = prop_compression
>  	},
> +	{
> +		.name = "drop_subtree_threshold",
> +		.desc = "subtree level threshold to mark qgroup inconsistent during"
> +		"snapshot deletion, can reduce qgroup workload of snapshot deletion",
> +		.read_only = 0,
> +		.types = prop_object_root,
> +		.handler = prop_drop_subtree_threshold
> +	},
>  	{NULL, NULL, 0, 0, NULL}
>  };
>  


-- 
With respect,
Roman