On Mon, Sep 10, 2018 at 07:47:29PM +0200, David Sterba wrote: > On Thu, Sep 06, 2018 at 03:52:17PM -0400, je...@suse.com wrote: > > From: Jeff Mahoney <je...@suse.com> > > > > When we fail to start a transaction in btrfs_dev_replace_start, > > we leave dev_replace->replace_start set to STARTED but clear > > ->srcdev and ->tgtdev. Later, that can result in an Oops in > > btrfs_dev_replace_progress when having state set to STARTED or > > SUSPENDED implies that ->srcdev is valid. > > > > Also fix error handling when the state is already STARTED or > > SUSPENDED while starting. That, too, will clear ->srcdev and ->tgtdev > > even though it doesn't own them. This should be an impossible case to > > hit since we should be protected by the BTRFS_FS_EXCL_OP bit being set. > > Let's add an ASSERT there while we're at it. > > > > Fixes: e93c89c1aaaaa (Btrfs: add new sources for device replace code) > > Signed-off-by: Jeff Mahoney <je...@suse.com> > > --- > > fs/btrfs/dev-replace.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/fs/btrfs/dev-replace.c b/fs/btrfs/dev-replace.c > > index e2ba0419297a..0581c8570a05 100644 > > --- a/fs/btrfs/dev-replace.c > > +++ b/fs/btrfs/dev-replace.c > > @@ -445,6 +445,7 @@ int btrfs_dev_replace_start(struct btrfs_fs_info > > *fs_info, > > break; > > case BTRFS_IOCTL_DEV_REPLACE_STATE_STARTED: > > case BTRFS_IOCTL_DEV_REPLACE_STATE_SUSPENDED: > > + ASSERT(0); > > I don't like it fixed that way. The asserts get compiled in > conditionally, so the behaviour might be differend dending on the > config. If the cases are unreachable, then the two cases can be replaced > by a default: and return EINVAL.
As this is a minor issue and a matter of cleanup, I'm going to merge the patch as-is because it's a bugfix.
