This repository was archived by the owner on Jul 8, 2024. It is now read-only.
Store projection metadata as extended attributes#60
Merged
chrisd8088 merged 9 commits intomasterfrom Mar 15, 2019
Merged
Conversation
kivikakk
reviewed
Mar 13, 2019
Contributor
kivikakk
left a comment
There was a problem hiding this comment.
Looking good, some general comments.
Collaborator
Author
kivikakk
reviewed
Mar 14, 2019
Contributor
kivikakk
left a comment
There was a problem hiding this comment.
This is all looking excellent, and the matter re: make_user_xattr_name has reminded me of a video I'm going to link in Slack in a moment!
No implementation as yet; just defining the expected API changes and additions.
Borrowing from overlayfs's internal overlayfs.h, we define a common "user.projection." extended attribute namespace prefix, and then define our private empty flag attribute using it. We expect to prepend our prefix to all user-defined projection attributes, e.g., "user.projection.vfsforgit.contentid", etc.
Refactor our handling of the "empty" projection flag extended attribute into two common base utility functions which get or set/remove xattrs, and a specialized set of wrappers of these common functions which get, set, or remove our projection flag attribute. The common base functions catch the the ENOATTR "attribute not found" error when reading or removing an attribute, and in this case, return success but reset the caller's 'size' argument to -1 to signal that the attribute was not found. We will subsequently use this common pair of base functions to manage additional, user-provided projection attributes, as will be required for VFSForGit support.
We introduce a common iteration function for parsing and applying user-provider lists of attributes, and use it to store such attributes when they are provided during a call to create a projected file or directory. The iteration function will also be useful when we add the ability for callers to subsequently read or reset these attributes after the initial time of creation. We store the name/value user attributes as xattrs under our library's "user.projection.*" namespace, unless they would conflict with our reserved "user.projection.empty" attribute; we allow clients to potentially read that attribute, but not overwrite or remove it.
When a normal setxattr() or removexattr() file operation is invoked via our FUSE event loop, we check if it might alter or remove one of the attributes we are managing within our "user.projection.*" namespace, and if so, we deny the request. While it is still possible for these xattrs to be modified through changes made directly against our lower storage filesystem, these checks should prevent inadvertent changes made through our mount point.
Complete the implementation of our attribute API with projfs_get_attrs() and projfs_set_attrs() function that permit a client to read, set, reset, or remove any of their user-provided projection attributes subsequent to the initial projection of a file or directory.
Ensure the constructed user-provided xattr name is always freed and doesn't leak memory, and separate name string construction from its validation and use, per advice from @kivikakk.
Rename and reverse the sense of the xattr name check functions to improve readability and code flow, per advice from @kivikakk.
Clarify the purpose of our projection-flag xattr handling functions (which get/set/remove the "user.projection.empty" extended attribute) by renaming to avoid the confusing "_empty" suffix, per @kivikakk's suggestion. Also use sizeof() to define the length of our reserved xattr namespace prefix string, and align some whitespace.
d36fb03 to
2a1423d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi @kivikakk -- I'm hoping you might take a look at the code so far in this branch and give me a review.
I'm not looking to merge this into
masterquite yet, as I'd really like to get me test suite additions included and I'm only halfway on those. But I thought I'd get your eyes on the code changes while I finish up the tests.The corresponding PR for the VFSForGit code is github/VFSForGit#8 and again I'm hoping not to merge that quite yet, but would very much appreciate a review!