Bug 561855

Created attachment 388800 [details]
[PATCH 1/2] xattrs: add support for lustre attributes

Created attachment 388801 [details]
[PATCH 2/2] xattrs: set attributes before extracting a file

Andreas, could you please have a look at the patches?

Do they still work for you?

Do we have any test case for them?

Thanks in advance!

Created attachment 389010 [details]
extract xattrs before writing file data

Attached is the current version of the extract xattrs first patch, with a few cleanups over the one you had.

Created attachment 389012 [details]
extract xattrs before writing file data

Created attachment 389013 [details]
allow lustre.* extended attributes to be backed up and restored

Updated version of the xattr-lustre patch.  The one attached to this bug failed to handle the lustre attributes correctly.

Comment on attachment 389013 [details]
allow lustre.* extended attributes to be backed up and restored

>--- tar-1.22.sun1.orig/src/xheader.c
>+++ tar-1.22.sun1/src/xheader.c
>@@ -1677,9 +1677,10 @@ struct xhdr_tab const xhdr_tab[] = {
>   { "SCHILY.xattr.system.posix_acl_default",
>     xattr_acls_d_coder, xattr_acls_d_decoder, false, false },
> 
>-  /* xattr's, use the star format note we only save the user/trusted varients... */
>+  /* xattrs use the star format.  note we only save some variants... */
>   { "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 },

The above is the only change on top of the attachment #388800 [details].

Over all the patch looks good to me, granting r+.

Comment on attachment 389012 [details]
extract xattrs before writing file data

>--- tar-1.22.sun1.orig/src/extract.c
>+++ tar-1.22.sun1/src/extract.c
>@@ -297,7 +297,6 @@ set_stat (char const *file_name,
> 
>   xattrs_acls_set(st, file_name, typeflag);
>   xattrs_selinux_set(st, file_name, typeflag);
>-  xattrs_xattrs_set(st, file_name, typeflag);

I'll need to test the above whether it's still safe.  There have been several changes in handling xattrs/selinux for symlinks etc. in the meantime.

>-		  | (old_files_option == OVERWRITE_OLD_FILES
>-		     ? O_TRUNC
>-		     : O_EXCL));
>+		 | (old_files_option == OVERWRITE_OLD_FILES
>+		   ? O_TRUNC
>+		   : O_EXCL));

The above ^^^ changes only indentation for no reason.

>@@ -817,8 +845,18 @@ extract_file (char *file_name, int typef
>     }
>   else
>     {
>+      int file_created = 0;
>+      if (set_xattr (file_name, &current_stat_info, invert_permissions,
>+    	             typeflag, &file_created))

A tab is embedded in the line above ^^^.

Comment on attachment 389012 [details]
extract xattrs before writing file data

>--- tar-1.22.sun1.orig/src/extract.c
>+++ tar-1.22.sun1/src/extract.c
>@@ -297,7 +297,6 @@ set_stat (char const *file_name,
> 
>   xattrs_acls_set(st, file_name, typeflag);
>   xattrs_selinux_set(st, file_name, typeflag);
>-  xattrs_xattrs_set(st, file_name, typeflag);

r-, the above breaks restoring of xattrs for directories.

Could we just keep the line there?

(In reply to comment #9)
> (From update of attachment 389012 [details])
> >--- tar-1.22.sun1.orig/src/extract.c
> >+++ tar-1.22.sun1/src/extract.c
> >@@ -297,7 +297,6 @@ set_stat (char const *file_name,
> > 
> >   xattrs_acls_set(st, file_name, typeflag);
> >   xattrs_selinux_set(st, file_name, typeflag);
> >-  xattrs_xattrs_set(st, file_name, typeflag);
> 
> r-, the above breaks restoring of xattrs for directories.
> 
> Could we just keep the line there?    

That would probably work - in some cases the xattrs will
be set twice, but at least for Lustre I don't think this
will cause a problem (it will silently ignore the xattrs
that cannot be changed after file creation).

Created attachment 394722 [details]
[v2] extract xattrs before writing file data

Andreas, could you please have a look at the new version of the patch?

Created attachment 395254 [details]
up2date version of the xattrs patch in Fedora

(including both of the patches above)

built as tar-1.22-16.fc14

Comment on attachment 394722 [details]
[v2] extract xattrs before writing file data

This version of the patch is obsolete, yet I'm being asked every week to review the patch and I'm not able to mark the patch '+' or remove my review request.

Comment on attachment 394722 [details]
[v2] extract xattrs before writing file data

I am canceling the review request.  Sorry about the inconveniences.