Re: [PATCH 1/2] btrfs: use num_device to check for the last surviving seed device

From: Anand Jain <anand.jain@oracle.com>
To: dsterba@suse.cz, Nikolay Borisov <nborisov@suse.com>
Cc: linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 1/2] btrfs: use num_device to check for the last surviving seed device
Date: Fri, 3 Sep 2021 11:07:46 +0800	[thread overview]
Message-ID: <e741e14d-3ea0-5f55-5438-73379be181dd@oracle.com> (raw)
In-Reply-To: <20210902153128.GY3379@twin.jikos.cz>

On 02/09/2021 23:31, David Sterba wrote:
> On Wed, Sep 01, 2021 at 12:22:30PM +0300, Nikolay Borisov wrote:
>>
>>
>> On 1.09.21 г. 9:43, Anand Jain wrote:
>>> For both sprout and seed fsids,
>>>   btrfs_fs_devices::num_devices provides device count including missing
>>>   btrfs_fs_devices::open_devices provides device count excluding missing
>>>
>>> We create a dummy struct btrfs_device for the missing device, so
>>> num_devices != open_devices when there is a missing device.
>>>
>>> In btrfs_rm_devices() we wrongly check for %cur_devices->open_devices
>>> before freeing the seed fs_devices. Instead we should check for
>>> %cur_devices->num_devices.
>>>
>>> Signed-off-by: Anand Jain <anand.jain@oracle.com>
>>
>> Is there a sequence of step that reproduce the problem?

Fundamentally per our design, we should have a struct btrfs_device even
for a missing device.

Here is an example where we break that rule after deleting a device in
a degraded seed device.

The below example needs boilerplate code here [1], to dump the struct
btrfs_fs_devices from the kernel.

[1] 
https://github.com/asj/bbp/blob/master/misc-next/0001-btrfs-boilerplate-procfs-devlist-and-fsinfo.patch

---------- cut ------
# Steps

# Create raid1 seed device and degrade it.
# Mount it with -o degraded.
# Add a RW device (sprout).
# Remount it so that it is writable now.
# Still we can't remove the seed device because it is degraded.
# So convert the raid1 chunks to single.
# Now remove the seed device (note: missing device is still there).
# At this stage the total devices should be 2 (one sprout and another
#  missing).

DEV1=/dev/vg/scratch0
DEV2=/dev/vg/scratch1
DEV3=/dev/vg/scratch2

local 
dfilter='fsid:|devid:|missing|total_devices|num_devices|open_devices|device:|MISSING|UUID'

         mkfs.btrfs -fq -draid1 -mraid1 $DEV1 $DEV2
         mount $DEV1 /btrfs
         xfs_io -f -c "pwrite 0 1M" /btrfs/foo
         umount /btrfs
         btrfstune -S1 $DEV1
         wipefs -a $DEV2
         btrfs dev scan --forget
         mount -o degraded $DEV1 /btrfs
         btrfs dev add -f $DEV3 /btrfs
         mount -o remount,rw $DEV3 /btrfs
         btrfs bal start --force -dconvert=single -mconvert=single /btrfs
         btrfs dev del $DEV1 /btrfs
	# dump btrfs_fs_devices.
         cat /proc/fs/btrfs/devlist  | egrep '$dfilter'
----------------

Before the patch:

total_devices = 2 but, we see only one device with devid 3.

[fsid: f3bd00d8-7be3-491c-a785-0d207a43b248]
	num_devices:		1
	open_devices:		1
	missing_devices:	0
	total_devices:		2
	[[UUID: 8c2d2dc6-7c0c-4a02-b63e-109131edd563]]
		device:		/dev/mapper/vg-scratch2  <-- DEV3
		devid:		3

After the patch:

total_devices = 2 and, we see both devid 3 and devid 2 (with the missing 
flag).

[fsid: 8542998a-64e8-4180-bb29-978403beb81c]
	num_devices:		1
	open_devices:		1
	missing_devices:	0
	total_devices:		2
	[[UUID: b0960a13-f956-4a0b-9ec2-b0da05060ec6]]
		device:		/dev/mapper/vg-scratch2  <-- DEV3
		devid:		3
	[[seed_fsid: 8dbae94f-f4c2-4875-9a88-b7db7847e740]]
		sprout_fsid:		8542998a-64e8-4180-bb29-978403beb81c
		num_devices:		1
		open_devices:		0
		missing_devices:	1
		total_devices:		1
		[[UUID: 9ade13a3-8264-4e41-908d-048dae3af370]]
			device:		(null)    <-- DEV1
			devid:		2
			dev_state:	|IN_FS_METADATA|MISSING|dev_stats_valid

Thanks, Anand