Check return value of ext3_journal_get_write_access() and
ext3_journal_dirty_metadata().
Signed-off-by: Namhyung Kim <[email protected]>
---
fs/ext3/namei.c | 14 ++++++++++++--
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
index 03fccc5..672cea1 100644
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -1644,8 +1644,13 @@ static int ext3_delete_entry (handle_t *handle,
if (!ext3_check_dir_entry("ext3_delete_entry", dir, de, bh, i))
return -EIO;
if (de == de_del) {
+ int err;
+
BUFFER_TRACE(bh, "get_write_access");
- ext3_journal_get_write_access(handle, bh);
+ err = ext3_journal_get_write_access(handle, bh);
+ if (err)
+ goto journal_error;
+
if (pde)
pde->rec_len = ext3_rec_len_to_disk(
ext3_rec_len_from_disk(pde->rec_len) +
@@ -1654,7 +1659,12 @@ static int ext3_delete_entry (handle_t *handle,
de->inode = 0;
dir->i_version++;
BUFFER_TRACE(bh, "call ext3_journal_dirty_metadata");
- ext3_journal_dirty_metadata(handle, bh);
+ err = ext3_journal_dirty_metadata(handle, bh);
+ if (err) {
+journal_error:
+ ext3_std_error(dir->i_sb, err);
+ return err;
+ }
return 0;
}
i += ext3_rec_len_from_disk(de->rec_len);
--
1.7.0.4
Check return value of ext3_journal_get_write_access() and
ext3_journal_dirty_metadata(). 'new_bh' should be kept in
order to get revoked in case of journal error in dir_bh.
Signed-off-by: Namhyung Kim <[email protected]>
---
fs/ext3/namei.c | 27 +++++++++++++++++++++------
1 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
index 672cea1..8061281 100644
--- a/fs/ext3/namei.c
+++ b/fs/ext3/namei.c
@@ -2371,7 +2371,9 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
goto end_rename;
} else {
BUFFER_TRACE(new_bh, "get write access");
- ext3_journal_get_write_access(handle, new_bh);
+ retval = ext3_journal_get_write_access(handle, new_bh);
+ if (retval)
+ goto journal_error1;
new_de->inode = cpu_to_le32(old_inode->i_ino);
if (EXT3_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
EXT3_FEATURE_INCOMPAT_FILETYPE))
@@ -2380,9 +2382,12 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
new_dir->i_ctime = new_dir->i_mtime = CURRENT_TIME_SEC;
ext3_mark_inode_dirty(handle, new_dir);
BUFFER_TRACE(new_bh, "call ext3_journal_dirty_metadata");
- ext3_journal_dirty_metadata(handle, new_bh);
- brelse(new_bh);
- new_bh = NULL;
+ retval = ext3_journal_dirty_metadata(handle, new_bh);
+ if (retval) {
+journal_error1:
+ ext3_std_error(new_dir->i_sb, retval);
+ goto end_rename;
+ }
}
/*
@@ -2429,10 +2434,20 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
ext3_update_dx_flag(old_dir);
if (dir_bh) {
BUFFER_TRACE(dir_bh, "get_write_access");
- ext3_journal_get_write_access(handle, dir_bh);
+ retval = ext3_journal_get_write_access(handle, dir_bh);
+ if (retval)
+ goto journal_error2;
PARENT_INO(dir_bh->b_data) = cpu_to_le32(new_dir->i_ino);
BUFFER_TRACE(dir_bh, "call ext3_journal_dirty_metadata");
- ext3_journal_dirty_metadata(handle, dir_bh);
+ retval = ext3_journal_dirty_metadata(handle, dir_bh);
+ if (retval) {
+journal_error2:
+ if (new_bh)
+ ext3_journal_revoke(handle, new_bh->b_blocknr,
+ new_bh);
+ ext3_std_error(new_dir->i_sb, retval);
+ goto end_rename;
+ }
drop_nlink(old_dir);
if (new_inode) {
drop_nlink(new_inode);
--
1.7.0.4
On Fri 19-11-10 16:28:35, Namhyung Kim wrote:
> Check return value of ext3_journal_get_write_access() and
> ext3_journal_dirty_metadata().
Thanks, merged.
Honza
>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> fs/ext3/namei.c | 14 ++++++++++++--
> 1 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index 03fccc5..672cea1 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -1644,8 +1644,13 @@ static int ext3_delete_entry (handle_t *handle,
> if (!ext3_check_dir_entry("ext3_delete_entry", dir, de, bh, i))
> return -EIO;
> if (de == de_del) {
> + int err;
> +
> BUFFER_TRACE(bh, "get_write_access");
> - ext3_journal_get_write_access(handle, bh);
> + err = ext3_journal_get_write_access(handle, bh);
> + if (err)
> + goto journal_error;
> +
> if (pde)
> pde->rec_len = ext3_rec_len_to_disk(
> ext3_rec_len_from_disk(pde->rec_len) +
> @@ -1654,7 +1659,12 @@ static int ext3_delete_entry (handle_t *handle,
> de->inode = 0;
> dir->i_version++;
> BUFFER_TRACE(bh, "call ext3_journal_dirty_metadata");
> - ext3_journal_dirty_metadata(handle, bh);
> + err = ext3_journal_dirty_metadata(handle, bh);
> + if (err) {
> +journal_error:
> + ext3_std_error(dir->i_sb, err);
> + return err;
> + }
> return 0;
> }
> i += ext3_rec_len_from_disk(de->rec_len);
> --
> 1.7.0.4
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
On Fri 19-11-10 16:28:36, Namhyung Kim wrote:
> Check return value of ext3_journal_get_write_access() and
> ext3_journal_dirty_metadata(). 'new_bh' should be kept in
> order to get revoked in case of journal error in dir_bh.
>
> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> fs/ext3/namei.c | 27 +++++++++++++++++++++------
> 1 files changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/fs/ext3/namei.c b/fs/ext3/namei.c
> index 672cea1..8061281 100644
> --- a/fs/ext3/namei.c
> +++ b/fs/ext3/namei.c
> @@ -2371,7 +2371,9 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
> goto end_rename;
> } else {
> BUFFER_TRACE(new_bh, "get write access");
> - ext3_journal_get_write_access(handle, new_bh);
> + retval = ext3_journal_get_write_access(handle, new_bh);
> + if (retval)
> + goto journal_error1;
> new_de->inode = cpu_to_le32(old_inode->i_ino);
> if (EXT3_HAS_INCOMPAT_FEATURE(new_dir->i_sb,
> EXT3_FEATURE_INCOMPAT_FILETYPE))
> @@ -2380,9 +2382,12 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
> new_dir->i_ctime = new_dir->i_mtime = CURRENT_TIME_SEC;
> ext3_mark_inode_dirty(handle, new_dir);
> BUFFER_TRACE(new_bh, "call ext3_journal_dirty_metadata");
> - ext3_journal_dirty_metadata(handle, new_bh);
> - brelse(new_bh);
> - new_bh = NULL;
> + retval = ext3_journal_dirty_metadata(handle, new_bh);
> + if (retval) {
> +journal_error1:
> + ext3_std_error(new_dir->i_sb, retval);
> + goto end_rename;
> + }
> }
>
> /*
> @@ -2429,10 +2434,20 @@ static int ext3_rename (struct inode * old_dir, struct dentry *old_dentry,
> ext3_update_dx_flag(old_dir);
> if (dir_bh) {
> BUFFER_TRACE(dir_bh, "get_write_access");
> - ext3_journal_get_write_access(handle, dir_bh);
> + retval = ext3_journal_get_write_access(handle, dir_bh);
> + if (retval)
> + goto journal_error2;
> PARENT_INO(dir_bh->b_data) = cpu_to_le32(new_dir->i_ino);
> BUFFER_TRACE(dir_bh, "call ext3_journal_dirty_metadata");
> - ext3_journal_dirty_metadata(handle, dir_bh);
> + retval = ext3_journal_dirty_metadata(handle, dir_bh);
> + if (retval) {
> +journal_error2:
> + if (new_bh)
> + ext3_journal_revoke(handle, new_bh->b_blocknr,
> + new_bh);
Uhuh, why ext3_journal_revoke()? I expect you want to cancel the changes
you possibly did to new_bh. ext3_journal_forget() is for that but even that
doesn't necessarily do what you want because it could cancel also changes
some unrelated operation did to the buffer. So the only way to really undo
the change is to set new_de->inode and new_de->file_type to original
values. Also since we already unlinked the inode from the old directory,
I'm not sure it's even beneficial to undo linking it to the new one. So I'd
just bail out as fast as we can and leave on fsck to handle the mess...
Honza
> + ext3_std_error(new_dir->i_sb, retval);
> + goto end_rename;
> + }
> drop_nlink(old_dir);
> if (new_inode) {
> drop_nlink(new_inode);
> --
> 1.7.0.4
>
--
Jan Kara <[email protected]>
SUSE Labs, CR
2010-11-22 (월), 19:05 +0100, Jan Kara:
> Uhuh, why ext3_journal_revoke()? I expect you want to cancel the changes
> you possibly did to new_bh. ext3_journal_forget() is for that but even that
> doesn't necessarily do what you want because it could cancel also changes
> some unrelated operation did to the buffer. So the only way to really undo
> the change is to set new_de->inode and new_de->file_type to original
> values. Also since we already unlinked the inode from the old directory,
> I'm not sure it's even beneficial to undo linking it to the new one. So I'd
> just bail out as fast as we can and leave on fsck to handle the mess...
>
> Honza
Right. I wanted cancel the changes but I wasn't sure what I did. Thanks
for the explanation. I'll send v2 soon.
--
Regards,
Namhyung Kim