Re: [PATCH 3/3] btrfs: free alien device due to device add

[David Sterba <dsterba@suse.cz>]

On Sat, May 02, 2020 at 04:01:28AM +0800, Anand Jain wrote:
>   Ah. I didn't notice that. Will fix.
> 
> > As there's NULL passed, 
> 
>   NULL is passed to the 2nd argument skip_device
> 
> > all stale devices will be removed from the list,
> 
>   No, It means it does not have any particular device to skip.
>   Added device is already part of mounted fs_device list,
>   the loop skips its check. So no need to skip_device.
> 
> > but we can remove just the device being added, no?
> 
>   It does exactly that.
>   btrfs_free_stale_devices(device_path, NULL);
> 
>   It removes the device from all other fs_devices which are _unmounted_.

Right, I got it wrong.

> > And before the whole
> > operation starts, not after. 
> 
>   What if the add fails? Then we have to add scanned device back to avoid
>   that mess. why not remove after we have successfully add the device
>   to the mounted fsid.

As there's no synchronization, there will be either need for a rollback
action or stale information after the change is permanent (device added
to new filesystem) but no fs_devices update happens.

The difference is that when the device is removed first, the rollback
happens only in exceptional cases, when the commit fails.

OTOH the time window between commit and removal happens each time. The
ohly hope here is that the window is really short so there's practically
zero chance of another process to jump in trying to use the device.