On Tue, Nov 27, 2018 at 08:53:41PM +0100, David Sterba wrote:
> We can use simple enum for values that are not part of on-disk format:
> global filesystem states.

Reviewed-by: Omar Sandoval <osan...@fb.com>

Some typos/wording suggestions below.

> Signed-off-by: David Sterba <dste...@suse.com>
> ---
>  fs/btrfs/ctree.h | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index a98507fa9192..f82ec5e41b0c 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -109,13 +109,26 @@ static inline unsigned long btrfs_chunk_item_size(int 
> num_stripes)
>  }
>  
>  /*
> - * File system states
> + * Runtime (in-memory) states of filesystem
>   */
> -#define BTRFS_FS_STATE_ERROR         0
> -#define BTRFS_FS_STATE_REMOUNTING    1
> -#define BTRFS_FS_STATE_TRANS_ABORTED 2
> -#define BTRFS_FS_STATE_DEV_REPLACING 3
> -#define BTRFS_FS_STATE_DUMMY_FS_INFO 4
> +enum {
> +     /* Global indicator of serious filesysystem errors */

filesysystem -> filesystem

> +     BTRFS_FS_STATE_ERROR,
> +     /*
> +      * Filesystem is being remounted, allow to skip some operations, like
> +      * defrag
> +      */
> +     BTRFS_FS_STATE_REMOUNTING,
> +     /* Track if the transaction abort has been reported */

Which one is "the" transaction abort? This gives me the impression that
this is a flag on the transaction, but it's actually filesystem state.
Maybe "Track if a transaction abort has been reported on this
filesystem"?

> +     BTRFS_FS_STATE_TRANS_ABORTED,
> +     /*
> +      * Indicate that replace source or target device state is changed and
> +      * allow to block bio operations
> +      */

Again, this makes it sound like it's device state, but it's actually
filesystem state. How about "Bio operations should be blocked on this
filesystem because a source or target device is being destroyed as part
of a device replace"?

> +     BTRFS_FS_STATE_DEV_REPLACING,
> +     /* The btrfs_fs_info created for self-tests */
> +     BTRFS_FS_STATE_DUMMY_FS_INFO,
> +};
>  
>  #define BTRFS_BACKREF_REV_MAX                256
>  #define BTRFS_BACKREF_REV_SHIFT              56
> -- 
> 2.19.1
> 

Reply via email to