Any comment about this patch? On mon, 26 Nov 2012 16:51:36 +0800, Miao Xie wrote: > Without this patch, we can add a device into the R/O fs. > > Steps to reproduce: > # mkfs.btrfs -d single -m single <disk0> <disk1> > # mount -o ro <disk0> <mnt0> > # mount -o ro <disk0> <mnt1> > # mount -o remount,rw <mnt0> > # umount <mnt0> > # btrfs device add <disk1> <mnt1> > > It is because we just check the R/O flag of the super block object. > It is not enough, because the kernel may set the R/O flag only for > the mount point. We need invoke > > mnt_want_write_file() > > to do the full check. > > But, this interface is also used for the seed fs, so I split it to > two functions, one is used for the common fs, and the other is for > the seed fs. The detail fix method is following: > > - When we mount a seed fs, we doesn't return -EACCESS. Instead, we just > set R/O flag for the super block, and tell the user that "the fs is a > seed fs, we mount it on R/O state" by printk > - If the fs is not a seed fs, we invoke the common device add function, > which will call mnt_want_write_file() at first and get write access. > - If the fs is a seed fs, we will check the flags of the mount point. > - If it is R/O, it means the users mount the fs on R/O state on their > own initiative, in other words, the users don't want to change anything > include adding a new device. So in this case, we will return -EROFS. > - If the R/O flag of the mount point is not set, we will make a new fs which > is based on the seed fs and add the device into it. At the end of this > process, we will clear R/O flag of the super block, and then we can > access the new fs freely. > > In this way, > - We will forbid adding/removing any device to/from a filesystem which is > mounted on R/O state(mount -o ro), even it is a seed filesystem. In short, > the new process follows the read-only rule more strictly. > - We can modify the new filesystem which is based on a seed filesystem > immediately after it is created, needn't do remount/umount-mount. > > Signed-off-by: Miao Xie <[email protected]> > --- > fs/btrfs/ioctl.c | 81 > ++++++++++++++++++++++++++++++++++++++++++++++-------- > fs/btrfs/super.c | 5 ++-- > fs/btrfs/volumes.c | 26 ++++++------------ > fs/btrfs/volumes.h | 3 +- > 4 files changed, 83 insertions(+), 32 deletions(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index e6525d1..650d82d 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -2218,13 +2218,14 @@ out: > return ret; > } > > -static long btrfs_ioctl_add_dev(struct btrfs_root *root, void __user *arg) > +static int btrfs_common_add_device(struct file *file, struct btrfs_root > *root, > + const char *devname) > { > - struct btrfs_ioctl_vol_args *vol_args; > int ret; > > - if (!capable(CAP_SYS_ADMIN)) > - return -EPERM; > + ret = mnt_want_write_file(file); > + if (ret) > + return ret; > > mutex_lock(&root->fs_info->volume_mutex); > if (root->fs_info->balance_ctl) { > @@ -2233,18 +2234,76 @@ static long btrfs_ioctl_add_dev(struct btrfs_root > *root, void __user *arg) > goto out; > } > > - vol_args = memdup_user(arg, sizeof(*vol_args)); > - if (IS_ERR(vol_args)) { > - ret = PTR_ERR(vol_args); > + ret = btrfs_init_new_device(root, devname, false); > +out: > + mutex_unlock(&root->fs_info->volume_mutex); > + return ret; > +} > + > +static int btrfs_seed_add_device(struct file *file, struct btrfs_root *root, > + const char *devname) > +{ > + struct super_block *sb = root->fs_info->sb; > + int ret; > + > + sb_start_write(sb); > + > + mutex_lock(&root->fs_info->volume_mutex); > + /* > + * Some one may add a new device into a seed fs, and make a > + * new fs, so let's add the device by the common method. > + */ > + if (!root->fs_info->fs_devices->seeding) { > + mutex_unlock(&root->fs_info->volume_mutex); > + sb_end_write(sb); > + return btrfs_common_add_device(file, root, devname); > + } > + > + down_write(&sb->s_umount); > + if (file->f_path.mnt->mnt_flags & MNT_READONLY) { > + ret = -EROFS; > goto out; > } > > - vol_args->name[BTRFS_PATH_NAME_MAX] = '\0'; > - ret = btrfs_init_new_device(root, vol_args->name); > + if (root->fs_info->balance_ctl) { > + printk(KERN_INFO "btrfs: balance in progress\n"); > + ret = -EINVAL; > + goto out; > + } > > - kfree(vol_args); > + ret = btrfs_init_new_device(root, devname, true); > + if (!ret) { > + sb->s_flags &= ~MS_RDONLY; > + printk(KERN_INFO "Btrfs created a new filesystem based on " > + "seed filesystem."); > + } > out: > + up_write(&sb->s_umount); > mutex_unlock(&root->fs_info->volume_mutex); > + sb_end_write(sb); > + return ret; > +} > + > +static long btrfs_ioctl_add_dev(struct file *file, void __user *arg) > +{ > + struct btrfs_root *root = BTRFS_I(fdentry(file)->d_inode)->root; > + struct btrfs_ioctl_vol_args *vol_args; > + int ret; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + vol_args = memdup_user(arg, sizeof(*vol_args)); > + if (IS_ERR(vol_args)) > + return PTR_ERR(vol_args); > + vol_args->name[BTRFS_PATH_NAME_MAX] = '\0'; > + > + if (root->fs_info->fs_state & BTRFS_SUPER_FLAG_SEEDING) > + ret = btrfs_seed_add_device(file, root, vol_args->name); > + else > + ret = btrfs_common_add_device(file, root, vol_args->name); > + > + kfree(vol_args); > return ret; > } > > @@ -3800,7 +3859,7 @@ long btrfs_ioctl(struct file *file, unsigned int > case BTRFS_IOC_RESIZE: > return btrfs_ioctl_resize(file, argp); > case BTRFS_IOC_ADD_DEV: > - return btrfs_ioctl_add_dev(root, argp); > + return btrfs_ioctl_add_dev(file, argp); > case BTRFS_IOC_RM_DEV: > return btrfs_ioctl_rm_dev(file, argp); > case BTRFS_IOC_FS_INFO: > diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c > index 915ac14..621fd3c 100644 > --- a/fs/btrfs/super.c > +++ b/fs/btrfs/super.c > @@ -1116,8 +1116,9 @@ static struct dentry *btrfs_mount(struct > file_system_type *fs_type, int flags, > goto error_fs_info; > > if (!(flags & MS_RDONLY) && fs_devices->rw_devices == 0) { > - error = -EACCES; > - goto error_close_devices; > + flags |= MS_RDONLY; > + printk(KERN_INFO "Btrfs detected seed filesystem, force it to " > + "be READONLY\n"); > } > > bdev = fs_devices->latest_bdev; > diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > index 0f5ebb7..66d6dd6 100644 > --- a/fs/btrfs/volumes.c > +++ b/fs/btrfs/volumes.c > @@ -1568,6 +1568,7 @@ static int btrfs_prepare_sprout(struct btrfs_root *root) > super_flags = btrfs_super_flags(disk_super) & > ~BTRFS_SUPER_FLAG_SEEDING; > btrfs_set_super_flags(disk_super, super_flags); > + root->fs_info->fs_state &= ~BTRFS_SUPER_FLAG_SEEDING; > > return 0; > } > @@ -1648,32 +1649,25 @@ error: > return ret; > } > > -int btrfs_init_new_device(struct btrfs_root *root, char *device_path) > +int btrfs_init_new_device(struct btrfs_root *root, const char *device_path, > + bool seed_fs) > { > struct request_queue *q; > struct btrfs_trans_handle *trans; > struct btrfs_device *device; > struct block_device *bdev; > struct list_head *devices; > - struct super_block *sb = root->fs_info->sb; > struct rcu_string *name; > u64 total_bytes; > - int seeding_dev = 0; > int ret = 0; > > - if ((sb->s_flags & MS_RDONLY) && !root->fs_info->fs_devices->seeding) > - return -EROFS; > - > bdev = blkdev_get_by_path(device_path, FMODE_WRITE | FMODE_EXCL, > root->fs_info->bdev_holder); > if (IS_ERR(bdev)) > return PTR_ERR(bdev); > > - if (root->fs_info->fs_devices->seeding) { > - seeding_dev = 1; > - down_write(&sb->s_umount); > + if (seed_fs) > mutex_lock(&uuid_mutex); > - } > > filemap_write_and_wait(bdev->bd_inode->i_mapping); > > @@ -1740,8 +1734,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char > *device_path) > device->mode = FMODE_EXCL; > set_blocksize(device->bdev, 4096); > > - if (seeding_dev) { > - sb->s_flags &= ~MS_RDONLY; > + if (seed_fs) { > ret = btrfs_prepare_sprout(root); > BUG_ON(ret); /* -ENOMEM */ > } > @@ -1776,7 +1769,7 @@ int btrfs_init_new_device(struct btrfs_root *root, char > *device_path) > total_bytes + 1); > mutex_unlock(&root->fs_info->fs_devices->device_list_mutex); > > - if (seeding_dev) { > + if (seed_fs) { > ret = init_first_rw_device(trans, root, device); > if (ret) { > btrfs_abort_transaction(trans, root, ret); > @@ -1806,9 +1799,8 @@ int btrfs_init_new_device(struct btrfs_root *root, char > *device_path) > btrfs_calc_num_tolerated_disk_barrier_failures(root->fs_info); > ret = btrfs_commit_transaction(trans, root); > > - if (seeding_dev) { > + if (seed_fs) { > mutex_unlock(&uuid_mutex); > - up_write(&sb->s_umount); > > if (ret) /* transaction commit */ > return ret; > @@ -1837,10 +1829,8 @@ error_trans: > kfree(device); > error: > blkdev_put(bdev, FMODE_EXCL); > - if (seeding_dev) { > + if (seed_fs) > mutex_unlock(&uuid_mutex); > - up_write(&sb->s_umount); > - } > return ret; > } > > diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h > index 53c06af..a76d598 100644 > --- a/fs/btrfs/volumes.h > +++ b/fs/btrfs/volumes.h > @@ -279,7 +279,8 @@ int btrfs_grow_device(struct btrfs_trans_handle *trans, > struct btrfs_device *btrfs_find_device(struct btrfs_root *root, u64 devid, > u8 *uuid, u8 *fsid); > int btrfs_shrink_device(struct btrfs_device *device, u64 new_size); > -int btrfs_init_new_device(struct btrfs_root *root, char *path); > +int btrfs_init_new_device(struct btrfs_root *root, const char *path, > + bool seed_fs); > int btrfs_balance(struct btrfs_balance_control *bctl, > struct btrfs_ioctl_balance_args *bargs); > int btrfs_resume_balance_async(struct btrfs_fs_info *fs_info); >
-- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
