Following hunk was missing from the patch:
xattrs_acls_set(st, file_name, typeflag);
xattrs_selinux_set(st, file_name, typeflag);
- xattrs_xattrs_set(st, file_name, typeflag);
Attached patch has been updated with it.
Thanks,
Girish
On Tue, 2008-12-23 at 22:32 +0530, Girish Shilamkar wrote:
> 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
Signed-off-by: Andreas Dilger <[email protected]>
Signed-off-by: Girish Shilamkar <[email protected]>
Index: tar-1.20/src/xattrs.c
===================================================================
--- tar-1.20.orig/src/xattrs.c
+++ tar-1.20/src/xattrs.c
@@ -237,15 +237,13 @@ void xattrs_xattrs_get(struct tar_stat_i
{
size_t len = strlen (attr);
ssize_t aret = 0;
-
- if (strncmp (attr, "user.", strlen("user.")) &&
- strncmp (attr, "trusted.", strlen("trusted.")))
- goto next_attr; /* only store normal xattrs */
-
- while (((fd == -1) ?
- ((aret = getxattr (file_name, attr, val, asz)) == -1) :
- ((aret = fgetxattr (fd, attr, val, asz)) == -1)) &&
- (errno == ERANGE))
+
+ /* Archive all xattrs during creation, decide at extraction time
+ * which ones are of interest/use for the target filesystem. */
+ while (((fd == -1) ?
+ ((aret = getxattr (file_name, attr, val, asz)) == -1) :
+ ((aret = fgetxattr (fd, attr, val, asz)) == -1)) &&
+ (errno == ERANGE))
{
asz <<= 1;
val = xrealloc (val, asz);
@@ -476,6 +474,7 @@ void xattrs_xattrs_set(struct tar_stat_i
keyword += strlen("SCHILY.xattr.");
if (strncmp (keyword, "user.", strlen("user.")) &&
+ strncmp (keyword, "lustre.", strlen("lustre.")) &&
strncmp (keyword, "trusted.", strlen("trusted.")))
continue; /* don't try and set anything but normal xattrs */
Index: tar-1.20/src/extract.c
===================================================================
--- tar-1.20.orig/src/extract.c
+++ tar-1.20/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);
if (0 < same_owner_option && permstatus != INTERDIR_PERMSTATUS)
{
@@ -592,6 +591,31 @@ maybe_recoverable (char *file_name, int
}
}
+/* Restore stat extended attributes (xattr) for FILE_NAME, using information
+ given in *ST. Restore before extraction because they may affect layout.
+ If not restoring permissions, invert the
+ INVERT_PERMISSIONS bits from the file's current permissions.
+ TYPEFLAG specifies the type of the file.
+ FILE_CREATED indicates set_xattr has created the file */
+static int
+set_xattr (char const *file_name, struct tar_stat_info const *st,
+ mode_t invert_permissions, char typeflag, int *file_created)
+{
+ int status = 0;
+ int interdir_made = 0;
+
+ if ((xattrs_option >= 0) && st->xattr_map_size) {
+ mode_t mode = current_stat_info.stat.st_mode & MODE_RWX & ~ current_umask;
+
+ do
+ status = mknod (file_name, mode ^ invert_permissions, 0);
+ while (status && maybe_recoverable ((char *)file_name, &interdir_made));
+ xattrs_xattrs_set(st, file_name, typeflag);
+ *file_created = 1;
+ }
+ return(status);
+}
+
/* Fix the statuses of all directories whose statuses need fixing, and
which are not ancestors of FILE_NAME. If AFTER_LINKS is
nonzero, do this for all such directories; otherwise, stop at the
@@ -753,13 +777,17 @@ extract_dir (char *file_name, int typefl
static int
-open_output_file (char *file_name, int typeflag, mode_t mode)
+open_output_file (char *file_name, int typeflag, mode_t mode, int file_created)
{
int fd;
int openflag = (O_WRONLY | O_BINARY | O_CREAT
- | (old_files_option == OVERWRITE_OLD_FILES
- ? O_TRUNC
- : O_EXCL));
+ | (old_files_option == OVERWRITE_OLD_FILES
+ ? O_TRUNC
+ : O_EXCL));
+
+ /* File might be created in set_xattr. So clear O_EXCL to avoid open() failure */
+ if (file_created)
+ openflag = openflag & ~O_EXCL;
#if O_CTG
/* Contiguous files (on the Masscomp) have to specify the size in
@@ -817,8 +845,18 @@ extract_file (char *file_name, int typef
}
else
{
+ int file_created = 0;
+ if (set_xattr (file_name, ¤t_stat_info, invert_permissions,
+ typeflag, &file_created))
+ {
+ skip_member ();
+ open_error (file_name);
+ return 1;
+ }
+
do
- fd = open_output_file (file_name, typeflag, mode ^ invert_permissions);
+ fd = open_output_file (file_name, typeflag, mode ^ invert_permissions,
+ file_created);
while (fd < 0 && maybe_recoverable (file_name, &interdir_made));
if (fd < 0)
Index: tar-1.20/src/xheader.c
===================================================================
--- tar-1.20.orig/src/xheader.c
+++ tar-1.20/src/xheader.c
@@ -1680,6 +1680,7 @@ struct xhdr_tab const xhdr_tab[] = {
/* 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 },
