image-rs | Failed to set xattr for tmpfs

[Xynnn007]

Thanks to @bpradipt , when we pull an image to tmpfs that does not support xattr, e.g. quay.io/bpradipt/hello-ocp:no-long-link

Whether xattr is supported by a mount path can be checked by mount | grep tmpfs. If there is no xattr inside the (...), it does not supports.

failures:

---- pull::tests::test_async_pull_client stdout ---- Got error on function call attempt 0. Will retry in 1s: failed to handle layer: hasher sha256: failed to unpack /tmp/.tmpbPSr5B/sha256_fda7f57d59e2441f415e2f05ff824b63f0d64d521f0afe0c1f10e09f5c30f22c/var/lib/dnf/history.sqlite

Caused by: 0: failed to set extended attributes to /tmp/.tmpbPSr5B/sha256_fda7f57d59e2441f415e2f05ff824b63f0d64d521f0afe0c1f10e09f5c30f22c/var/lib/dnf/history.sqlite. Xattrs: key="user.overlay.origin", value="". 1: Operation not supported (os error 95) Got error on function call attempt 1. Will retry in 1s: failed to handle layer: hasher sha256: failed to unpack /tmp/.tmpbPSr5B/sha256_fda7f57d59e2441f415e2f05ff824b63f0d64d521f0afe0c1f10e09f5c30f22c/var/lib/dnf/history.sqlite

[bpradipt]

May be this needs to be conditional based on extended attributes support in the underlying fs - https://github.com/confidential-containers/guest-components/blob/main/image-rs/src/unpack.rs#L29

[Xynnn007]

The original image-rs implementation with tar does not enable set_unpack_xattrs. I think it is reasonable, because we can detect the current platform, but it is hard to detect the destination directory supports xattrs -- it could be a non-exist dir.

There are two ways

1. Query the destination's parent path recursively until an existing path is found, checking whether it supports xattr
2. follow old image-rs way, s.t. disable set_unpack_xattrs by default. This is the comments that tar says

   /// This flag is disabled by default and is currently only implemented on
   /// Unix using xattr support. This may eventually be implemented for
   /// Windows, however, if other archive implementations are found which do
   /// this as well.

I prefer to way 2 and lazily fix situations where real xattrs keeping is needed. wdyt ? @bpradipt

[bpradipt]

@Xynnn007 2 seems simplest. Alternatively a modified variation for option -1, where we don't check recursively and only check the destination path. If query is not possible, then checking by creating a dummy file in the destination and setting/removing an extended attribute. If it succeeds then we set xattrs otherwise we don't.

[Xynnn007]

Alternatively a modified variation for option -1, where we don't check recursively and only check the destination path. If query is not possible, then checking by creating a dummy file in the destination and setting/removing an extended attribute. If it succeeds then we set xattrs otherwise we don't.

Yes. This is a good choice -- if we could live with the time cost. Let me do a simple test to check the timeout, if it explodes we'd better choose 1 as we do not know if we really need it.

[Xynnn007]

Ok everytime we found there is a target path we will try to delete that. So we could do an easy check for the target dir only once. I think the cost would be really low.