Skip to content

Commit

Permalink
xz: Add --synchronous
Browse files Browse the repository at this point in the history
xz's default behavior is to delete the input file after successful
compression or decompression (unless writing to standard output).
If the system crashes soon after the deletion, it is possible that
the newly written file has not yet hit the disk while the previous
delete operation might have. In that case neither the original file
nor the written file is available.

The --synchronous option makes xz call fsync() on the file and possibly
the directory where the file was created. A similar option was added to
GNU gzip 1.7 in 2016. There some differences in behavior:

  - When writing to standard output and processing multiple input files,
    xz calls fsync() after every file while gzip does so only after all
    files have been processed.

  - This has no effect on "xz --list". xz doesn't sync standard output
    in --list mode but gzip does.

Portability notes:

  - <libgen.h> and dirname() should be available on all POSIX systems,
    and aren't needed on non-POSIX systems.

  - fsync() is available on all POSIX systems. The directory syncing
    could be changed to fdatasync() although at least on ext4 it
    doesn't seem to make a performance difference in xz's usage.
    fdatasync() would need a build system check to support (old)
    special cases, for example, MINIX 3.3.0 doesn't have fdatasync()
    and Solaris 10 needs -lrt.

  - On native Windows, _commit() is used to replace fsync(). Directory
    syncing isn't done and shouldn't be needed. (In Cygwin, fsync() on
    directories is a no-op.) It is known that syncing will fail if
    writing to stdout and stdout isn't a regular file.

  - DJGPP has fsync() for files. ;-)

Co-authored-by: Sebastian Andrzej Siewior <[email protected]>
Link: https://bugs.debian.org/814089
Link: https://www.mail-archive.com/[email protected]/msg00282.html
Closes: #151
Closes: #159
  • Loading branch information
Larhzu and sebastianas committed Dec 27, 2024
1 parent f95681a commit 4d8962b
Show file tree
Hide file tree
Showing 8 changed files with 211 additions and 4 deletions.
7 changes: 7 additions & 0 deletions src/xz/args.c
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
bool opt_stdout = false;
bool opt_force = false;
bool opt_keep_original = false;
bool opt_synchronous = false;
bool opt_robot = false;
bool opt_ignore_check = false;

Expand Down Expand Up @@ -217,6 +218,7 @@ parse_real(args_info *args, int argc, char **argv)
OPT_LZMA1,
OPT_LZMA2,

OPT_SYNCHRONOUS,
OPT_SINGLE_STREAM,
OPT_NO_SPARSE,
OPT_FILES,
Expand Down Expand Up @@ -249,6 +251,7 @@ parse_real(args_info *args, int argc, char **argv)
{ "force", no_argument, NULL, 'f' },
{ "stdout", no_argument, NULL, 'c' },
{ "to-stdout", no_argument, NULL, 'c' },
{ "synchronous", no_argument, NULL, OPT_SYNCHRONOUS },
{ "single-stream", no_argument, NULL, OPT_SINGLE_STREAM },
{ "no-sparse", no_argument, NULL, OPT_NO_SPARSE },
{ "suffix", required_argument, NULL, 'S' },
Expand Down Expand Up @@ -655,6 +658,10 @@ parse_real(args_info *args, int argc, char **argv)
optarg, 0, UINT64_MAX);
break;

case OPT_SYNCHRONOUS:
opt_synchronous = true;
break;

default:
message_try_help();
tuklib_exit(E_ERROR, E_ERROR, false);
Expand Down
2 changes: 1 addition & 1 deletion src/xz/args.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ typedef struct {
extern bool opt_stdout;
extern bool opt_force;
extern bool opt_keep_original;
// extern bool opt_recursive;
extern bool opt_synchronous;
extern bool opt_robot;
extern bool opt_ignore_check;

Expand Down
6 changes: 6 additions & 0 deletions src/xz/coder.c
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,12 @@ coder_normal(file_pair *pair)
if (coder_write_output(pair))
break;

// If --flush-timeout is combined with
// --synchronous, each flush is also
// synced to permanent storage.
if (opt_synchronous && io_sync_dest(pair))
break;

// Mark that we haven't seen any new input
// since the previous flush.
pair->src_has_seen_input = false;
Expand Down
138 changes: 138 additions & 0 deletions src/xz/file_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
# include <io.h>
#else
# include <poll.h>
# include <libgen.h>
static bool warn_fchown;
#endif

Expand Down Expand Up @@ -56,6 +57,10 @@ static bool warn_fchown;
# define S_ISREG(m) (((m) & _S_IFMT) == _S_IFREG)
#endif

#if defined(_WIN32) && !defined(__CYGWIN__)
# define fsync _commit
#endif

#ifndef O_BINARY
# define O_BINARY 0
#endif
Expand All @@ -64,6 +69,14 @@ static bool warn_fchown;
# define O_NOCTTY 0
#endif

#ifndef O_SEARCH
# define O_SEARCH O_RDONLY
#endif

#ifndef O_DIRECTORY
# define O_DIRECTORY 0
#endif

// Using this macro to silence a warning from gcc -Wlogical-op.
#if EAGAIN == EWOULDBLOCK
# define IS_EAGAIN_OR_EWOULDBLOCK(e) ((e) == EAGAIN)
Expand Down Expand Up @@ -450,6 +463,63 @@ io_copy_attrs(const file_pair *pair)
}


extern bool
io_sync_dest(file_pair *pair)
{
assert(pair->dest_fd != -1);

if (fsync(pair->dest_fd)) {
// If dest_fd is STDOUT_FILENO, we might be writing to
// something which cannot be synced and thus syncing will
// fail with EINVAL. Ignore the error in that case and
// return successfully.
//
// FIXME? On Windows, fsync() is actually _commit() due
// to the #define at the top of this file. _commit() sets
// errno to EBADF instead of EINVAL when stdout is console
// or nul. Ignoring EBADF wouldn't be good though because
// it might be that _commit() uses EBADF for *all* errors.
// It feels safest to leave this broken for cases where
// stdout isn't a regular file.
if (errno == EINVAL) {
assert(pair->dest_fd == STDOUT_FILENO);
return false;
}

message_error(_("%s: Synchronizing the file failed: %s"),
tuklib_mask_nonprint(pair->dest_name),
strerror(errno));
return true;
}

#ifndef TUKLIB_DOSLIKE
// If we have a file descriptor of the directory that contains
// the file, try to sync the directory.
if (pair->dir_fd != -1) {
if (fsync(pair->dir_fd)) {
message_error(_("%s: Synchronizing the directory of "
"the file failed: %s"),
tuklib_mask_nonprint(pair->dest_name),
strerror(errno));
return true;
}

// With the combination of --flush-timeout and --synchronous,
// this function may be called multiple times for the same
// file. The directory needs to be synced only once though.
//
// NOTE: This is correct but weird. A typical use case of
// --flush-timeout writes to stdout, and then dir_fd isn't
// available.
(void)close(pair->dir_fd);
pair->dir_fd = -1;
}
#endif

return false;
}


/// Opens the source file. Returns false on success, true on error.
static bool
io_open_src_real(file_pair *pair)
Expand Down Expand Up @@ -717,6 +787,9 @@ io_open_src(const char *src_name)
.dest_name = NULL,
.src_fd = -1,
.dest_fd = -1,
#ifndef TUKLIB_DOSLIKE
.dir_fd = -1,
#endif
.src_eof = false,
.src_has_seen_input = false,
.flush_needed = false,
Expand Down Expand Up @@ -819,6 +892,46 @@ io_open_dest_real(file_pair *pair)
if (pair->dest_name == NULL)
return true;

#ifndef TUKLIB_DOSLIKE
// If --synchronous is used, open also the directory
// so that we can sync it.
if (opt_synchronous) {
char *buf = xstrdup(pair->dest_name);
const char *dir_name = dirname(buf);

// O_NOCTTY and O_NONBLOCK are there in case
// O_DIRECTORY is 0 and dir_name doesn't refer
// to a directory. (We opened the source file
// already but directories might have been renamed
// after the source file was opened.)
pair->dir_fd = open(dir_name, O_SEARCH | O_DIRECTORY
| O_NOCTTY | O_NONBLOCK);
if (pair->dir_fd == -1) {
// Since we did open the source file
// successfully, we should rarely get here.
// Perhaps something has been renamed or
// had its permissions changed.
//
// In an odd case, the directory has write
// and search permissions but not read
// permission (d-wx------), and O_SEARCH is
// actually O_RDONLY. Then we would be able
// to create a new file and only the directory
// syncing would be impossible. But if user
// specifies --synchronous, let's be strict
// about it.
message_error(_("%s: Opening the directory "
"failed: %s"),
tuklib_mask_nonprint(dir_name),
strerror(errno));
free(buf);
goto error;
}

free(buf);
}
#endif

#ifdef __DJGPP__
struct stat st;
if (stat(pair->dest_name, &st) == 0) {
Expand Down Expand Up @@ -866,6 +979,10 @@ io_open_dest_real(file_pair *pair)
strerror(errno));
goto error;
}

// If using --synchronous, we could sync dir_fd now and
// close it. However, performance can be better if this is
// delayed until dest_fd has been synced in io_sync_dest().
}

if (fstat(pair->dest_fd, &pair->dest_st)) {
Expand Down Expand Up @@ -971,6 +1088,14 @@ io_open_dest_real(file_pair *pair)
return false;

error:
#ifndef TUKLIB_DOSLIKE
// io_close() closes pair->dir_fd but let's do it here anyway.
if (pair->dir_fd != -1) {
(void)close(pair->dir_fd);
pair->dir_fd = -1;
}
#endif

free(pair->dest_name);
return true;
}
Expand Down Expand Up @@ -1072,6 +1197,19 @@ io_close(file_pair *pair, bool success)
if (success && pair->dest_fd != -1 && pair->dest_fd != STDOUT_FILENO)
io_copy_attrs(pair);

// Synchronize the file (and possibly its directory) if requested.
if (opt_synchronous) {
if (success && pair->dest_fd != -1)
success = !io_sync_dest(pair);

#ifndef TUKLIB_DOSLIKE
// If io_sync_dest() was successfully called, it
// already closed dir_fd. Otherwise we do it here.
if (pair->dir_fd != -1)
(void)close(pair->dir_fd);
#endif
}

// Close the destination first. If it fails, we must not remove
// the source file!
if (io_close_dest(pair, success))
Expand Down
15 changes: 15 additions & 0 deletions src/xz/file_io.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,12 @@ typedef struct {
/// File descriptor of the target file
int dest_fd;

#ifndef TUKLIB_DOSLIKE
/// File descriptor of the directory of the target file (which is
/// also the directory of the source file)
int dir_fd;
#endif

/// True once end of the source file has been detected.
bool src_eof;

Expand Down Expand Up @@ -180,3 +186,12 @@ extern bool io_pread(file_pair *pair, io_buf *buf, size_t size, uint64_t pos);
/// \return On success, false is returned. On error, error message
/// is printed and true is returned.
extern bool io_write(file_pair *pair, const io_buf *buf, size_t size);


/// \brief Synchronizes the destination file to permanent storage
///
/// \param pair File pair having the destination file open for writing
///
/// \return On success, false is returned. On error, error message
/// is printed and true is returned.
extern bool io_sync_dest(file_pair *pair);
8 changes: 6 additions & 2 deletions src/xz/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -1069,7 +1069,8 @@ message_help(bool long_help)
e |= tuklib_wrapf(stdout, &wrap2,
" --block-size=%s\v%s\r"
" --block-list=%s\v%s\r"
" --flush-timeout=%s\v%s",
" --flush-timeout=%s\v%s\r"
" --synchronous\v%s",
_("SIZE"),
W_("start a new .xz block after every SIZE bytes "
"of input; use this to set the block size "
Expand All @@ -1084,7 +1085,10 @@ message_help(bool long_help)
W_("when compressing, if more than NUM "
"milliseconds has passed since the previous "
"flush and reading more input would block, "
"all pending data is flushed out"));
"all pending data is flushed out"),
W_("synchronize the output file to the storage device "
"after successful compression, "
"decompression, or flushing"));

e |= tuklib_wrapf(stdout, &wrap2,
" --memlimit-compress=%1$s\n"
Expand Down
4 changes: 3 additions & 1 deletion src/xz/sandbox.c
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,9 @@ sandbox_init(void)
// rights because files are created with open() using O_EXCL and
// without O_TRUNC.
//
// LANDLOCK_ACCESS_FS_READ_DIR is included here to get a clear error
// LANDLOCK_ACCESS_FS_READ_DIR is required when using --synchronous.
//
// LANDLOCK_ACCESS_FS_READ_DIR is also helpful to show a clear error
// message if xz is given a directory name. Without this permission
// the message would be "Permission denied" but with this permission
// it's "Is a directory, skipping". It could be worked around with
Expand Down
35 changes: 35 additions & 0 deletions src/xz/xz.1
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,41 @@ is unsuitable for decompressing the stream in real time due to how
.B xz
does buffering.
.TP
.B \-\-synchronous
After successful compression or decompression,
synchronize the target file to the storage device.
The synchronization is done before possibly removing the source file,
thus at least one of the two files should exist after a system crash.
.IP ""
On POSIX systems,
also the directory containing the file is synchronized
.BR "except when writing to standard output".
If the directory isn't synchronized,
it's possible that the directory entry for the file won't exist after a crash
even if the file itself has been synchronized.
A workaround is to create an empty file first and synchronize the directory.
An example using
.BR sync (1)
from GNU coreutils:
.RS
.IP ""
.nf
.ft CR
touch foo/bar/target.xz
sync foo/bar
sometool | xz --synchronous > foo/bar/target.xz
.ft R
.fi
.RE
.IP ""
If
.B \-\-synchronous
is used together with
.BR \-\-flush\-timeout ,
the file is also synchronized after every flush.
This way all the data until the most recent flush
should be recoverable after a system crash.
.TP
.BI \-\-memlimit\-compress= limit
Set a memory usage limit for compression.
If this option is specified multiple times,
Expand Down

0 comments on commit 4d8962b

Please sign in to comment.