From a3fdd5f029768273ae3418ef455bab5e04134d56 Mon Sep 17 00:00:00 2001 From: Mike Gelfand Date: Wed, 10 Dec 2014 18:23:11 +0000 Subject: [PATCH] #5369: Improve file allocation error checking (initial patch by g.proskurin) Additionally, * always close file descriptor on error in cached_file_open (FD leak), * only store file descriptor to tr_cached_file on success, * call ftruncate after xfsctl-based preallocation so that correct size is reported by the system. --- libtransmission/CMakeLists.txt | 1 + libtransmission/Makefile.am | 1 + libtransmission/error-types.h | 27 ++++++ libtransmission/fdlimit.c | 147 +++++++++++++++++++++------------ libtransmission/file-posix.c | 54 +++++++----- 5 files changed, 159 insertions(+), 71 deletions(-) create mode 100644 libtransmission/error-types.h diff --git a/libtransmission/CMakeLists.txt b/libtransmission/CMakeLists.txt index 5da288eb3..c50f0637d 100644 --- a/libtransmission/CMakeLists.txt +++ b/libtransmission/CMakeLists.txt @@ -70,6 +70,7 @@ endif() set(${PROJECT_NAME}_PUBLIC_HEADERS error.h + error-types.h file.h log.h makemeta.h diff --git a/libtransmission/Makefile.am b/libtransmission/Makefile.am index 712ce217a..75f1a4eb8 100644 --- a/libtransmission/Makefile.am +++ b/libtransmission/Makefile.am @@ -95,6 +95,7 @@ noinst_HEADERS = \ crypto-utils.h \ completion.h \ error.h \ + error-types.h \ fdlimit.h \ file.h \ handshake.h \ diff --git a/libtransmission/error-types.h b/libtransmission/error-types.h new file mode 100644 index 000000000..c12578f12 --- /dev/null +++ b/libtransmission/error-types.h @@ -0,0 +1,27 @@ +/* + * This file Copyright (C) 2014 Mnemosyne LLC + * + * It may be used under the GNU GPL versions 2 or 3 + * or any future license endorsed by Mnemosyne LLC. + * + * $Id$ + */ + +#ifndef TR_ERROR_TYPES_H +#define TR_ERROR_TYPES_H + +#ifdef _WIN32 + +#include + +#define TR_ERROR_IS_ENOSPC(code) ((code) == ERROR_DISK_FULL) + +#else /* _WIN32 */ + +#include + +#define TR_ERROR_IS_ENOSPC(code) ((code) == ENOSPC) + +#endif /* _WIN32 */ + +#endif /* TR_ERROR_TYPES_H */ diff --git a/libtransmission/fdlimit.c b/libtransmission/fdlimit.c index 73695d674..df45942f0 100644 --- a/libtransmission/fdlimit.c +++ b/libtransmission/fdlimit.c @@ -17,6 +17,7 @@ #include "transmission.h" #include "error.h" +#include "error-types.h" #include "fdlimit.h" #include "file.h" #include "log.h" @@ -38,58 +39,74 @@ ***/ static bool -preallocate_file_sparse (tr_sys_file_t fd, uint64_t length) +preallocate_file_sparse (tr_sys_file_t fd, uint64_t length, tr_error ** error) { - const char zero = '\0'; - bool success = false; + tr_error * my_error = NULL; - if (!length) - success = true; + if (length == 0) + return true; - if (!success) - success = tr_sys_file_preallocate (fd, length, TR_SYS_FILE_PREALLOC_SPARSE, NULL); + if (tr_sys_file_preallocate (fd, length, TR_SYS_FILE_PREALLOC_SPARSE, &my_error)) + return true; - if (!success) /* fallback: the old-style seek-and-write */ + dbgmsg ("Preallocating (sparse, normal) failed (%d): %s", my_error->code, my_error->message); + + if (!TR_ERROR_IS_ENOSPC (my_error->code)) { - /* seek requires signed offset, so length should be in mod range */ - assert (length < 0x7FFFFFFFFFFFFFFFULL); + const char zero = '\0'; + + tr_error_clear (&my_error); - success = tr_sys_file_seek (fd, length - 1, TR_SEEK_SET, NULL, NULL) && - tr_sys_file_write (fd, &zero, 1, NULL, NULL) && - tr_sys_file_truncate (fd, length, NULL); + /* fallback: the old-style seek-and-write */ + if (tr_sys_file_write_at (fd, &zero, 1, length - 1, NULL, &my_error) && + tr_sys_file_truncate (fd, length, &my_error)) + return true; + + dbgmsg ("Preallocating (sparse, fallback) failed (%d): %s", my_error->code, my_error->message); } - return success; + tr_error_propagate (error, &my_error); + return false; } static bool -preallocate_file_full (const char * filename, uint64_t length) +preallocate_file_full (tr_sys_file_t fd, uint64_t length, tr_error ** error) { - bool success = false; + tr_error * my_error = NULL; - tr_sys_file_t fd = tr_sys_file_open (filename, TR_SYS_FILE_WRITE | TR_SYS_FILE_CREATE, 0666, NULL); - if (fd != TR_BAD_SYS_FILE) + if (length == 0) + return true; + + if (tr_sys_file_preallocate (fd, length, 0, &my_error)) + return true; + + dbgmsg ("Preallocating (full, normal) failed (%d): %s", my_error->code, my_error->message); + + if (!TR_ERROR_IS_ENOSPC (my_error->code)) { - success = tr_sys_file_preallocate (fd, length, 0, NULL); + uint8_t buf[4096]; + bool success = true; + + memset (buf, 0, sizeof (buf)); + tr_error_clear (&my_error); - if (!success) /* if nothing else works, do it the old-fashioned way */ + /* fallback: the old-fashioned way */ + while (success && length > 0) { - uint8_t buf[ 4096 ]; - memset (buf, 0, sizeof (buf)); - success = true; - while (success && (length > 0)) - { - const uint64_t thisPass = MIN (length, sizeof (buf)); - uint64_t bytes_written; - success = tr_sys_file_write (fd, buf, thisPass, &bytes_written, NULL) && bytes_written == thisPass; - length -= thisPass; - } + const uint64_t thisPass = MIN (length, sizeof (buf)); + uint64_t bytes_written; + success = tr_sys_file_write (fd, buf, thisPass, &bytes_written, &my_error); + length -= bytes_written; } - tr_sys_file_close (fd, NULL); + if (success) + return true; + + dbgmsg ("Preallocating (full, fallback) failed (%d): %s", my_error->code, my_error->message); } - return success; + tr_error_propagate (error, &my_error); + return false; } /***** @@ -140,6 +157,7 @@ cached_file_open (struct tr_cached_file * o, tr_sys_path_info info; bool already_existed; bool resize_needed; + tr_sys_file_t fd = TR_BAD_SYS_FILE; tr_error * error = NULL; /* create subfolders, if any */ @@ -148,21 +166,15 @@ cached_file_open (struct tr_cached_file * o, char * dir = tr_sys_path_dirname (filename, NULL); if (!tr_sys_dir_create (dir, TR_SYS_DIR_CREATE_PARENTS, 0777, &error)) { - const int err = error->code; tr_logAddError (_("Couldn't create \"%1$s\": %2$s"), dir, error->message); - tr_error_free (error); tr_free (dir); - return err; + goto fail; } tr_free (dir); } already_existed = tr_sys_path_get_info (filename, 0, &info, NULL) && info.type == TR_SYS_PATH_IS_FILE; - if (writable && !already_existed && (allocation == TR_PREALLOCATE_FULL)) - if (preallocate_file_full (filename, file_size)) - tr_logAddDebug ("Preallocated file \"%s\"", filename); - /* we can't resize the file w/o write permissions */ resize_needed = already_existed && (file_size < info.size); writable |= resize_needed; @@ -170,14 +182,40 @@ cached_file_open (struct tr_cached_file * o, /* open the file */ flags = writable ? (TR_SYS_FILE_WRITE | TR_SYS_FILE_CREATE) : 0; flags |= TR_SYS_FILE_READ | TR_SYS_FILE_SEQUENTIAL; - o->fd = tr_sys_file_open (filename, flags, 0666, &error); + fd = tr_sys_file_open (filename, flags, 0666, &error); - if (o->fd == TR_BAD_SYS_FILE) + if (fd == TR_BAD_SYS_FILE) { - const int err = error->code; tr_logAddError (_("Couldn't open \"%1$s\": %2$s"), filename, error->message); - tr_error_free (error); - return err; + goto fail; + } + + if (writable && !already_existed && allocation != TR_PREALLOCATE_NONE) + { + bool success = false; + const char * type = NULL; + + if (allocation == TR_PREALLOCATE_FULL) + { + success = preallocate_file_full (fd, file_size, &error); + type = _("full"); + } + else if (allocation == TR_PREALLOCATE_SPARSE) + { + success = preallocate_file_sparse (fd, file_size, &error); + type = _("sparse"); + } + + assert (type != NULL); + + if (!success) + { + tr_logAddError (_("Couldn't preallocate file \"%1$s\" (%2$s, size: %3$"PRIu64"): %4$s"), + filename, type, file_size, error->message); + goto fail; + } + + tr_logAddDebug (_("Preallocated file \"%1$s\" (%2$s, size: %3$"PRIu64")"), filename, type, file_size); } /* If the file already exists and it's too large, truncate it. @@ -186,18 +224,25 @@ cached_file_open (struct tr_cached_file * o, * http://trac.transmissionbt.com/ticket/2228 * https://bugs.launchpad.net/ubuntu/+source/transmission/+bug/318249 */ - if (resize_needed && !tr_sys_file_truncate (o->fd, file_size, &error)) + if (resize_needed && !tr_sys_file_truncate (fd, file_size, &error)) { - const int err = error->code; tr_logAddError (_("Couldn't truncate \"%1$s\": %2$s"), filename, error->message); - tr_error_free (error); - return err; + goto fail; } - if (writable && !already_existed && (allocation == TR_PREALLOCATE_SPARSE)) - preallocate_file_sparse (o->fd, file_size); - + o->fd = fd; return 0; + +fail: + { + const int err = error->code; + tr_error_free (error); + + if (fd != TR_BAD_SYS_FILE) + tr_sys_file_close (fd, NULL); + + return err; + } } /*** diff --git a/libtransmission/file-posix.c b/libtransmission/file-posix.c index a5fa7ec0d..e3e0ee601 100644 --- a/libtransmission/file-posix.c +++ b/libtransmission/file-posix.c @@ -607,6 +607,8 @@ tr_sys_file_read_at (tr_sys_file_t handle, assert (handle != TR_BAD_SYS_FILE); assert (buffer != NULL || size == 0); + /* seek requires signed offset, so it should be in mod range */ + assert (offset < UINT64_MAX / 2); #ifdef HAVE_PREAD @@ -681,6 +683,8 @@ tr_sys_file_write_at (tr_sys_file_t handle, assert (handle != TR_BAD_SYS_FILE); assert (buffer != NULL || size == 0); + /* seek requires signed offset, so it should be in mod range */ + assert (offset < UINT64_MAX / 2); #ifdef HAVE_PWRITE @@ -801,15 +805,18 @@ tr_sys_file_preallocate (tr_sys_file_t handle, /* fallocate64 is always preferred, so try it first */ ret = fallocate64 (handle, 0, 0, size) != -1; + if (ret || errno == ENOSPC) + goto out; + #endif - if (!ret && (flags & TR_SYS_FILE_PREALLOC_SPARSE) == 0) + if ((flags & TR_SYS_FILE_PREALLOC_SPARSE) == 0) { int code = errno; #ifdef HAVE_XFS_XFS_H - if (!ret && platform_test_xfs_fd (handle)) + if (platform_test_xfs_fd (handle)) { xfs_flock64_t fl; @@ -819,46 +826,53 @@ tr_sys_file_preallocate (tr_sys_file_t handle, ret = xfsctl (NULL, handle, XFS_IOC_RESVSP64, &fl) != -1; + if (ret) + ret = ftruncate (handle, size) != -1; + code = errno; + + if (ret || code == ENOSPC) + goto non_sparse_out; } #endif #ifdef __APPLE__ - if (!ret) - { - fstore_t fst; + { + fstore_t fst; - fst.fst_flags = F_ALLOCATECONTIG; - fst.fst_posmode = F_PEOFPOSMODE; - fst.fst_offset = 0; - fst.fst_length = size; - fst.fst_bytesalloc = 0; + fst.fst_flags = F_ALLOCATEALL; + fst.fst_posmode = F_PEOFPOSMODE; + fst.fst_offset = 0; + fst.fst_length = size; + fst.fst_bytesalloc = 0; - ret = fcntl (handle, F_PREALLOCATE, &fst) != -1; + ret = fcntl (handle, F_PREALLOCATE, &fst) != -1; - if (ret) - ret = ftruncate (handle, size) != -1; + if (ret) + ret = ftruncate (handle, size) != -1; - code = errno; - } + code = errno; + + if (ret || code == ENOSPC) + goto non_sparse_out; + } #endif #ifdef HAVE_POSIX_FALLOCATE - if (!ret) - { - code = posix_fallocate (handle, 0, size); - ret = code == 0; - } + code = posix_fallocate (handle, 0, size); + ret = code == 0; #endif +non_sparse_out: errno = code; } +out: if (!ret) set_system_error (error, errno); -- 2.40.0