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

Linux/POSIX properties persistence #1780

Merged
merged 27 commits into from
Jun 1, 2022
Merged

Linux/POSIX properties persistence #1780

merged 27 commits into from
Jun 1, 2022

Conversation

adreed-msft
Copy link
Member

@adreed-msft adreed-msft commented May 12, 2022

This PR implements a new flag, --preserve-posix-properties that allows a user to persist the results of a statx(2) (or stat(2) if statx is unavailable) syscall on upload.

Download is not implemented in this PR. S2S is handled by metadata, and in S2S --preserve-posix-properties acts equivalently to --include-directory-stubs.

@shalinijoshi19
Copy link
Collaborator

Hey @adreed-msft ! Thanks for putting this up - question - does this also include the metadata side-car'ing to Az Blobs in particular?

cmd/copy.go Outdated Show resolved Hide resolved
cmd/copy.go Outdated Show resolved Hide resolved
cmd/copy.go Outdated Show resolved Hide resolved
cmd/copy.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@linuxsmiths linuxsmiths left a comment

Choose a reason for hiding this comment

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

LG mostly. Few comments.

cmd/copyEnumeratorInit.go Outdated Show resolved Hide resolved
cmd/zc_enumerator.go Show resolved Hide resolved
cmd/zc_traverser_blob.go Outdated Show resolved Hide resolved
cmd/zc_traverser_blob.go Outdated Show resolved Hide resolved
cmd/zc_traverser_local_other.go Outdated Show resolved Hide resolved
cmd/zc_traverser_local_windows.go Outdated Show resolved Hide resolved
common/fe-ste-models.go Outdated Show resolved Hide resolved

mask uint32
attributes uint64
numLinks uint64
Copy link
Collaborator

Choose a reason for hiding this comment

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

better to align the types with statx struct.
so numLinks should be uint32
and mode should be uint16

Copy link
Member Author

Choose a reason for hiding this comment

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

I disagree. This struct (and interface) is OS-agnostic, but POSIX specific. If we decide to support some other POSIX-based OS like MacOS or FreeBSD in the future for the POSIX property persistence feature, this could cause problems if we adhere to Linux-specific data typing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant those sizes are POSIX specific not Linux specific ("stat()" is a POSIX call and these sizes are not specific to statx()). So any POSIX OS should have the same sizes.

ste/sourceInfoProvider-Local_linux.go Show resolved Hide resolved
@adreed-msft adreed-msft marked this pull request as ready for review May 27, 2022 20:11
@shalinijoshi19
Copy link
Collaborator

The PR remains in draft state, as download is partially broken in some aspects (attributes fail to even be written, etc.), there is no automated testing of the feature yet, and the build fails on other OSes due to certain function signatures..

This PR implements a new flag, --preserve-posix-properties that allows a user to persist the results of a statx(2) (or stat(2) if statx is unavailable) syscall.

To work around cases where the representative device ID may not be related on the destination system, --preserve-posix-properties has two modes: FullFidelity, and SpecialFilesAndProperties, where FullFidelity persists even files representing devices, and SpecialFilesAndProperties skips over these files.

Do update the description wiht this version of the PR pls! thanks

cmd/copy.go Outdated Show resolved Hide resolved
cmd/sync.go Outdated Show resolved Hide resolved
Copy link
Contributor

@zezha-msft zezha-msft left a comment

Choose a reason for hiding this comment

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

lgtm

@zezha-msft zezha-msft merged commit 698f69a into dev Jun 1, 2022
@siminsavani-msft siminsavani-msft added this to the 10.16.0 milestone Jul 14, 2022
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.

None yet

5 participants