-
Notifications
You must be signed in to change notification settings - Fork 48
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
passthrough example for ext fs #96
base: master
Are you sure you want to change the base?
Conversation
Thanks! Could you elaborate on the differences to the existing |
Ext filesystem compatibility checked by https://github.com/zfsonlinux/fstest.git. I don't know how good this testsuit is, but i also found no better or new one.
|
To rephrase, the difference between the existing If so, what would it take to modify |
I do not need a new fs, it was choosen to not break other stuff and because different lower fs have different behavoir see second change, regarding time. The only relevant change for passthroughps.py in general in my opinion is to correct the create syscall as stated to fix owner and mode. |
I do not see any changes, I just see one large new file being added. Could you please modify passthroughfs.py instead of adding a new file? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the updated PR, some feedback from me.
if fields.update_mode is False and \ | ||
fields.update_uid is False and \ | ||
fields.update_gid is False: | ||
if chown == os.chown: | ||
chown(path_or_fh, -1, -1, follow_symlinks=False) | ||
if chown == os.fchown: | ||
chown(path_or_fh, -1, -1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this is rather strange:
- why is chown called at all if neither uid nor gid shall be set (both are -1)?
- is the
x is False
really wanted / correct instead of the usual boolean evaluation usingnot x
? consider x == 0 or x == None, for example. - in 311/314
chown
is always called withfollow_symlinks=False
, no matter whether path or fh is used, so 320/322 look inconsistent with these.
@@ -479,3 +493,4 @@ def main(): | |||
|
|||
if __name__ == '__main__': | |||
main() | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
accidental change?
@@ -1,5 +1,6 @@ | |||
#!/usr/bin/env python3 | |||
''' | |||
ext filesystem compatibility checked by https://github.com/zfsonlinux/fstest.git |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is rather a remark or property of this implementation, but nothing the description of this should start with, so move it a bit lower, please.
Ext filesystem compatibility checked by https://github.com/zfsonlinux/fstest.git. I don't know how good this testsuit is, but i also found no better or new one.
Could be merged, i have only tested with ext4 as source fs. This testsuit has a lot of testcases.