Re: [PATCH v3] btrfs: add fs state details to error messages.

[David Sterba]

On Tue, Feb 22, 2022 at 03:42:28PM -0500, Sweet Tea Dorminy wrote:
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -66,6 +66,46 @@ static struct file_system_type btrfs_root_fs_type;
>  
>  static int btrfs_remount(struct super_block *sb, int *flags, char *data);
>  
> +#define STATE_STRING_PREFACE ": state "
> +#define STATE_STRING_BUF_LEN \
> +	(sizeof(STATE_STRING_PREFACE) + BTRFS_FS_STATE_COUNT)
> +
> +/* Characters to print to indicate error conditions. RO is not an error. */
> +static const char * const fs_state_strings[] = {
> +	[BTRFS_FS_STATE_ERROR]			= "E",
> +	[BTRFS_FS_STATE_REMOUNTING]		= "M",
> +	[BTRFS_FS_STATE_RO]			= NULL,
> +	[BTRFS_FS_STATE_TRANS_ABORTED]		= "A",
> +	[BTRFS_FS_STATE_DEV_REPLACING]		= "P",
> +	[BTRFS_FS_STATE_DUMMY_FS_INFO]		= NULL,
> +	[BTRFS_FS_STATE_NO_CSUMS]		= NULL,
> +	[BTRFS_FS_STATE_LOG_CLEANUP_ERROR]	= "L",

Yeah that's the idea with the table, but I think you don't need to use
strings, it should be sufficient to use chars, and 0 works for the empty
ones.  The way you did it consumes more memory and has indirection with
the pointers to the actual single letter strings.

I'm not sure if we want the non-error states like remounting or
replacing, but actually why not, even if it's a transient state it's
another piece of information that could be useful eventually.