Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

For nfs engine, treat randwrite the same as write when opening file #1775

Closed
wants to merge 4 commits into from
Closed

For nfs engine, treat randwrite the same as write when opening file #1775

wants to merge 4 commits into from

Conversation

panxiao2014
Copy link

When use nfs engine, only write mode can work if file doesn't exist before test starts. This code change just made randwrite behaves the same as write. So both write and randwrite could work in scenario where file doesn't exist before test start.

Note: other than write and randwrite mode, nfs engine would report error when test start. This is because fio would not layout the file before test start. This also indicate that currently nfs engine requires user to make sure files are ready before test when choose mode such as read, randread and readwrite.

This requires more code changes. So now just add one line code change to let randwrite can work.

When use nfs engine, only write mode can work if file doesn't exist,
otherwise user has to make sure file exists before test starts.
This change will support randwrite mode when file doesn't exist.

Pan Xiao <[email protected]>
@panxiao2014
Copy link
Author

panxiao2014 commented Jun 13, 2024

Hi @vincentkfu , I saw there is an error when checking build on cygwin platform: multiple definition of `strtoll' found. I did not touch this part of code. Could you please check if there is additional task that I should do before submitting PR?

Thanks.

@vincentkfu
Copy link
Collaborator

That failure is due to changes in an external library and unrelated to your changes.

@panxiao2014
Copy link
Author

Thanks Vincent. We are using fio for our NFS server test. The change here could make our test script automation works easier. Please help to check if this code change could be promoted.

engines/nfs.c Outdated
@@ -280,7 +280,7 @@ static int fio_libnfs_open(struct thread_data *td, struct fio_file *f)
nfs_data = calloc(1, sizeof(struct nfs_data));
nfs_data->options = options;

if (td->o.td_ddir == TD_DDIR_WRITE)
if (td->o.td_ddir == TD_DDIR_WRITE || td->o.td_ddir == TD_DDIR_RANDWRITE)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should just use:

if (td_write(td))

it's just a basic bug in the NFS engine, that it checks specifically for == TD_DDIR_WRITE rather than & TD_DDIR_WRITE.

@panxiao2014
Copy link
Author

Thanks Jens. I have updated the code change and run some test to verify it. Please review.

@axboe
Copy link
Owner

axboe commented Jun 14, 2024

Looks fine, but please squash that into one commit. It's a bit pointless to have a suboptimal fix, and then a fixup on top.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants