containerd / continuity

A transport-agnostic, filesystem metadata manifest system
https://containerd.io
Apache License 2.0
139 stars 67 forks source link

fs: add WithAllowXAttrErrors CopyOpt #138

Closed AkihiroSuda closed 5 years ago

AkihiroSuda commented 5 years ago

This option allows ignoring errors during copying xattr like security.selinux, which is not always supported.

Reported in several issues including

Signed-off-by: Akihiro Suda suda.akihiro@lab.ntt.co.jp

AkihiroSuda commented 5 years ago

cc @tonistiigi

AkihiroSuda commented 5 years ago

So the PR appeared to have nothing to do with the buildkit failure but should be still useful

nixpanic commented 5 years ago

Please consider this change on top of yours. It continues copying xattrs for a file, even if one of them fails (like security.selinux).

Feel free to incorporate the modification in an updated PR. Thanks!

AkihiroSuda commented 5 years ago

@nixpanic Thanks, cherry-picked your commit

AkihiroSuda commented 5 years ago

@dmcgowan @cpuguy83 PTAL?

cpuguy83 commented 5 years ago

Oh, nice! Also related: https://github.com/moby/moby/issues/38155

cpuguy83 commented 5 years ago

I think for this case, being able to pass in an error handler on file copy would be handy.

Something like:

func(err error) error

Where if the handler wants to ignore the error it would return nil.

AkihiroSuda commented 5 years ago

Added type XAttrErrorHandler func(dst, src, xattrKey string, err error) error. PTAL.

nixpanic commented 5 years ago

Hey guys, any progress with reviewing/merging? This PR works well for me and I'd like to see it included so that I can consume it in kubernetes-incubator/external-storage#1013 and a future CSI driver.

Thanks!