Re: [PATCH 1/6] btrfs: metadata_uuid: fix failed assertion due to unsuccessful device scan

[Nikolay Borisov <nborisov@suse.com>]

On 12.12.19 г. 13:01 ч., damenly.su@gmail.com wrote:

<snip>

> Acutally, there are two devices in the fs. Device 2 with
> FSID_CHANGING_V2 allocated a fs_devices. But, device 1 found the
> fs_devices but failed to be added into since fs_devices->opened (

It's not clear why device 1 wasn't able to be added to the fs_devices
allocated by dev 2. Please elaborate?


> the thread is doing mount device 1). But device 1's fsid was copied
> to fs_devices->fsid then the assertion failed.


dev 1 fsid should be copied iff its transid is newer.

> 
> The solution is that only if a new device was added into a existing
> fs_device, then the fs_devices->fsid is allowed to be rewritten.

fs_devices->fsid must be re-written by any device which is _newer_ w.r.t
to the transid.

> 
> Fixes: 7a62d0f07377 ("btrfs: Handle one more split-brain scenario during fsid change")
> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
> ---
>  fs/btrfs/volumes.c | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index d8e5560db285..9efa4123c335 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -732,6 +732,9 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>  		BTRFS_FEATURE_INCOMPAT_METADATA_UUID);
>  	bool fsid_change_in_progress = (btrfs_super_flags(disk_super) &
>  					BTRFS_SUPER_FLAG_CHANGING_FSID_V2);
> +	bool fs_devices_found = false;
> +
> +	*new_device_added = false;
>  
>  	if (fsid_change_in_progress) {
>  		if (!has_metadata_uuid) {
> @@ -772,24 +775,11 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>  
>  		device = NULL;
>  	} else {
> +		fs_devices_found = true;
> +
>  		mutex_lock(&fs_devices->device_list_mutex);
>  		device = btrfs_find_device(fs_devices, devid,
>  				disk_super->dev_item.uuid, NULL, false);
> -
> -		/*
> -		 * If this disk has been pulled into an fs devices created by
> -		 * a device which had the CHANGING_FSID_V2 flag then replace the
> -		 * metadata_uuid/fsid values of the fs_devices.
> -		 */
> -		if (has_metadata_uuid && fs_devices->fsid_change &&
> -		    found_transid > fs_devices->latest_generation) {
> -			memcpy(fs_devices->fsid, disk_super->fsid,
> -					BTRFS_FSID_SIZE);
> -			memcpy(fs_devices->metadata_uuid,
> -					disk_super->metadata_uuid, BTRFS_FSID_SIZE);
> -
> -			fs_devices->fsid_change = false;
> -		}
>  	}
>  
>  	if (!device) {
> @@ -912,6 +902,22 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>  		}
>  	}
>  
> +	/*
> +	 * If the new added disk has been pulled into an fs devices created by
> +	 * a device which had the CHANGING_FSID_V2 flag then replace the
> +	 * metadata_uuid/fsid values of the fs_devices.
> +	 */
> +	if (*new_device_added && fs_devices_found &&
> +	    has_metadata_uuid && fs_devices->fsid_change &&
> +	    found_transid > fs_devices->latest_generation) {
> +		memcpy(fs_devices->fsid, disk_super->fsid,
> +		       BTRFS_FSID_SIZE);
> +		memcpy(fs_devices->metadata_uuid,
> +		       disk_super->metadata_uuid, BTRFS_FSID_SIZE);
> +
> +		fs_devices->fsid_change = false;
> +	}
> +
>  	/*
>  	 * Unmount does not free the btrfs_device struct but would zero
>  	 * generation along with most of the other members. So just update
>