| Message ID | a20a37162b49e5b1de7a5ec70607102b61ac94eb.1604649817.git.anand.jain@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cleanup btrfs_free_extra_devids() drop arg step | expand |
On 11/6/20 3:06 AM, Anand Jain wrote: > Following patch > btrfs: dev-replace: fail mount if we don't have replace item with target device > dropped the multi ops of the function btrfs_free_extra_devids(), where now This is jumbled together so I was confused, write it like The following patch btrfs: dev-replace: fail mount if we don't have replace item with target device dropped the usage of the step argument in btrfs_free_extra_devids() Other than that the code is fine, you can fix that up and add Reviewed-by: Josef Bacik <josef@toxicpanda.com> Thanks, Josef
On 11/6/20 3:06 AM, Anand Jain wrote: > Following patch > btrfs: dev-replace: fail mount if we don't have replace item with target device > dropped the multi ops of the function btrfs_free_extra_devids(), where now > it doesn't check the replace target device. So btrfs_free_extra_devid() > doesn't need the 2nd argument %step anymore. Perpetuate the related > changes back to the functions in the stack. > > Signed-off-by: Anand Jain <anand.jain@oracle.com> Actually nm, I forgot to build, this thing doesn't compile. Thanks, Josef
On 7/11/20 1:02 am, Josef Bacik wrote: > On 11/6/20 3:06 AM, Anand Jain wrote: >> Following patch >> btrfs: dev-replace: fail mount if we don't have replace item with >> target device >> dropped the multi ops of the function btrfs_free_extra_devids(), where >> now >> it doesn't check the replace target device. So btrfs_free_extra_devid() >> doesn't need the 2nd argument %step anymore. Perpetuate the related >> changes back to the functions in the stack. >> >> Signed-off-by: Anand Jain <anand.jain@oracle.com> > > Actually nm, I forgot to build, this thing doesn't compile. Thanks, The compile fail is not real to this patch. It happened due change in patch's order in the misc-next. In specific, the compile fail happens due to the dependent patch as below, btrfs: fix btrfs_find_device unused arg seed which I fixed locally as below. As David is reviewing the above patch, I didn't want to send another revision and confuse him more. To compile successfully, pls, after the above patch apply for the following changes, ----------------------------------------------- diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c index f6e1a4b0ae84..dc3481014f16 100644 --- a/fs/btrfs/dev-replace.c +++ b/fs/btrfs/dev-replace.c @@ -96,7 +96,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) * a replace target, fail the mount. */ if (btrfs_find_device(fs_info->fs_devices, - BTRFS_DEV_REPLACE_DEVID, NULL, NULL, false)) { + BTRFS_DEV_REPLACE_DEVID, NULL, NULL)) { btrfs_err(fs_info, "found replace target device without a valid replace item"); ret = -EUCLEAN; @@ -159,7 +159,7 @@ int btrfs_init_dev_replace(struct btrfs_fs_info *fs_info) * replace target, fail the mount. */ if (btrfs_find_device(fs_info->fs_devices, - BTRFS_DEV_REPLACE_DEVID, NULL, NULL, false)) { + BTRFS_DEV_REPLACE_DEVID, NULL, NULL)) { btrfs_err(fs_info, "replace devid present without an active replace item"); ret = -EUCLEAN; ------------------------- When this patch was sent, it was tested with fstests btrfs and finds no new regression. Thanks, Anand > > Josef
On Sat, Nov 07, 2020 at 07:41:01AM +0800, Anand Jain wrote: > > > On 7/11/20 1:02 am, Josef Bacik wrote: > > On 11/6/20 3:06 AM, Anand Jain wrote: > >> Following patch > >> btrfs: dev-replace: fail mount if we don't have replace item with > >> target device > >> dropped the multi ops of the function btrfs_free_extra_devids(), where > >> now > >> it doesn't check the replace target device. So btrfs_free_extra_devid() > >> doesn't need the 2nd argument %step anymore. Perpetuate the related > >> changes back to the functions in the stack. > >> > >> Signed-off-by: Anand Jain <anand.jain@oracle.com> > > > > Actually nm, I forgot to build, this thing doesn't compile. Thanks, > > The compile fail is not real to this patch. It happened due change in > patch's order in the misc-next. In specific, the compile fail happens > due to the dependent patch as below, > > btrfs: fix btrfs_find_device unused arg seed > > which I fixed locally as below. As David is reviewing the above patch, > I didn't want to send another revision and confuse him more. To compile > successfully, pls, after the above patch apply for the following > changes, With so many cleanup patches floating around compilation failure due to some dependency is not a problem. I'll notice and if there's something beyond trivial to fix it I'd let you know and ask for a resend. The patch "btrfs: dev-replace: fail mount if we don't have replace item with" got merged to master meanwhile so I've updated the reference to also include the commit id. With that the patch is now in misc-next, thanks.
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 212806d59012..3b07cce1937e 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -3156,11 +3156,13 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device } /* - * Keep the devid that is marked to be the target device for the - * device replace procedure + * At this point we know all the devices that make this FS, including + * the seed devices but we don't know yet if the replace target is + * required. So free devices not part of this FS but skip the replace + * traget device if any. And the replace target is checked further + * below in btrfs_init_dev_replace(). */ - btrfs_free_extra_devids(fs_devices, 0); - + btrfs_free_extra_devids(fs_devices); if (!fs_devices->latest_bdev) { btrfs_err(fs_info, "failed to read devices"); goto fail_tree_roots; @@ -3207,8 +3209,6 @@ int __cold open_ctree(struct super_block *sb, struct btrfs_fs_devices *fs_device goto fail_block_groups; } - btrfs_free_extra_devids(fs_devices, 1); - ret = btrfs_sysfs_add_fsid(fs_devices); if (ret) { btrfs_err(fs_info, "failed to init sysfs fsid interface: %d", diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c index 9dd55a6f38de..b08f232170ee 100644 --- a/fs/btrfs/volumes.c +++ b/fs/btrfs/volumes.c @@ -1038,7 +1038,7 @@ static struct btrfs_fs_devices *clone_fs_devices(struct btrfs_fs_devices *orig) } static void __btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, - int step, struct btrfs_device **latest_dev) + struct btrfs_device **latest_dev) { struct btrfs_device *device, *next; @@ -1083,16 +1083,16 @@ static void __btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, * After we have read the system tree and know devids belonging to this * filesystem, remove the device which does not belong there. */ -void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step) +void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices) { struct btrfs_device *latest_dev = NULL; struct btrfs_fs_devices *seed_dev; mutex_lock(&uuid_mutex); - __btrfs_free_extra_devids(fs_devices, step, &latest_dev); + __btrfs_free_extra_devids(fs_devices, &latest_dev); list_for_each_entry(seed_dev, &fs_devices->seed_list, seed_list) - __btrfs_free_extra_devids(seed_dev, step, &latest_dev); + __btrfs_free_extra_devids(seed_dev, &latest_dev); fs_devices->latest_bdev = latest_dev->bdev; diff --git a/fs/btrfs/volumes.h b/fs/btrfs/volumes.h index bd95cea85a65..5e08274d1252 100644 --- a/fs/btrfs/volumes.h +++ b/fs/btrfs/volumes.h @@ -449,7 +449,7 @@ struct btrfs_device *btrfs_scan_one_device(const char *path, fmode_t flags, void *holder); int btrfs_forget_devices(const char *path); void btrfs_close_devices(struct btrfs_fs_devices *fs_devices); -void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices, int step); +void btrfs_free_extra_devids(struct btrfs_fs_devices *fs_devices); void btrfs_assign_next_active_device(struct btrfs_device *device, struct btrfs_device *this_dev); struct btrfs_device *btrfs_find_device_by_devspec(struct btrfs_fs_info *fs_info,
Following patch btrfs: dev-replace: fail mount if we don't have replace item with target device dropped the multi ops of the function btrfs_free_extra_devids(), where now it doesn't check the replace target device. So btrfs_free_extra_devid() doesn't need the 2nd argument %step anymore. Perpetuate the related changes back to the functions in the stack. Signed-off-by: Anand Jain <anand.jain@oracle.com> --- fs/btrfs/disk-io.c | 12 ++++++------ fs/btrfs/volumes.c | 8 ++++---- fs/btrfs/volumes.h | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-)