xfs
[Top] [All Lists]

Re: [PATCH, RFC] xfstests: add d_type checking to fsstress

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH, RFC] xfstests: add d_type checking to fsstress
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Mon, 16 Sep 2013 19:54:38 -0500
Cc: Eric Sandeen <sandeen@xxxxxxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130916225550.GG19103@dastard>
References: <52377E50.3040907@xxxxxxxxxx> <20130916225550.GG19103@dastard>
User-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:17.0) Gecko/20130801 Thunderbird/17.0.8
On 9/16/13 5:55 PM, Dave Chinner wrote:
> On Mon, Sep 16, 2013 at 04:55:28PM -0500, Eric Sandeen wrote:
>> This patch adds a "-D" switch to fsstress so that every time
>> we call readdir, we stat the dentry & compare it's st_mode
>> to the d_type.
>>
>> If -D is specified only once, it ignores DT_UNKNOWN.  If specified
>> twice, it considers DT_UNKNOWN to be an error.
> 
> Hmmmm. DT_UNKNOWN is actually a valid type on disk right through to the
> userspace interface. I can't think of why we'd want to consider it
> invalid, especially as right now xfs_repair siply zeros the field
> when recreating directory entries...
> 
> Cheers,
> 
> Dave.

 no fair signing off w/ more text below ;)

>> +void test_d_type(int opno, pathname_t *f, struct dirent64 *de)
>> +{
>> +    struct stat64 sb;
>> +    char path[PATH_MAX];
>> +
>> +    snprintf(path, PATH_MAX, "%s/%s", f->path, de->d_name);
>> +
>> +    /* Don't check ./. or ./.. */
>> +    if (!strncmp(path, "./.", 3))
>> +            return;
> 
> . and .. should have the values of DT_UNKNOWN or DT_DIR. They are
> the only valid values for these entries.

Hm let me look at something, I saw something that prompted this
but now that I think about it maybe it's a bug.

>> +
>> +    if (lstat64(path, &sb)) {
>> +            printf("%d/%d: getdents - can't stat %s\n",
>> +                    procid, opno, path);
>> +    } else {
>> +            int bad_d_type = 0;
>> +
>> +            switch (de->d_type) {
>> +                    case DT_BLK:
>> +                            if (!S_ISBLK(sb.st_mode))
>> +                                    bad_d_type++;
>> +                            break;
>> +                    case DT_CHR:
>> +                            if (!S_ISCHR(sb.st_mode))
>> +                                    bad_d_type++;
>> +                            break;
>> +                    case DT_DIR:
>> +                            if (!S_ISDIR(sb.st_mode))
>> +                                    bad_d_type++;
>> +                            break;
>> +                    case DT_FIFO:
>> +                            if (!S_ISFIFO(sb.st_mode))
>> +                                    bad_d_type++;
>> +                            break;
>> +                    case DT_LNK:
>> +                            if (!S_ISLNK(sb.st_mode))
>> +                                    bad_d_type++;
>> +                            break;
>> +                    case DT_REG:
>> +                            if (!S_ISREG(sb.st_mode))
>> +                                    bad_d_type++;
>> +                            break;
>> +                    case DT_SOCK:
>> +                            if (!S_ISSOCK(sb.st_mode))
>> +                                    bad_d_type++;
>> +                            break;
>> +                    case DT_UNKNOWN:
>> +                            if (verify_d_type > 1)
>> +                                    bad_d_type++;
>> +                    break;
>> +            }
> 
> And DT_WHT? That's defined on disk and in the user interface ;)

but fsstress won't create it, will it?
 
> i.e. this will not do the right thing with an unknown de->d_type.

... but we know what fsstress can possibly create, right, so testing
those created types should be safe, I'd think.

I'll give this some thought & send V2.

-Eric

> Cheers,
> 
> Dave.
> 

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