| Message ID | 8da8919da73bdaf0e652f03d59391e7710da6e5c.1606938211.git.josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup error handling in relocation | expand |
On 2020/12/3 上午3:50, Josef Bacik wrote: > btrfs_record_root_in_trans will return errors in the future, so handle > the error properly in create_subvol. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > fs/btrfs/ioctl.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 703212ff50a5..ad50e654ee64 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -714,7 +714,11 @@ static noinline int create_subvol(struct inode *dir, > /* Freeing will be done in btrfs_put_root() of new_root */ > anon_dev = 0; > > - btrfs_record_root_in_trans(trans, new_root); > + ret = btrfs_record_root_in_trans(trans, new_root); > + if (ret) { Dont' we need to call btrfs_put_root()? Or since we're going to abort transaction anyway, it doesn't matter that much any more? Thanks, Qu > + btrfs_abort_transaction(trans, ret); > + goto fail; > + } > > ret = btrfs_create_subvol_root(trans, new_root, root, new_dirid); > btrfs_put_root(new_root); >
On 12/2/20 9:43 PM, Qu Wenruo wrote: > > > On 2020/12/3 上午3:50, Josef Bacik wrote: >> btrfs_record_root_in_trans will return errors in the future, so handle >> the error properly in create_subvol. >> >> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >> --- >> fs/btrfs/ioctl.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c >> index 703212ff50a5..ad50e654ee64 100644 >> --- a/fs/btrfs/ioctl.c >> +++ b/fs/btrfs/ioctl.c >> @@ -714,7 +714,11 @@ static noinline int create_subvol(struct inode *dir, >> /* Freeing will be done in btrfs_put_root() of new_root */ >> anon_dev = 0; >> >> - btrfs_record_root_in_trans(trans, new_root); >> + ret = btrfs_record_root_in_trans(trans, new_root); >> + if (ret) { > > Dont' we need to call btrfs_put_root()? Or since we're going to abort > transaction anyway, it doesn't matter that much any more? > Nope you're right, and in fact it's a little broken without this patch as well, I'll fix the existing brokenness and fix this mistake as well. Good catch! Josef
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 703212ff50a5..ad50e654ee64 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -714,7 +714,11 @@ static noinline int create_subvol(struct inode *dir, /* Freeing will be done in btrfs_put_root() of new_root */ anon_dev = 0; - btrfs_record_root_in_trans(trans, new_root); + ret = btrfs_record_root_in_trans(trans, new_root); + if (ret) { + btrfs_abort_transaction(trans, ret); + goto fail; + } ret = btrfs_create_subvol_root(trans, new_root, root, new_dirid); btrfs_put_root(new_root);
btrfs_record_root_in_trans will return errors in the future, so handle the error properly in create_subvol. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- fs/btrfs/ioctl.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)