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

Add pid in all fuse ops #92

Merged
merged 7 commits into from
Oct 28, 2020
Merged

Add pid in all fuse ops #92

merged 7 commits into from
Oct 28, 2020

Conversation

xinmeigui-db
Copy link
Contributor

Add pid in all fuse ops

@xinmeigui-db
Copy link
Contributor Author

Hi @stapelberg , would appreciate some comments & feedback!

@stapelberg
Copy link
Collaborator

Adding the caller process id to all ops sounds okay to me in general, but:

  1. Why do you need to disable writeback caching for the pid to become available?
  2. Your pull request seems to contain a lot of unrelated commits? Can you clean it up please? (rebase)

Thanks,

@xinmeigui-db
Copy link
Contributor Author

Adding the caller process id to all ops sounds okay to me in general, but:

  1. Why do you need to disable writeback caching for the pid to become available?
  2. Your pull request seems to contain a lot of unrelated commits? Can you clean it up please? (rebase)

Thanks,

Thanks for the review!

  1. We disabled the writeback cache because when the write comes from the writeback cache, there is no pid associated with that (the test may be flaky as a result).
  2. Rebased!

@dotslash
Copy link
Contributor

dotslash commented Oct 28, 2020

Your pull request seems to contain a lot of unrelated commits? Can you clean it up please? (rebase)

If the PR is squash-merged, then this is not relevant. right?

@stapelberg stapelberg merged commit 36e01f1 into jacobsa:master Oct 28, 2020
@stapelberg
Copy link
Collaborator

If the PR is squash-merged, then this is not relevant. right?

Cleanly separated commits also make the review easier, no matter if they end up in the git history or not :)

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