On Feb 13, 2020, at 3:15 AM, Jan Kara <jack@xxxxxxx> wrote: > > Currently, ext2fs_mkdir() and ext2fs_symlink() update allocation bitmaps > and other information only close to the end of the function, in > particular after calling to ext2fs_link(). When ext2fs_link() will > support indexed directories, it will also need to allocate blocks and > that would cause filesystem corruption in case allocation info isn't > properly updated. So make sure ext2fs_mkdir() and ext2fs_symlink() > update allocation info before calling into ext2fs_link(). > > Signed-off-by: Jan Kara <jack@xxxxxxx> I was wondering if this was done at the end of the function to avoid the need to undo it if there was an error in the middle of the operation? I suppose the worst that would happen in that case is an extra bit set in the block bitmap until the next e2fsck, which is a relatively safe side-effect... I'm not sure whether e2fsck would abort anyway in the case either of these functions return an error? In any case, this is better than what is there currently. Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx> > --- > lib/ext2fs/mkdir.c | 14 +++++++------- > lib/ext2fs/symlink.c | 14 +++++++------- > 2 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/lib/ext2fs/mkdir.c b/lib/ext2fs/mkdir.c > index 2a63aad16715..947003ebf309 100644 > --- a/lib/ext2fs/mkdir.c > +++ b/lib/ext2fs/mkdir.c > @@ -143,6 +143,13 @@ errcode_t ext2fs_mkdir(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t inum, > } > } > > + /* > + * Update accounting.... > + */ > + if (!inline_data) > + ext2fs_block_alloc_stats2(fs, blk, +1); > + ext2fs_inode_alloc_stats2(fs, ino, +1, 1); > + > /* > * Link the directory into the filesystem hierarchy > */ > @@ -175,13 +182,6 @@ errcode_t ext2fs_mkdir(ext2_filsys fs, ext2_ino_t parent, ext2_ino_t inum, > goto cleanup; > } > > - /* > - * Update accounting.... > - */ > - if (!inline_data) > - ext2fs_block_alloc_stats2(fs, blk, +1); > - ext2fs_inode_alloc_stats2(fs, ino, +1, 1); > - > cleanup: > if (block) > ext2fs_free_mem(&block); > diff --git a/lib/ext2fs/symlink.c b/lib/ext2fs/symlink.c > index 7f78c5f75549..3e07a539daf3 100644 > --- a/lib/ext2fs/symlink.c > +++ b/lib/ext2fs/symlink.c > @@ -162,6 +162,13 @@ need_block: > goto cleanup; > } > > + /* > + * Update accounting.... > + */ > + if (!fastlink && !inlinelink) > + ext2fs_block_alloc_stats2(fs, blk, +1); > + ext2fs_inode_alloc_stats2(fs, ino, +1, 0); > + > /* > * Link the symlink into the filesystem hierarchy > */ > @@ -179,13 +186,6 @@ need_block: > goto cleanup; > } > > - /* > - * Update accounting.... > - */ > - if (!fastlink && !inlinelink) > - ext2fs_block_alloc_stats2(fs, blk, +1); > - ext2fs_inode_alloc_stats2(fs, ino, +1, 0); > - > cleanup: > if (block_buf) > ext2fs_free_mem(&block_buf); > -- > 2.16.4 > Cheers, Andreas
Attachment:
signature.asc
Description: Message signed with OpenPGP
