diff mbox series

[5/6] btrfs: copy fsid and metadata_uuid for pulled disk without INCOMPAT_METADATA_UUID

Message ID 20191212110132.11063-6-Damenly_Su@gmx.com (mailing list archive)
State New, archived
Headers show
Series btrfs: metadata uuid fixes and enhancements | expand

Commit Message

Su Yue Dec. 12, 2019, 11:01 a.m. UTC 
From: Su Yue <Damenly_Su@gmx.com>

Since a scanned device may be the device pulled into disk without
metadata_uuid feature, there may already be changing devices there.
Here copy fsid and metadata_uuid for above case.

Signed-off-by: Su Yue <Damenly_Su@gmx.com>
---
 fs/btrfs/volumes.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Nikolay Borisov Jan. 6, 2020, 3:12 p.m. UTC | #1 
On 12.12.19 г. 13:01 ч., damenly.su@gmail.com wrote:
> From: Su Yue <Damenly_Su@gmx.com>
> 
> Since a scanned device may be the device pulled into disk without
> metadata_uuid feature, there may already be changing devices there.
> Here copy fsid and metadata_uuid for above case.
> 
> Signed-off-by: Su Yue <Damenly_Su@gmx.com>


What does this patch fix, why is it needed? It seems to be independent
of the split brain fixes?

> ---
>  fs/btrfs/volumes.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index faf9cdd14f33..b21ab45e76a0 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -964,13 +964,16 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>  	 * metadata_uuid/fsid values of the fs_devices.
>  	 */
>  	if (*new_device_added && fs_devices_found &&
> -	    has_metadata_uuid && fs_devices->fsid_change &&
> +	    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);
> -
> +		if (has_metadata_uuid)
> +			memcpy(fs_devices->metadata_uuid,
> +			       disk_super->metadata_uuid, BTRFS_FSID_SIZE);
> +		else
> +			memcpy(fs_devices->metadata_uuid,
> +			       disk_super->fsid, BTRFS_FSID_SIZE);
>  		fs_devices->fsid_change = false;
>  	}
>  
>
Su Yue Jan. 7, 2020, 1:31 a.m. UTC | #2 
On 2020/1/6 11:12 PM, Nikolay Borisov wrote:
>
>
> On 12.12.19 г. 13:01 ч., damenly.su@gmail.com wrote:
>> From: Su Yue <Damenly_Su@gmx.com>
>>
>> Since a scanned device may be the device pulled into disk without
>> metadata_uuid feature, there may already be changing devices there.
>> Here copy fsid and metadata_uuid for above case.
>>
>> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
>
>
> What does this patch fix, why is it needed? It seems to be independent
> of the split brain fixes?
>

Sorry for the messy and short commit log.
It's one of the split brain fixes.

As mails I replied you earlier, the case
is for device which succeed to sync in
the second transaction and is without
metadata_uuid feature. If there is fs_devices
already scanned, the device's fsid instead of
metadata_uuid(NULL here) should be copied into
the fs_devices->metada_uuid field.

Thanks.

>> ---
>>   fs/btrfs/volumes.c | 11 +++++++----
>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index faf9cdd14f33..b21ab45e76a0 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -964,13 +964,16 @@ static noinline struct btrfs_device *device_list_add(const char *path,
>>   	 * metadata_uuid/fsid values of the fs_devices.
>>   	 */
>>   	if (*new_device_added && fs_devices_found &&
>> -	    has_metadata_uuid && fs_devices->fsid_change &&
>> +	    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);
>> -
>> +		if (has_metadata_uuid)
>> +			memcpy(fs_devices->metadata_uuid,
>> +			       disk_super->metadata_uuid, BTRFS_FSID_SIZE);
>> +		else
>> +			memcpy(fs_devices->metadata_uuid,
>> +			       disk_super->fsid, BTRFS_FSID_SIZE);
>>   		fs_devices->fsid_change = false;
>>   	}
>>
>>
Nikolay Borisov Jan. 7, 2020, 7:18 a.m. UTC | #3 
On 7.01.20 г. 3:31 ч., Su Yue wrote:
> On 2020/1/6 11:12 PM, Nikolay Borisov wrote:
>>
>>
>> On 12.12.19 г. 13:01 ч., damenly.su@gmail.com wrote:
>>> From: Su Yue <Damenly_Su@gmx.com>
>>>
>>> Since a scanned device may be the device pulled into disk without
>>> metadata_uuid feature, there may already be changing devices there.
>>> Here copy fsid and metadata_uuid for above case.
>>>
>>> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
>>
>>
>> What does this patch fix, why is it needed? It seems to be independent
>> of the split brain fixes?
>>
> 
> Sorry for the messy and short commit log.
> It's one of the split brain fixes.
> 
> As mails I replied you earlier, the case
> is for device which succeed to sync in
> the second transaction and is without
> metadata_uuid feature. If there is fs_devices
> already scanned, the device's fsid instead of
> metadata_uuid(NULL here) should be copied into
> the fs_devices->metada_uuid field.

I figures as much as I started tackling the problem. So this must be
part of the patch which fixes aforementioned split brain scenario. I
have already developed some patches + tests. So will be sending those,
since they are very similar to what you posted originally I will retain
your SOB line.

> 
> Thanks.
> 
>>> ---
>>>   fs/btrfs/volumes.c | 11 +++++++----
>>>   1 file changed, 7 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index faf9cdd14f33..b21ab45e76a0 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -964,13 +964,16 @@ static noinline struct btrfs_device
>>> *device_list_add(const char *path,
>>>        * metadata_uuid/fsid values of the fs_devices.
>>>        */
>>>       if (*new_device_added && fs_devices_found &&
>>> -        has_metadata_uuid && fs_devices->fsid_change &&
>>> +        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);
>>> -
>>> +        if (has_metadata_uuid)
>>> +            memcpy(fs_devices->metadata_uuid,
>>> +                   disk_super->metadata_uuid, BTRFS_FSID_SIZE);
>>> +        else
>>> +            memcpy(fs_devices->metadata_uuid,
>>> +                   disk_super->fsid, BTRFS_FSID_SIZE);
>>>           fs_devices->fsid_change = false;
>>>       }
>>>
>>>
>
Su Yue Jan. 7, 2020, 7:34 a.m. UTC | #4 
On 2020/1/7 3:18 PM, Nikolay Borisov wrote:
>
>
> On 7.01.20 г. 3:31 ч., Su Yue wrote:
>> On 2020/1/6 11:12 PM, Nikolay Borisov wrote:
>>>
>>>
>>> On 12.12.19 г. 13:01 ч., damenly.su@gmail.com wrote:
>>>> From: Su Yue <Damenly_Su@gmx.com>
>>>>
>>>> Since a scanned device may be the device pulled into disk without
>>>> metadata_uuid feature, there may already be changing devices there.
>>>> Here copy fsid and metadata_uuid for above case.
>>>>
>>>> Signed-off-by: Su Yue <Damenly_Su@gmx.com>
>>>
>>>
>>> What does this patch fix, why is it needed? It seems to be independent
>>> of the split brain fixes?
>>>
>>
>> Sorry for the messy and short commit log.
>> It's one of the split brain fixes.
>>
>> As mails I replied you earlier, the case
>> is for device which succeed to sync in
>> the second transaction and is without
>> metadata_uuid feature. If there is fs_devices
>> already scanned, the device's fsid instead of
>> metadata_uuid(NULL here) should be copied into
>> the fs_devices->metada_uuid field.
>
> I figures as much as I started tackling the problem. So this must be
> part of the patch which fixes aforementioned split brain scenario. I
> have already developed some patches + tests. So will be sending those,
> since they are very similar to what you posted originally I will retain
> your SOB line.
>

I'm okay about this way. Thanks.
>>
>> Thanks.
>>
>>>> ---
>>>>    fs/btrfs/volumes.c | 11 +++++++----
>>>>    1 file changed, 7 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>>> index faf9cdd14f33..b21ab45e76a0 100644
>>>> --- a/fs/btrfs/volumes.c
>>>> +++ b/fs/btrfs/volumes.c
>>>> @@ -964,13 +964,16 @@ static noinline struct btrfs_device
>>>> *device_list_add(const char *path,
>>>>         * metadata_uuid/fsid values of the fs_devices.
>>>>         */
>>>>        if (*new_device_added && fs_devices_found &&
>>>> -        has_metadata_uuid && fs_devices->fsid_change &&
>>>> +        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);
>>>> -
>>>> +        if (has_metadata_uuid)
>>>> +            memcpy(fs_devices->metadata_uuid,
>>>> +                   disk_super->metadata_uuid, BTRFS_FSID_SIZE);
>>>> +        else
>>>> +            memcpy(fs_devices->metadata_uuid,
>>>> +                   disk_super->fsid, BTRFS_FSID_SIZE);
>>>>            fs_devices->fsid_change = false;
>>>>        }
>>>>
>>>>
>>
diff mbox series

Patch

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index faf9cdd14f33..b21ab45e76a0 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -964,13 +964,16 @@  static noinline struct btrfs_device *device_list_add(const char *path,
 	 * metadata_uuid/fsid values of the fs_devices.
 	 */
 	if (*new_device_added && fs_devices_found &&
-	    has_metadata_uuid && fs_devices->fsid_change &&
+	    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);
-
+		if (has_metadata_uuid)
+			memcpy(fs_devices->metadata_uuid,
+			       disk_super->metadata_uuid, BTRFS_FSID_SIZE);
+		else
+			memcpy(fs_devices->metadata_uuid,
+			       disk_super->fsid, BTRFS_FSID_SIZE);
 		fs_devices->fsid_change = false;
 	}