Re: [Bug-tar] patches for acl, xattrs, and selinux support

[Girish Shilamkar]

Hello,

On Fri, 2008-12-19 at 18:25 -0700, Andreas Dilger wrote:

> I've been trying to get some feedback on some changes to the xattr patches,
> but haven't been able to contact any of the people who worked on these in
> Red Hat.  Is it better to just email you directly (and/or this list), or
> file a bug in Bugzilla?
> 
> In particular, we'd like to:
> - Always backup all xattrs on a file instead of current practise of filtering
>   the xattrs at backup time.  This ensures they are all available later on
>   if needed, even if they are not all restored by default.  The filtering
>   would only be done at restore time, and that can be fixed if it is wrong.
> - Change the restoration of xattrs to be before any file data is written.
>   This allows the xattrs to contain layout hints, which is important for
>   distributed filesystems (Lustre, pNFS) and possibly also ext4, btrfs, XFS,
>   that have smarter allocators that can take advantage of this in the future
>   once they realize what is possible.
Please find the attached a patch which comes close to implementing the
above changes. The patch has been applied on top tar-1.20-6.

Comments/suggestions are most welcome.

While adding the last hunk in the patch viz.
---------------------------------------------------------------------
/* xattr's, use the star format note we only save the user/trusted
varients... */ 
{ "SCHILY.xattr.user",    xattr_coder, xattr_decoder, false, true },
   { "SCHILY.xattr.trusted", xattr_coder, xattr_decoder, false, true },
+  { "SCHILY.xattr.lustre", xattr_coder, xattr_decoder, false, true },
 
   /* ignore everything else in the xattr namespaces... */
   { "SCHILY.xattr",         dummy_coder, dummy_decoder, false, true }
------------------------------------------------------------------------

I noticed that to allow any xattr other than user and trusted we not
only need to change the code which decides which xattrs to store in
xattrs_xattrs_get & xattrs_xattrs_set but also add it corresponding
entry in xhdr_tab[].

Isn't this a redundant check or am I missing something here ?

How about using following hunk, instead of the above:

-  /* xattr's, use the star format note we only save the user/trusted
varients... */ 
-  { "SCHILY.xattr.user",    xattr_coder, xattr_decoder, false, true },
-  { "SCHILY.xattr.trusted", xattr_coder, xattr_decoder, false, true },
-
-  /* ignore everything else in the xattr namespaces... */
-  { "SCHILY.xattr",         dummy_coder, dummy_decoder, false, true },
+  { "SCHILY.xattr",         xattr_coder, xattr_decoder, false, true },

This would allow implementation of the change, which Andreas had
mentioned, about backing up all the xattrs (patch as of now only adds
"SCHILY.xattr.lustre"). And the xattrs to be re-stored can be restricted
in xattrs_xattrs_set()

Thanks,
Girish

Extract-xattrs-before-data.patch
Description: Text Data