On Mon, Dec 06, 2021 at 10:32:07PM +0800, Anand Jain wrote:

<snip>

quoted

quoted

  I got it. It shouldn't be difficult to reproduce and, I could reproduce.
Without this patch.


  Below is a device with two different paths. dm and its mapper.

----------
  $ ls -bli /dev/mapper/vg-scratch1  /dev/dm-1
  561 brw-rw---- 1 root disk 252, 1 Dec  3 12:13 /dev/dm-1
  565 lrwxrwxrwx 1 root root      7 Dec  3 12:13 /dev/mapper/vg-scratch1 ->
../dm-1
----------

  Clean the fs_devices.

----------
  $ btrfs dev scan --forget
----------

  Use the mapper to do mkfs.btrfs.

----------
  $ mkfs.btrfs -fq /dev/mapper/vg-scratch0
  $ mount /dev/mapper/vg-scratch0 /btrfs
----------

  Crete raid1 again using mapper path.

----------
  $ mkfs.btrfs -U $uuid -fq -draid1 -mraid1 /dev/mapper/vg-scratch1
/dev/mapper/vg-scratch2
----------

  Use dm path to add the device which belongs to another btrfs filesystem.

----------
  $ btrfs dev add -f /dev/dm-1 /btrfs
----------

  Now mount the above raid1 in degraded mode.

----------
  $ mount -o degraded /dev/mapper/vg-scratch2 /btrfs1
----------

Ahhh nice, I couldn't figure out a way to trigger it manually.  I wonder if we
can figure out a way to do this in xfstests without needing to have your
SCRATCH_DEV on lvm already?

Yep. A dm linear on top of a raw device will help. I have a rough draft
working. I could send it to xfstests if you want?

Yes please, that would be helpful.

quoted

quoted

quoted

Yeah I was a little fuzzy on this.  I think *any* failure should mean that we
remove the device from the fs_devices tho right?  So that we show we're missing
a device, since we can't actually access it?  I'm actually asking, because I
think we can go either way, but to me I think any failure sure result in the
removal of the device so we can re-scan the correct one.  Thanks,

It is difficult to generalize, I guess. For example, consider the transient
errors during the boot-up and the errors due to slow to-ready devices or the
system-related errors such as ENOMEM/EACCES, all these does not call for
device-free. If we free the device for transient errors, any further attempt
to mount will fail unless it is device-scan again.

Here the bug is about btrfs_free_stale_devices() which failed to identify
the same device when tricked by mixing the dm and mapper paths.
Can I check with you if there is another way to fix this by checking the
device major and min number or the serial number from the device inquiry
page?

quoted

I suppose I could just change it so that our verification proceses, like the
MAGIC or FSID checks, return ENODATA and we only do it in those cases.  Does
that seem reasonable?

The 'btrfs device add' calls btrfs_free_stale_devices(), however
device_path_matched() fails to match the device by its path. So IMO, fix has
to be in device_path_matched() but with a different parameter to match
instead of device path.

Here is another manifestation of the same problem.

$ mkfs.btrfs -fq /dev/dm-0
$ cat /proc/fs/btrfs/devlist | grep device:
 device: /dev/dm-0

$ btrfs dev scan --forget /dev/dm-0
ERROR: cannot unregister device '/dev/mapper/tsdb': No such file or
directory

Above, mkfs.btrfs does not sanitizes the input device path however, the
forget command sanitizes the input device to /dev/mapper/tsdb and the
device_path_matched() in btrfs_free_stale_devices() fails. So fix has to be
in device_path_matched() and, why not replace strcmp() with compare major
and minor device numbers?

I like that better actually, you mind wiring it up since it's your idea?
Thanks,

Josef