Re: [PATCH 2/2] btrfs-progs: move inode cache removal to rescue group

[Qu Wenruo <wqu@suse.com>]

On 2023/10/10 00:53, David Sterba wrote:
> On Mon, Oct 09, 2023 at 03:17:00PM +1030, Qu Wenruo wrote:
>> The option "--clear-ino-cache" is not really that suitable for "btrfs
>> check" group.
>>
>> Let's move it to "btrfs rescue" group to fix those small hiccups, just
>> like the existing "btrfs rescue fix-device-size" command.
>>
>> For now, "btrfs check --clear-ino-cache" would still work, with one
>> extra warning referring to "btrfs rescue clear-ino-cache".
>> This is mostly to reduce the surprise, and keep script users (I doubt if
>> there is any though) happy for now.
>>
>> In the next or two releases, we would fully remove the support in "btrfs
>> check" group.
>>
>> Another small change is, in the documents, we refer to the feature as
>> "inode map", which doesn't match with the mount option documents.
>> Since we're here, unify them to "inode cache" feature.
>>
>> Issue: #669
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Please don't forget to add the new command to btrfs-completion. I've
> noticed 2 more were missing so it's done in a separate commit.
> 
>> ---
>>   Documentation/btrfs-check.rst  |  5 +++-
>>   Documentation/btrfs-rescue.rst |  6 ++++
>>   check/main.c                   |  1 +
>>   cmds/rescue.c                  | 52 ++++++++++++++++++++++++++++++++++
>>   4 files changed, 63 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/btrfs-check.rst b/Documentation/btrfs-check.rst
>> index cf8de9fcc888..3c5f96f1951f 100644
>> --- a/Documentation/btrfs-check.rst
>> +++ b/Documentation/btrfs-check.rst
>> @@ -84,8 +84,11 @@ SAFE OR ADVISORY OPTIONS
>>           See also the *clear_cache* mount option.
>>   
>>   --clear-ino-cache
>> -        remove leftover items pertaining to the deprecated inode map feature
>> +        remove leftover items pertaining to the deprecated `inode cache` feature
>>   
>> +	.. warning::
>> +		This option is deprecated, please use `btrfs rescue clear-ino-cache`
>> +		instead, this option would be removed in the future eventually.
>>   
>>   DANGEROUS OPTIONS
>>   -----------------
>> diff --git a/Documentation/btrfs-rescue.rst b/Documentation/btrfs-rescue.rst
>> index 39d250cefa48..e99aa4ad8a7e 100644
>> --- a/Documentation/btrfs-rescue.rst
>> +++ b/Documentation/btrfs-rescue.rst
>> @@ -50,6 +50,12 @@ fix-device-size <device>
>>   
>>                   WARNING: CPU: 3 PID: 439 at fs/btrfs/ctree.h:1559 btrfs_update_device+0x1c5/0x1d0 [btrfs]
>>   
>> +clear-ino-cache <device>
>> +        Remove leftover items pertaining to the deprecated `inode cache` feature.
>> +
>> +	The `inode cache` feature (enabled by mount option "inode_cache") is
>> +	fully removed in v5.11 kernel.
>> +
>>   clear-uuid-tree <device>
>>           Clear UUID tree, so that kernel can re-generate it at next read-write
>>           mount.
>> diff --git a/check/main.c b/check/main.c
>> index 1174939fd6eb..7760511b85d9 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -10242,6 +10242,7 @@ static int cmd_check(const struct cmd_struct *cmd, int argc, char **argv)
>>   	}
>>   
>>   	if (clear_ino_cache) {
>> +		warning("--clear-ino-cache option is deprecated, please use \"btrfs rescue clear-ino-cache\" instead.")
> 
> No "." at the end of the text and the ";" is missing, does not
> compile.

My bad, I noticed the warning is missing thus added it in the last minute...

Do I need to resend or you have already fixed it during merge?

Thanks,
Qu
> 
>>   		ret = clear_ino_cache_items(gfs_info);
>>   		err = ret;
>>   		goto close_out;
>> diff --git a/cmds/rescue.c b/cmds/rescue.c
>> index be6f5016d5a9..38f4e1423434 100644
>> --- a/cmds/rescue.c
>> +++ b/cmds/rescue.c
>> @@ -34,6 +34,7 @@
>>   #include "common/utils.h"
>>   #include "common/help.h"
>>   #include "common/open-utils.h"
>> +#include "common/clear-cache.h"
>>   #include "cmds/commands.h"
>>   #include "cmds/rescue.h"
>>   
>> @@ -405,6 +406,56 @@ out:
>>   }
>>   static DEFINE_SIMPLE_COMMAND(rescue_clear_uuid_tree, "clear-uuid-tree");
>>   
>> +static const char * const cmd_rescue_clear_ino_cache_usage[] = {
>> +	"btrfs rescue clear-ino-cache <device>",
>> +	"remove leftover items pertaining to the deprecated inode cache feature",
>> +	NULL
>> +};
>> +
>> +static int cmd_rescue_clear_ino_cache(const struct cmd_struct *cmd,
>> +				      int argc, char **argv)
>> +{
>> +	struct open_ctree_args oca = { 0 };
>> +	struct btrfs_fs_info *fs_info;
>> +	char *devname;
>> +	int ret;
>> +
>> +	clean_args_no_options(cmd, argc, argv);
>> +
>> +	if (check_argc_exact(argc, 2))
>> +		return 1;
>> +
>> +	devname = argv[optind];
>> +	ret = check_mounted(devname);
>> +	if (ret < 0) {
>> +		errno = -ret;
>> +		error("could not check mount status: %m");
>> +		goto out;
>> +	} else if (ret) {
>> +		error("%s is currently mounted", devname);
>> +		ret = -EBUSY;
>> +		goto out;
>> +	}
>> +	oca.filename = devname;
>> +	oca.flags = OPEN_CTREE_WRITES;
>> +	fs_info = open_ctree_fs_info(&oca);
>> +	if (!fs_info) {
>> +		error("could not open btrfs");
>> +		ret = -EIO;
>> +		goto out;
>> +	}
>> +	ret = clear_ino_cache_items(fs_info);
>> +	if (ret < 0) {
>> +		errno = -ret;
>> +		error("failed to clear ino cache: %m");
>> +	} else {
>> +		pr_verbose(LOG_DEFAULT, "Successfully cleared ino cache");
>> +	}
>> +out:
>> +	return !!ret;
>> +}
>> +static DEFINE_SIMPLE_COMMAND(rescue_clear_ino_cache, "clear-ino-cache");
>> +
>>   static const char rescue_cmd_group_info[] =
>>   "toolbox for specific rescue operations";
>>   
>> @@ -416,6 +467,7 @@ static const struct cmd_group rescue_cmd_group = {
>>   		&cmd_struct_rescue_fix_device_size,
>>   		&cmd_struct_rescue_create_control_device,
>>   		&cmd_struct_rescue_clear_uuid_tree,
>> +		&cmd_struct_rescue_clear_ino_cache,
>>   		NULL
>>   	}
>>   };
>> -- 
>> 2.42.0