Sergey, Thank you for considering these changes, and for your response.
On Wed, Mar 04, 2009 at 11:13:12AM +0200, Sergey Poznyakoff wrote: > Thanks a lot. These are very reasonable changes. I have one doubt, > though: are you sure that a file disappearing from under the running tar > can be treated as unimportant error, It has to be treated that way when backing up live systems. > especially when in incremental backup mode? Yes, "especially" is the right word here, because when doing incremental backups tar pre-builds filelists, which makes it run into this problem more often as the delay between the time a filelist is built and the time a certain directory is finally backed up is substantial. Not ignoring this "error" renders the exit code useless. Unfortunately, there's the problem described in the lengthy comment added by the patch - there's currently no way for dump_file() to know whether a filename was explicitly given to tar (such as via the command-line) or obtained by tar during initial traversal of the directory trees. (This problem is specific to incremental backups.) I think this may be remedied with a subsequent patch, to be developed/tested/reviewed/committed at a later time if someone invests the time. It'd be best to not ignore disappearing files if those were given to tar explicitly, but to me that's not a good enough reason to block the current patch from inclusion, because right now the exit code with incremental backups of live systems is practically useless. Also, there's the question whether a disappearing file (that was not given to tar explicitly) should be treated as no error at all or as a benign error (warning printed and/or exit code set to TAREXIT_DIFFERS). In unpatched tar 1.20/1.21, this differs by context - there are a few checks for ENOENT already. The patch simply covers additional cases, which I think were previously missed, and it also treats those ENOENT's differently in different places in the code. I tried to keep this inconsistency consistent with the way it was in unpatched tar 1.20/1.21, and to apply my common sense. Maybe a review of all of those cases, not only those being patched now, is in order - but I think it should be approached as a separate task. If at a later time we choose to reflect disappearing files in the exit code in any way, then maybe another exit code should be introduced (neither TAREXIT_DIFFERS, not TAREXIT_FAILURE), or maybe a command-line option added to control whether tar should exit with TAREXIT_DIFFERS in those cases. I'd wait to see if there's any demand for that, though. On backup setups that I co-administer, we'd ignore those "errors" anyway, whereas TAREXIT_DIFFERS (which usually corresponds to the "file changed as we read it" warning from tar) results in a warning appearing inside backup reports, and TAREXIT_FAILURE results in the Subject line changed and in a Nagios alert. Finally, I find the choice of specific exit code values for TAREXIT_DIFFERS and TAREXIT_FAILURE unfortunate. In older versions of tar (such as 1.15.1 to be specific), they were swapped, which I think worked better. We need to treat TAREXIT_DIFFERS as unimportant, even though it currently corresponds to exit code 1, which is also commonly produced by system libraries on unexpected/critical errors. I think it'd be best to leave exit code 1 alone (not have tar itself ever produce it), or to un-swap these two exit codes (if compatibility with older versions of tar is a plus). (While we're at it, besides exit code 1, another one that I'd keep "reserved for libraries" is 255 aka -1.) I don't know why those exit codes were swapped (in paxutils?), so I am a bit out of context here, and obviously I am of the opinion that this issue should be discussed and maybe patched separately. Thanks again, Alexander
