Re: [PATCH 1/2] btrfs-progs: do extra warning when setting ro false on received subvolume

[Nikolay Borisov <nborisov@suse.com>]

On 10.09.21 г. 9:03, Qu Wenruo wrote:
> When flipping received subvolume from RO to RW, any new write to the
> subvolume could cause later incremental receive to fail or corrupt data.
> 
> Thus we're trying to clear received UUID when doing such RO->RW flip, to
> prevent data corruption.
> 
> But unfortunately the old (and wrong) behavior has been there for a
> while, and changing the kernel behavior will make existing users
> confused.
> 
> Thus here we enhance subvolume read-only prop to do extra warning on
> users to educate them about both the new behavior and the problems of
> old behaviors.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  props.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/props.c b/props.c
> index 81509e48cd16..b86ecc61b5b3 100644
> --- a/props.c
> +++ b/props.c
> @@ -39,6 +39,15 @@
>  #define ENOATTR ENODATA
>  #endif
>  
> +static void do_warn_about_rorw_flip(const char *path)
> +{
> +	warning("Flipping subvolume %s from RO to RW will cause either:", path);
> +	warning("- No more incremental receive based on this subvolume");
> +	warning("  Newer kernels will remove the recevied UUID to prevent corruption");
> +	warning("- Data corruption or receive corruption doing incremental receive");
> +	warning("  Older kernels can't detect the modification, and cause corruption or receive failure");

This is a bad format for a warning, let's keep it simple - just state
that flipping ro->rw would break incremental send and that's that.

> +}
> +
>  static int prop_read_only(enum prop_object_type type,
>  			  const char *object,
>  			  const char *name,
> @@ -48,6 +57,9 @@ static int prop_read_only(enum prop_object_type type,
>  	bool read_only;
>  
>  	if (value) {
> +		struct btrfs_util_subvolume_info subvol;
> +		u8 empty_uuid[BTRFS_UUID_SIZE] = { 0 };
> +
>  		if (!strcmp(value, "true")) {
>  			read_only = true;
>  		} else if (!strcmp(value, "false")) {
> @@ -57,6 +69,15 @@ static int prop_read_only(enum prop_object_type type,
>  			return -EINVAL;
>  		}
>  
> +		err = btrfs_util_subvolume_info(object, 0, &subvol);
> +		if (err) {
> +			warning("unable to get subvolume info for path %s",
> +				object);
> +		} else if (!read_only && memcmp(empty_uuid, subvol.received_uuid,
> +				   BTRFS_UUID_SIZE)){
> +			do_warn_about_rorw_flip(object);
> +		}
> +
>  		err = btrfs_util_set_subvolume_read_only(object, read_only);
>  		if (err) {
>  			error_btrfs_util(err);
>