From d6084621b2aa0fde914948dfb15e8d2ed2885049 Mon Sep 17 00:00:00 2001 From: David Kocik Date: Fri, 11 Jun 2021 11:46:19 +0200 Subject: [PATCH 1/4] debug log in copy_file_linux --- src/libslic3r/utils.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index 1ac45f1b5..719d2f6fb 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -443,6 +443,8 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste ec.clear(); int err = 0; + BOOST_LOG_TRIVIAL(trace) << "copy_file_linux("< Date: Mon, 14 Jun 2021 09:42:43 +0200 Subject: [PATCH 2/4] more debug message for copy_file_linux() --- src/libslic3r/utils.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index 719d2f6fb..f89b08155 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -460,7 +460,7 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste } break; } - BOOST_LOG_TRIVIAL(trace) << "infile.fd"; + BOOST_LOG_TRIVIAL(trace) << "infile.fd " << infile.fd; struct ::stat from_stat; @@ -495,7 +495,7 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste } break; } - BOOST_LOG_TRIVIAL(trace) << "outfile.fd"; + BOOST_LOG_TRIVIAL(trace) << "outfile.fd " << outfile.fd; struct ::stat to_stat; if (::fstat(outfile.fd, &to_stat) != 0) @@ -532,7 +532,9 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste std::size_t size_to_copy = max_send_size; if (size_left < static_cast(max_send_size)) size_to_copy = static_cast(size_left); + BOOST_LOG_TRIVIAL(trace) << "sendfile " << outfile.fd << "; " << infile.fd << "; " << size_to_copy; ssize_t sz = ::sendfile(outfile.fd, infile.fd, nullptr, size_to_copy); + BOOST_LOG_TRIVIAL(trace) << sz; if (sz < 0) { err = errno; if (err == EINTR) From f60f08fc643d34a63837963fe898fefbc013e3da Mon Sep 17 00:00:00 2001 From: Vojtech Bubnik Date: Mon, 14 Jun 2021 12:18:46 +0200 Subject: [PATCH 3/4] Workaround of boost::filesystem::copy_file() incompatibility on some file systems (eCrypt ...) Should fix #4716 #6588 --- src/libslic3r/utils.cpp | 60 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index f89b08155..d4b65da2f 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -424,6 +424,53 @@ std::error_code rename_file(const std::string &from, const std::string &to) } #ifdef __linux__ +// Copied from boost::filesystem. +// Called by copy_file_linux() in case linux sendfile() API is not supported. +int copy_file_linux_read_write(int infile, int outfile, uintmax_t file_size) +{ + std::vector buf( + // Prefer the buffer to be larger than the file size so that we don't have + // to perform an extra read if the file fits in the buffer exactly. + std::clamp(file_size + (file_size < ~static_cast(0u)), + // Min and max buffer sizes are selected to minimize the overhead from system calls. + // The values are picked based on coreutils cp(1) benchmarking data described here: + // https://github.com/coreutils/coreutils/blob/d1b0257077c0b0f0ee25087efd46270345d1dd1f/src/ioblksize.h#L23-L72 + 8u * 1024u, 256u * 1024u), + 0); + +#if defined(POSIX_FADV_SEQUENTIAL) + ::posix_fadvise(infile, 0, 0, POSIX_FADV_SEQUENTIAL); +#endif + + // Don't use file size to limit the amount of data to copy since some filesystems, like procfs or sysfs, + // provide files with generated content and indicate that their size is zero or 4096. Just copy as much data + // as we can read from the input file. + while (true) { + ssize_t sz_read = ::read(infile, buf.data(), buf.size()); + if (sz_read == 0) + break; + if (sz_read < 0) { + int err = errno; + if (err == EINTR) + continue; + return err; + } + // Allow for partial writes - see Advanced Unix Programming (2nd Ed.), + // Marc Rochkind, Addison-Wesley, 2004, page 94 + for (ssize_t sz_wrote = 0; sz_wrote < sz_read;) { + ssize_t sz = ::write(outfile, buf.data() + sz_wrote, static_cast(sz_read - sz_wrote)); + if (sz < 0) { + int err = errno; + if (err == EINTR) + continue; + return err; + } + sz_wrote += sz; + } + } + return 0; +} + // Copied from boost::filesystem, to support copying a file to a weird filesystem, which does not support changing file attributes, // for example ChromeOS Linux integration or FlashAIR WebDAV. // Copied and simplified from boost::filesystem::detail::copy_file() with option = overwrite_if_exists and with just the Linux path kept, @@ -537,6 +584,19 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste BOOST_LOG_TRIVIAL(trace) << sz; if (sz < 0) { err = errno; + if (offset == 0u) { + // sendfile may fail with EINVAL if the underlying filesystem does not support it. + // See https://patchwork.kernel.org/project/linux-nfs/patch/20190411183418.4510-1-olga.kornievskaia@gmail.com/ + // https://bugzilla.redhat.com/show_bug.cgi?id=1783554. + // https://github.com/boostorg/filesystem/commit/4b9052f1e0b2acf625e8247582f44acdcc78a4ce + if (err == EINVAL || err == EOPNOTSUPP) { + err = copy_file_linux_read_write(infile.fd, outfile.fd, from_stat.st_size); + if (err < 0) + goto fail; + // Succeeded. + break; + } + } if (err == EINTR) continue; if (err == 0) From 4146f1a62e2609cf972c9b5fb00c275a80f24bf3 Mon Sep 17 00:00:00 2001 From: David Kocik Date: Tue, 15 Jun 2021 16:12:55 +0200 Subject: [PATCH 4/4] fix of #6588 - using same copy function for updating presets as for exporting gcode --- src/libslic3r/utils.cpp | 22 ---------------------- src/slic3r/Utils/PresetUpdater.cpp | 15 +++++++-------- 2 files changed, 7 insertions(+), 30 deletions(-) diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index d4b65da2f..1f3079fba 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -490,8 +490,6 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste ec.clear(); int err = 0; - BOOST_LOG_TRIVIAL(trace) << "copy_file_linux("<(max_send_size)) size_to_copy = static_cast(size_left); - BOOST_LOG_TRIVIAL(trace) << "sendfile " << outfile.fd << "; " << infile.fd << "; " << size_to_copy; ssize_t sz = ::sendfile(outfile.fd, infile.fd, nullptr, size_to_copy); - BOOST_LOG_TRIVIAL(trace) << sz; if (sz < 0) { err = errno; if (offset == 0u) { @@ -606,9 +589,6 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste offset += sz; } } - - BOOST_LOG_TRIVIAL(trace) << "sendfile loop"; - // If we created a new file with an explicitly added S_IWUSR permission, // we may need to update its mode bits to match the source file. if (to_mode != from_mode && ::fchmod(outfile.fd, from_mode) != 0) { @@ -632,8 +612,6 @@ bool copy_file_linux(const boost::filesystem::path &from, const boost::filesyste if (err != 0) goto fail_errno; - BOOST_LOG_TRIVIAL(trace) << "copy_file_linux success"; - return true; } #endif // __linux__ diff --git a/src/slic3r/Utils/PresetUpdater.cpp b/src/slic3r/Utils/PresetUpdater.cpp index 60dfe05c7..c65ec31d3 100644 --- a/src/slic3r/Utils/PresetUpdater.cpp +++ b/src/slic3r/Utils/PresetUpdater.cpp @@ -56,16 +56,15 @@ static const char *TMP_EXTENSION = ".download"; void copy_file_fix(const fs::path &source, const fs::path &target) { - static const auto perms = fs::owner_read | fs::owner_write | fs::group_read | fs::others_read; // aka 644 - BOOST_LOG_TRIVIAL(debug) << format("PresetUpdater: Copying %1% -> %2%", source, target); - - // Make sure the file has correct permission both before and after we copy over it - if (fs::exists(target)) { - fs::permissions(target, perms); + std::string error_message; + CopyFileResult cfr = copy_file(source.string(), target.string(), error_message, false); + if (cfr != CopyFileResult::SUCCESS) { + BOOST_LOG_TRIVIAL(error) << "Copying failed(" << cfr << "): " << error_message; + throw Slic3r::CriticalException(GUI::format( + _L("Copying of file %1% to %2% failed: %3%"), + source, target, error_message)); } - fs::copy_file(source, target, fs::copy_option::overwrite_if_exists); - fs::permissions(target, perms); } struct Update