xattr ioengine

[struschev]

Hello!

I've prepared support of xattr operations measurement into fio. The only thing, I think, is that multiple operations in one IOPS doesn't quite fit the fio philosophy. I would like to know your opinion on this matter.

[vincentkfu]

The second patch is not needed since there is no change to the ioengine ops struct.

[struschev]

The second patch is not needed since there is no change to the ioengine ops struct.

Oh, ok, I've understood something wrong, so I reverted commit

[struschev]

Isn't it normal that CI didn't work for Windows and Macos?

[sitsofe]

Isn't it normal that CI didn't work for Windows and Macos?

We do tend to see the odd failure but this macOS error looks to be a permanent build error:

engines/fileoperations.c:271:61: error: too few arguments to function call, expected 6, have 5
                ret = setxattr(f->file_name, attrname, attrval, o->size, 0);
                      ~~~~~~~~                                            ^
/Applications/Xcode_15.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/xattr.h:65:5: note: 'setxattr' declared here
int setxattr(const char *path, const char *name, const void *value, size_t size, u_int32_t position, int options);
    ^
engines/fileoperations.c:325:42: error: too few arguments to function call, expected 4, have 3
        buflen = listxattr(f->file_name, NULL, 0);
                 ~~~~~~~~~                      ^
/Applications/Xcode_15.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/xattr.h:73:9: note: 'listxattr' declared here
ssize_t listxattr(const char *path, char *namebuff, size_t size, int options);
        ^
engines/fileoperations.c:334:50: error: too few arguments to function call, expected 4, have 3
        buflen = listxattr(f->file_name, attrbuf, buflen);
                 ~~~~~~~~~                              ^
/Applications/Xcode_15.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/xattr.h:73:9: note: 'listxattr' declared here
ssize_t listxattr(const char *path, char *namebuff, size_t size, int options);
        ^
engines/fileoperations.c:342:52: error: too few arguments to function call, expected 6, have 4
                vallen = getxattr(f->file_name, attrname, NULL, 0);
                         ~~~~~~~~                                ^
/Applications/Xcode_15.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/xattr.h:61:9: note: 'getxattr' declared here
ssize_t getxattr(const char *path, const char *name, void *value, size_t size, u_int32_t position, int options);
        ^
engines/fileoperations.c:350:61: error: too few arguments to function call, expected 6, have 4
                        vallen = getxattr(f->file_name, attrname, attrval, vallen);
                                 ~~~~~~~~                                        ^
/Applications/Xcode_15.2.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/sys/xattr.h:61:9: note: 'getxattr' declared here
ssize_t getxattr(const char *path, const char *name, void *value, size_t size, u_int32_t position, int options);
        ^

(from https://github.com/axboe/fio/actions/runs/18996147708/job/54256207771?pr=2009 ).

Windows also looks like a build failure:

engines/fileoperations.c:13:10: fatal error: sys/xattr.h: No such file or directory
   13 | #include <sys/xattr.h>
      |          ^~~~~~~~~~~~~
compilation terminated.

(from https://github.com/axboe/fio/actions/runs/18996147708/job/54256207773?pr=2009#step:8:183 ).

[sitsofe]

If you're introducing a platform specific engine you will have to add a configure probe to stop it being built on platforms that are not supported. See what e4defrag engine does for an example. For an engine which has parts that may not be available see the #ifdef FIO_HAVE_PWRITEV2 sections in https://github.com/axboe/fio/blob/master/engines/sync.c , https://github.com/axboe/fio/blob/master/options.c and the pwritev probe in https://github.com/axboe/fio/blob/master/configure .

[axboe]

What @sitsofe said, but also please sanitize your git history and commit messages too. Your commit message does not follow the required format (signed-off-by, for example), and it should also have an actual description of why this commit exists. All it has is "add xattr ioengines" which is pretty obvious. Explain what the engines are, and what they do. Basically duplicate some of the manual additions you have.

And don't add a commit you don't need (the version bump) and then add a revert of that. I don't want useless commits like that in the git history. Just rebase your branch, dropping those two patches, and then force push a new one for the PR.

Outside of that, I do think the engine makes sense, and it mostly looks good. There are some useless additions like variables that you don't really need. Things like int do_lat = !td->o.disable_lat; - just drop that and use td->o.disable_lat in the code. They don't add anything or optimize anything, they only help to obfuscate what's actually being tested here.

[struschev]

Fixed compilation errors, git history and commit message

[struschev]

Hello! Did you have a time to look my PR?

[axboe]

Please replace all the ifdef for WIN32 with a proper CONFIG* flag, as generated by either configure, or in this case just set for windows. Also please look at reducing the number of them in the first place. Having them littered throughout the code makes for a fragile and hard to maintain code base. There are numerous examples in the code base for how to do that, either with stubs that return an error for the non-CONFIG_FOO case or similar.