xfs
[Top] [All Lists]

Re: [PATCH] mkfs: don't try to detect filesystems on regular files via b

To: Jim Meyering <jim@xxxxxxxxxxxx>
Subject: Re: [PATCH] mkfs: don't try to detect filesystems on regular files via blkid
From: Eric Sandeen <sandeen@xxxxxxxxxx>
Date: Thu, 04 Feb 2010 10:54:17 -0600
Cc: xfs mailing list <xfs@xxxxxxxxxxx>
In-reply-to: <878wb9j6e8.fsf@xxxxxxxxxxxx>
References: <4B6AF6FC.6030101@xxxxxxxxxx> <878wb9j6e8.fsf@xxxxxxxxxxxx>
User-agent: Thunderbird 2.0.0.21 (X11/20090320)
Jim Meyering wrote:
> Eric Sandeen wrote:
>> https://bugzilla.redhat.com/show_bug.cgi?id=561870
>>
>> # dd if=/dev/zero of=k bs=1MB count=2 seek=20; mkfs.xfs k
>> # mkfs.xfs: probe of k failed, cannot detect existing filesystem.
>> # mkfs.xfs: Use the -f option to force overwrite.
>>
>> blkid fails to do a probe of a regular file.
>>
>> I wish blkid would cope with this, but for now it might
>> be better to just turn it off.
>>
>> Reported-by: Jim Meyering <meyering@xxxxxxxxxx>
>> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
>> ---
>>
>> diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
>> index 9baf116..de87647 100644
>> --- a/mkfs/xfs_mkfs.c
>> +++ b/mkfs/xfs_mkfs.c
>> @@ -300,10 +300,15 @@ check_overwrite(
>>      int             fd;
>>      long long       size;
>>      int             bsz;
>> +    struct stat     statbuf;
>>
>>      if (!device || !*device)
>>              return 0;
>>
>> +    /* blkid can't get info from a regular file */
>> +    if (!stat(device, &statbuf) && S_ISREG(statbuf.st_mode))
>> +            return 0;
>> +
>>      ret = -1; /* will reset on success of all setup calls */
>>
>>      fd = open(device, O_RDONLY);
> 
> Hi Eric,
> 
> Did you consider calling fstat after opening,
> rather than "stat" before?
> 
>         /* blkid can't get info from a regular file */
>         if (!fstat(fd, &statbuf) && S_ISREG(statbuf.st_mode)) {
>                 close(fd);
>                 return 0;
>         }
> 
> That's slightly more efficient, and not prone to confusion
> in the unlikely event that "device" changes inode between
> the stat and the open calls.

Ok, yes that's probably better, will resend.  thanks.

-Eric

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