xfs
[Top] [All Lists]

Re: [PATCH 7/8] xfsprogs: clean up errors in libxfs_mount() consistently

To: Alex Elder <aelder@xxxxxxx>
Subject: Re: [PATCH 7/8] xfsprogs: clean up errors in libxfs_mount() consistently
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Mon, 14 Nov 2011 05:28:45 -0500
Cc: xfs@xxxxxxxxxxx
In-reply-to: <fa64086e0baa316ab27ca7cffa1e9cd214525a87.1320955676.git.aelder@xxxxxxx>
References: <3ed89446f6dc1231ec1de712f242522052952b7e.1320955675.git.aelder@xxxxxxx> <fa64086e0baa316ab27ca7cffa1e9cd214525a87.1320955676.git.aelder@xxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Nov 10, 2011 at 02:35:17PM -0600, Alex Elder wrote:
> Until the perag structures for the filesystem have been set up,
> initialization of the mount point only assigns computed values and
> has no other side-effects (such as allocating additional
> structures).  So up to that point, returning a null pointer to
> signal an error is adequate.  Once the perag initialization is done
> there needs to be some teardown in case of an error.
> 
> Here the handling of errors is inconsistent.  If early perag
> initialization fails, the code currently just exits.  Then, if
> getting a reference to the root inode results in an error, a null
> pointer is returned but without first cleaning up the perag
> structures.  Next, if rtmount_inodes() returns an error, the
> reference to the root inode (if any) is released, but again the
> perag structures are not cleaned up.  Finally, when the perag data
> is read in, if an error occurs, a null pointer is returned but the
> root inode pointer reference is not released and the perag
> structures are not cleaned up.
> 
> Remedy all of that by having each of these error cases jump to error
> handling code at the end of the function.  That code needs to
> release the reference to the root inode and release all of the perag
> structures.  This (plus a few other things that will be no-ops at
> this point in the mount process) is exactly what libxfs_umount(), so
> just use that function to implement this cleanup activity.
> 
> Signed-off-by: Alex Elder <aelder@xxxxxxx>

Did you make sure libxfs_umount doesn't do too much work?  I'm usually
more of a fan of uninding each operation individually.

<Prev in Thread] Current Thread [Next in Thread>