On Wed, Nov 14, 2018 at 11:15 AM Filipe Manana <fdman...@gmail.com> wrote:
>
> On Wed, Nov 14, 2018 at 9:14 AM Anand Jain <anand.j...@oracle.com> wrote:
> >
> > Support for a new command 'btrfs dev forget [dev]' is proposed here
> > to undo the effects of 'btrfs dev scan [dev]'. For this purpose
> > this patch proposes to use ioctl #5 as it was empty.
> > IOW(BTRFS_IOCTL_MAGIC, 5, ..)
> > This patch adds new ioctl BTRFS_IOC_FORGET_DEV which can be sent from
> > the /dev/btrfs-control to forget one or all devices, (devices which are
> > not mounted) from the btrfs kernel.
> >
> > The argument it takes is struct btrfs_ioctl_vol_args, and ::name can be
> > set to specify the device path. And all unmounted devices can be removed
> > from the kernel if no device path is provided.
> >
> > Again, the devices are removed only if the relevant fsid aren't mounted.
> >
> > This new cli can provide..
> > . Release of unwanted btrfs_fs_devices and btrfs_devices memory if the
> > device is not going to be mounted.
> > . Ability to mount the device in degraded mode when one of the other
> > device is corrupted like in split brain raid1.
> > . Running test cases which requires btrfs.ko-reload if the rootfs
> > is btrfs.
> >
> > Signed-off-by: Anand Jain <anand.j...@oracle.com>
> > Reviewed-by: Nikolay Borisov <nbori...@suse.com>
> > ---
> > fs/btrfs/super.c | 3 +++
> > fs/btrfs/volumes.c | 9 +++++++++
> > fs/btrfs/volumes.h | 1 +
> > include/uapi/linux/btrfs.h | 2 ++
> > 4 files changed, 15 insertions(+)
> >
> > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> > index 345c64d810d4..f99db6899004 100644
> > --- a/fs/btrfs/super.c
> > +++ b/fs/btrfs/super.c
> > @@ -2246,6 +2246,9 @@ static long btrfs_control_ioctl(struct file *file,
> > unsigned int cmd,
> > ret = PTR_ERR_OR_ZERO(device);
> > mutex_unlock(&uuid_mutex);
> > break;
> > + case BTRFS_IOC_FORGET_DEV:
> > + ret = btrfs_forget_devices(vol->name);
> > + break;
> > case BTRFS_IOC_DEVICES_READY:
> > mutex_lock(&uuid_mutex);
> > device = btrfs_scan_one_device(vol->name, FMODE_READ,
> > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> > index f435d397019e..e1365a122657 100644
> > --- a/fs/btrfs/volumes.c
> > +++ b/fs/btrfs/volumes.c
> > @@ -1208,6 +1208,15 @@ static int btrfs_read_disk_super(struct block_device
> > *bdev, u64 bytenr,
> > return 0;
> > }
> >
> > +int btrfs_forget_devices(const char *path)
> > +{
> > + mutex_lock(&uuid_mutex);
> > + btrfs_free_stale_devices(strlen(path) ? path:NULL, NULL);
>
> One space before : and another one after it please.
>
> Now the more important: don't use strlen, use strnlen. Some malicious
> or sloppy user might have passed a non-null terminated string, you
> don't want strlen to go past the limits of btrfs_ioctl_vol_args for
> obvious reasons.
In fact that's a problem for the entire use of vol->name in
btrfs_control_ioctl. The name's last byte should be set to '\0' to
avoid issues.
I'll send a fix for that, so if David fixes the white spaces on commit
there's no need for a v12.
>
> Also, please, not just to make a maintainer's life easier, but current
> and future reviewers, add the patch version to each patch's subject
> and not just the cover letter. Also list (after ---) what changes
> between each patch version in the patch itself and not the cover
> letter.
>
> V12, here we go.
>
> > + mutex_unlock(&uuid_mutex);
> > +
> > + return 0;
> > +}
> > +
> > /*
> > * Look for a btrfs signature on a device. This may be called out of the
> > mount path
> > * and we are not allowed to call set_blocksize during the scan. The
> > superblock
> > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h
> > index aefce895e994..180297d04938 100644
> > --- a/fs/btrfs/volumes.h
> > +++ b/fs/btrfs/volumes.h
> > @@ -406,6 +406,7 @@ int btrfs_open_devices(struct btrfs_fs_devices
> > *fs_devices,
> > fmode_t flags, void *holder);
> > struct btrfs_device *btrfs_scan_one_device(const char *path,
> > fmode_t flags, void *holder);
> > +int btrfs_forget_devices(const char *path);
> > int btrfs_close_devices(struct btrfs_fs_devices *fs_devices);
> > void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int
> > step);
> > void btrfs_assign_next_active_device(struct btrfs_device *device,
> > diff --git a/include/uapi/linux/btrfs.h b/include/uapi/linux/btrfs.h
> > index 5ca1d21fc4a7..b1be7f828cb4 100644
> > --- a/include/uapi/linux/btrfs.h
> > +++ b/include/uapi/linux/btrfs.h
> > @@ -836,6 +836,8 @@ enum btrfs_err_code {
> > struct btrfs_ioctl_vol_args)
> > #define BTRFS_IOC_SCAN_DEV _IOW(BTRFS_IOCTL_MAGIC, 4, \
> > struct btrfs_ioctl_vol_args)
> > +#define BTRFS_IOC_FORGET_DEV _IOW(BTRFS_IOCTL_MAGIC, 5, \
> > + struct btrfs_ioctl_vol_args)
> > /* trans start and trans end are dangerous, and only for
> > * use by applications that know how to avoid the
> > * resulting deadlocks
> > --
> > 1.8.3.1
> >
>
>
> --
> Filipe David Manana,
>
> “Whether you think you can, or you think you can't — you're right.”
--
Filipe David Manana,
“Whether you think you can, or you think you can't — you're right.”
