From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from relay.sgi.com (relay2.corp.sgi.com [137.38.102.29]) by oss.sgi.com (Postfix) with ESMTP id BFCC27F3F for ; Fri, 6 Jun 2014 11:47:58 -0500 (CDT) Received: from cuda.sgi.com (cuda1.sgi.com [192.48.157.11]) by relay2.corp.sgi.com (Postfix) with ESMTP id 8E6D630404E for ; Fri, 6 Jun 2014 09:47:55 -0700 (PDT) Received: from sandeen.net (sandeen.net [63.231.237.45]) by cuda.sgi.com with ESMTP id wB2C6K9FHQXQw5FB for ; Fri, 06 Jun 2014 09:47:53 -0700 (PDT) Message-ID: <5391F0BC.9010308@sandeen.net> Date: Fri, 06 Jun 2014 11:47:56 -0500 From: Eric Sandeen MIME-Version: 1.0 Subject: Re: [PATCH 2/2 V2] mkfs.xfs: don't call blkid_get_topology on regular files References: <53852A05.5040006@redhat.com> <5390C1B7.7050106@sandeen.net> <20140606152059.GB3048@laptop.bfoster> <5391DDE4.90007@sandeen.net> In-Reply-To: <5391DDE4.90007@sandeen.net> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: xfs-bounces@oss.sgi.com Sender: xfs-bounces@oss.sgi.com To: Brian Foster Cc: Eric Sandeen , xfs-oss On 6/6/14, 10:27 AM, Eric Sandeen wrote: > On 6/6/14, 10:21 AM, Brian Foster wrote: >> On Thu, Jun 05, 2014 at 02:15:03PM -0500, Eric Sandeen wrote: >>> If we encounter a target that's really a regular file, >>> even without "-d file..." on the cmdline, call >>> platform_findsizes() instead of blkid_get_topology to >>> try to discover the "sector size" via the fsgeom() call. >>> >>> Signed-off-by: Eric Sandeen >>> --- >>> >>> V2: Lose local "isa_file" flag, just switch based on >>> (xi->disfile) or (stat works & S_ISREG) >>> >>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c >>> index 188b6b3..3b8bf67 100644 >>> --- a/mkfs/xfs_mkfs.c >>> +++ b/mkfs/xfs_mkfs.c >>> @@ -456,9 +456,25 @@ static void get_topology( >>> struct fs_topology *ft, >>> int force_overwrite) >>> { >>> - if (!xi->disfile) { >>> - const char *dfile = xi->volname ? xi->volname : xi->dname; >>> + struct stat statbuf; >>> + char *dfile = xi->volname ? xi->volname : xi->dname; >>> >>> + /* >>> + * Don't call blkid for topology if this is a "-d file" target, or >>> + * if we've simply been pointed at a regular file. platform_findsizes >>> + * will attempt to find the underlying sector size of the host fs. >>> + */ >>> + if (xi->disfile || >>> + (!stat(dfile, &statbuf) && S_ISREG(statbuf.st_mode))) { >>> + int fd; >>> + long long dummy; >>> + >>> + fd = open(dfile, O_RDONLY); >>> + if (fd >= 0) { >>> + platform_findsizes(dfile, fd, &dummy, &ft->lsectorsize); >>> + close(fd); >>> + } >> >> The patch looks fine: >> >> Reviewed-by: Brian Foster >> >> It does look like we'd still be susceptible to error in a situation >> where the file hasn't been created yet (which would occur in >> libxfs_init()). > > Hm, let's see - the only way to have mkfs create the file is with the > -d file,name=foo,size=bar invocation. > > Sprinkling some printf's into the if/else above, I see: > > # rm -f testfile > # mkfs/mkfs.xfs -dfile,name=testfile,size=1g > ** doing platform_findsizes ** > meta-data=testfile isize=256 agcount=4, agsize=65536 blks > ... > > so it looks like the file does get created before we get here. Oh, I see, sorry. Crud, you're right. What prompted all this was that making a fs-in-a-file on a filesystem hosted on a hard 4k sector device fails, because it tries to do 512-byte DIO. If we use the "-d file" invocation, it works, because it doesn't do direct IO. Sigh. It's complicated. Let me think about the 2nd patch just a bit more. Perhaps only doing the platform_findsizes if stat succeeds, not if xi->disfile. -Eric _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs