From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Price Date: Tue, 13 Aug 2013 10:04:24 +0100 Subject: [Cluster-devel] [PATCH] gfs2_grow: Don't try to open an empty string In-Reply-To: <1376041277.2718.3.camel@menhir> References: <1375981344-30943-1-git-send-email-anprice@redhat.com> <1376041277.2718.3.camel@menhir> Message-ID: <5209F698.2010402@redhat.com> List-Id: To: cluster-devel.redhat.com MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit On 09/08/13 10:41, Steven Whitehouse wrote: > Hi, > > On Thu, 2013-08-08 at 18:02 +0100, Andrew Price wrote: >> sdp->device_name wasn't getting set in gfs2_grow so is_gfs2() (called >> via check_for_gfs2()) fails to open "" and so we get an ENOENT. >> >> This fixes the open("") by setting sdp->device_name before passing it to >> is_pathname_mounted(), which has been updated to make it more clear >> which args will be modified by it. >> >> is_gfs2() and check_for_gfs2() have also been removed from libgfs2 as >> these were the only places they were being called and they aren't >> needed: superblock checking is done further along via other functions >> calling check_sb(). >> > Definitely an improvement over what we had before, but I wonder whether > we can do better still. Is the fd always open before we call > is_pathname_mounted() I wonder? If so we should be able to just pass the > fd to it which reduces the possibility for races I think, and may also > simplify things a bit more, Well the problem here is that we don't know whether the path is a device or a mount point (gfs2_{grow,jadd} accept both) beforehand so any open before is_pathname_mounted() is called is going to be a guess. I think we probably can improve on this function in the process of making libgfs2 more fd-centric but it'll take a bit more work so I'll come back to that later and push this patch independently as it solves a problem that's currently holding up testing. Cheers, Andy