On Tue, Feb 06, 2024 at 04:21:15PM -0800, Boris Burkov wrote: > On Mon, Feb 05, 2024 at 06:43:39PM +0100, David Sterba wrote: > > --- a/fs/btrfs/volumes.c > > +++ b/fs/btrfs/volumes.c > > @@ -738,6 +738,7 @@ static noinline struct btrfs_device *device_list_add(const char *path, > > bool same_fsid_diff_dev = false; > > bool has_metadata_uuid = (btrfs_super_incompat_flags(disk_super) & > > BTRFS_FEATURE_INCOMPAT_METADATA_UUID); > > + bool can_create_new = *new_device_added; > > It took me quite a while to figure out all the intended logic with the > now in/out parameter. I think it's probably too cute? Why not just add > another parameter "can_create_new_device"? I think it feels kind of > weird on the caller side too, to set "new_device_added" to true, but > then still rely on it to actually get set to true. > > Once I got past this, the logic made sense, so definitely don't block > yourself on this nit. I had the separate parameter for the debugging version and removed for the final but I agree that it makes things less clear. Also there's something wrong with the device matching so the meaning of the parameter will be unchanged.
