[Ocfs2-devel] [patch 1/8] ocfs2: should add inode into orphan dir after updating entry in ocfs2_rename()

[Mark Fasheh]

On Thu, Jun 12, 2014 at 03:52:14PM -0700, Mark Fasheh wrote:
> On Mon, Jun 09, 2014 at 01:03:59PM -0700, Andrew Morton wrote:
> > From: alex chen <alex.chen at huawei.com>
> > Subject: ocfs2: should add inode into orphan dir after updating entry in ocfs2_rename()
> > 
> > There are two files a and b in dir /mnt/ocfs2.
> >     node A                           node B
> > mv a b
> > In ocfs2_rename(), after calling
> > ocfs2_orphan_add(), the inode of
> > file b will be added into orphan
> > dir.
> > 
> > If ocfs2_update_entry() fails,
> > ocfs2_rename return error and mv
> > operation fails. But file b still
> > exists in the parent dir.
> > 
> > ocfs2_queue_orphan_scan
> > -> ocfs2_queue_recovery_completion
> > -> ocfs2_complete_recovery
> > -> ocfs2_recover_orphans
> > The inode of the file b will be
> > put with iput().
> > 
> > ocfs2_evict_inode
> > -> ocfs2_delete_inode
> > -> ocfs2_wipe_inode
> > -> ocfs2_remove_inode
> > OCFS2_VALID_FL in the inode
> > i_flags will be cleared.
> > 
> >                                    The file b still can be accessed
> >                                    on node B.
> >                                    ls /mnt/ocfs2
> >                                    When first read the file b with
> >                                    ocfs2_read_inode_block(). It will
> >                                    validate the inode using
> >                                    ocfs2_validate_inode_block().
> >                                    Because OCFS2_VALID_FL not set in
> >                                    the inode i_flags, so the file
> >                                    system will be readonly.
> > 
> > So we should add inode into orphan dir after updating entry in
> > ocfs2_rename().
> 
> That looks fine. I guess the downside now is that if we fail during orphan
> add we could leak the file which would have been otherwise replaced. In my
> opinion though this is much better than going readonly because we have a
> link to a dead file.
> 
> 
> Thanks for this Alex,
> 	--Mark

Err, probably helps if I leave this, sorry:

Reviewed-by: Mark Fasheh <mfasheh at suse.de>

--
Mark Fasheh