Re: [Bug-tar] adding ACLs when there are none
Hello Linda, On Tuesday, March 04, 2014 15:38:22 Linda A. Walsh wrote: Pavel Raiskup wrote: When --acls option is on (regardless of tarball contents or tarball format), we should explicitly set OR delete default ACLs for extracted directories. Prior to this update, we always created arbitrary default ACLs based standard file permissions. Why would tar create any acls if there are none in the source tar? I was not clear in the sentence probably, please read that like: Set (if these default ACLs are also in archive) OR delete them (because these may already be inherited from parent directory default ACLs). -- When you pass --acls, you want to have extracted files exactly as stored in the archive (from the ACLs perspective). If I read correctly, in ACLs compatbile world — no-ACLs means that you know that you don't want ACLs to be used. I saw someone else have a similar complaint about acls being created when the tar didn't have acls but the --acls option was used. Isn't that exactly the reason for 7fe7adcbb9 commit? Prior to that commit, *default* ACLs were created. From `man acl`: OBJECT CREATION AND DEFAULT ACLs The access ACL of a file object is initialized when the object is created with any of the creat(), mkdir(), mknod(), mkfifo(), or open() functions. If a default ACL is associated with a directory, the mode parameter to the functions creating file objects and the default ACL of the directory are used to determine the ACL of the new object: I wouldn't want a non-acl containing tarball to overwrite or change default acls in a directory that already exists. If I said --acl=reset or similar, that might be a desirable feature. These are IMO candidates for omitting --acls option, no? Or could you give an example? What *exactly* do you expect the --acls should behave by default? Combine existing acls in parent directory (default acls) with the stored in archive? Thanks, Pavel
Re: [Bug-tar] seek_hole proposal
Pavel Raiskup prais...@redhat.com wrote: Hello all, I am trying to prepare patch which would reuse lseek's SEEK_HOLE/SEEK_DATA, the [v1] is attached. Some info: Note that after discussion [1] I still think that existing ST_IS_SPARSE macro is better for file-sparseness detection than using SEEK_HOLE (not worth having additional syscalls open~seek~close). Your code is not compatible to the SEEK_HOLE interface. A file is sparse in case that pathconf()/fpathconf(f, _PC_MIN_HOLE_SIZE) return a positive number and lseek(f, (off_t)0, SEEK_HOLE) returns a number stat.st_size. see man pathconf: 11. If a filesystem supports the reporting of holes (see lseek(2), pathconf() and fpathconf() return a positive number that represents the minimum hole size returned in bytes. The offsets of holes returned will be aligned to this same value. A spe- cial value of 1 is returned if the filesystem does not specify the minimum hole size but still reports holes. In other cases, the file stil may be sparse, but the filesystem does not support SEEK_HOLE. I e.g. doubt that Linux correctly implements SEEK_HOLE for NFS. Jörg -- EMail:jo...@schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin j...@cs.tu-berlin.de(uni) joerg.schill...@fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/ URL: http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily
Re: [Bug-tar] seek_hole proposal [v2]
On Wednesday, March 05, 2014 12:14:17 Joerg Schilling wrote:
Pavel Raiskup prais...@redhat.com wrote:
Note that after discussion [1] I still think that existing ST_IS_SPARSE
macro is better for file-sparseness detection than using SEEK_HOLE (not
worth having additional syscalls open~seek~close).
Your code is not compatible to the SEEK_HOLE interface. A file is sparse in
case that pathconf()/fpathconf(f, _PC_MIN_HOLE_SIZE) return a positive number
and lseek(f, (off_t)0, SEEK_HOLE) returns a number stat.st_size.
Yes, apart from pathconf, the patch-v1 contained bug - I should react on
hole_offset, not data_offset. And there was yet another bug - we need to
reposition the file offset to the beginning in this case (no FS support);
so there needs to be done this change:
if (offset == 0 /* first loop */
- data_offset == st-stat.st_size)
-return false;
+ data_offset == 0
+ hole_offset == st-stat.st_size)
+{
+ lseek (fd, 0, SEEK_SET);
+ return false;
+}
Joerg, thanks for pointing that out.
see man pathconf:
[...]
In other cases, the file stil may be sparse, but the filesystem does not
support SEEK_HOLE. I e.g. doubt that Linux correctly implements SEEK_HOLE
for NFS.
Correct, Linux's NFS returns just the virtual hole at the end of sparse
file, which is now OK - we would still fallback to raw hole detection.
Fixed patch v2 attached.
Pavel
From 0563cfd261f6c47f25924ae6fef542230fdf2794 Mon Sep 17 00:00:00 2001
From: Pavel Raiskup prais...@redhat.com
Date: Sun, 23 Feb 2014 13:12:54 +0100
Subject: [PATCH] tar: use SEEK_HOLE for hole detection
Reuse the SEEK_HOLE/SEEK_DATA feature of lseek when possible.
This makes the sparse file archivation to be quite faster as tar
does not need to perform additional read of whole file in order to
detect file sparse map.
This lseek feature is not yet fully POSIX but it is fairly widely
implemented these days.
Also implement --hole-detection option for proper method
selection.
* src/common.h (HOLE_DETECTION_RAW, HOLE_DETECTION_SEEK)
(HOLE_DETECTION_ALL, hole_detection): New constants and variable.
* src/sparse.c (sparse_scan_file_wholesparse): New function as a
method for detecting sparse files without any data.
(sparse_scan_file_raw): Renamed from sparse_scan_file, removed the
completely-sparse detection if-branch.
(sparse_scan_file_seek): Implements method for hole detection
using lseek.
(sparse_scan_file): Reimplemented function as a wrapper for all
methods.
* src/tar.c (HOLE_DETECTION_OPTION): New option.
(parse_opt): Handle new --hole-detection option.
* tests/sparse02.at: Use --hole-detection=raw as the seek method
creates little bit bigger archives causing test to fail.
* tests/checkseekhole.c: SEEK_HOLE detection helper.
* tests/sparsemv.at: Likewise.
* tests/sparsemvp.at: Likewise.
* tests/sparse05.at: New test-case.
* tests/testsuite.at: Cover new testcase.
* tests/Makefile.am: Likewise.
* doc/tar.1: Document.
* doc/tar.texi: Likewise.
---
doc/tar.1 | 6 ++
doc/tar.texi | 70 +++-
src/common.h | 6 ++
src/sparse.c | 173 --
src/tar.c | 16 +
tests/Makefile.am | 5 +-
tests/checkseekhole.c | 90 ++
tests/sparse02.at | 2 +-
tests/sparse05.at | 56
tests/sparsemv.at | 1 +
tests/sparsemvp.at| 1 +
tests/testsuite.at| 16 +
12 files changed, 392 insertions(+), 50 deletions(-)
create mode 100644 tests/checkseekhole.c
create mode 100644 tests/sparse05.at
diff --git a/doc/tar.1 b/doc/tar.1
index b33f55b..d2898b6 100644
--- a/doc/tar.1
+++ b/doc/tar.1
@@ -259,6 +259,12 @@ When listing or extracting, the actual contents of \fIFILE\fR is not
inspected, it is needed only due to syntactical requirements. It is
therefore common practice to use \fB/dev/null\fR in its place.
.TP
+\fB\-\-hole\-detection\fR=\fIMETHOD\fR
+Use method to detect holes in sparse files. This option implies
+\fB\-\-sparse\fR. Currently there are \fIseek\fR and \fIraw\fR methods
+implemented. Default is \fIseek\fR with fallback to \fIraw\fR when not
+applicable.
+.TP
\fB\-G\fR, \fB\-\-incremental\fR
Handle old GNU-format incremental backups.
.TP
diff --git a/doc/tar.texi b/doc/tar.texi
index 9bb5a83..0e894f9 100644
--- a/doc/tar.texi
+++ b/doc/tar.texi
@@ -2748,6 +2748,13 @@ they refer to, instead of creating usual hard link members.
@command{tar} will print out a short message summarizing the operations and
options to @command{tar} and exit. @xref{help}.
+@opsummary{hole-detection}
+@item --hole-detection=@var{method}
+Use method to detect holes in sparse files. This option implies
+@option{--sparse}. Currently there are @var{seek} and @var{raw} methods
+implemented. Default is @var{seek} with fallback to @var{raw} when not
+applicable. @xref{sparse}.
+
@opsummary{ignore-case}
Re: [Bug-tar] adding ACLs when there are none
Pavel Raiskup wrote: These are IMO candidates for omitting --acls option, no? Or could you give an example? What *exactly* do you expect the --acls should behave by default? Combine existing acls in parent directory (default acls) with the stored in archive? Leaving default acls in the dir -- case in in point... If the SetGid bit is set on a directory on linux, it is usually propagated to lower lower level dirs to permit a particular type of access to be propagated to lower level files and dirs. If a default acl is set on a dir I see it used in the same way.
Re: [Bug-tar] adding ACLs when there are none
Pavel Raiskup wrote: Or could you give an example? What *exactly* do you expect the --acls should behave by default? Combine existing acls in parent directory (default acls) with the stored in archive? Thanks, Pavel - Sorry, didn't finish that thought...If there are default acls set in the tar, they would replace such default acls that are present, but undefined ACLs in the tar wouldn't overwrite set acl's that propagate from the parent.
Re: [Bug-tar] adding ACLs when there are none
On Wednesday, March 05, 2014 05:06:06 Linda A. Walsh wrote: Pavel Raiskup wrote: Or could you give an example? What *exactly* do you expect the --acls should behave by default? Combine existing acls in parent directory (default acls) with the stored in archive? Thanks, Pavel - If the SetGid bit is set on a directory on linux, it is usually propagated to lower lower level dirs to permit a particular type of access to be propagated to lower level files and dirs. The _default_ seems to be matter of taste. Looking at how the SetGid works in GNU tar, the bit is inherited from parent by default (no additional option passed). But when you specify '-p' option, then the bit is not inherited as you want (the permissions stored in archive have a priority). I would rather take --acls similarly to -p in this regard. If a default acl is set on a dir I see it used in the same way. Sorry, didn't finish that thought...If there are default acls set in the tar, they would replace such default acls that are present, but undefined ACLs in the tar wouldn't overwrite set acl's that propagate from the parent. And what if you want to combine default ACLs somehow? Also what if you want the inherited (not in archive-stored) ACLs to affect also all extracted files? I agree that you may want to specify exact behavior. The --acls could take optional arguments like --acls=inherit-defaults (your scenario). Possibly --acls=combine (in future) to merge as much as possible (little bit more work here would be needed). Thoughts? Pavel
Re: [Bug-tar] adding ACLs when there are none
Pavel Raiskup prais...@redhat.com wrote: On Wednesday, March 05, 2014 05:06:06 Linda A. Walsh wrote: Pavel Raiskup wrote: Or could you give an example? What *exactly* do you expect the --acls should behave by default? Combine existing acls in parent directory (default acls) with the stored in archive? Thanks, Pavel - If the SetGid bit is set on a directory on linux, it is usually propagated to lower lower level dirs to permit a particular type of access to be propagated to lower level files and dirs. The _default_ seems to be matter of taste. Looking at how the SetGid works in GNU tar, the bit is inherited from parent by default (no additional option passed). But when you specify '-p' option, then the bit is not inherited as you want (the permissions stored in archive have a priority). I would rather take --acls similarly to -p in this regard. This does nit seem to be correct. The BSD sgroup bit on directories only propagates to directories and not to files. The default acls propagate to files also. Note that the bevior in star has been defined in 2001 after talking to various people. gtar should behave similar. Jörg -- EMail:jo...@schily.isdn.cs.tu-berlin.de (home) Jörg Schilling D-13353 Berlin j...@cs.tu-berlin.de(uni) joerg.schill...@fokus.fraunhofer.de (work) Blog: http://schily.blogspot.com/ URL: http://cdrecord.berlios.de/private/ ftp://ftp.berlios.de/pub/schily
