From 982a14c2e950763a9a188d83a36c28de23912ff1 Mon Sep 17 00:00:00 2001 From: Arvid Norberg Date: Thu, 9 May 2013 02:50:16 +0000 Subject: [PATCH] extend move_storage functionality to have more flexible behavior --- ChangeLog | 1 + bindings/python/src/torrent_handle.cpp | 8 +-- docs/manual.rst | 33 ++++++++-- include/libtorrent/disk_io_thread.hpp | 1 + include/libtorrent/storage.hpp | 40 ++++++++++-- include/libtorrent/torrent.hpp | 3 +- include/libtorrent/torrent_handle.hpp | 4 +- src/disk_io_thread.cpp | 2 +- src/storage.cpp | 89 ++++++++++++++++++-------- src/torrent.cpp | 12 ++-- src/torrent_handle.cpp | 8 +-- test/test_storage.cpp | 10 +-- test/test_transfer.cpp | 4 +- 13 files changed, 152 insertions(+), 63 deletions(-) diff --git a/ChangeLog b/ChangeLog index 60f224788..a7b7c3d19 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,4 @@ + * make move_storage more generic to allow both overwriting files as well as taking existing ones * fix choking issue at high upload rates * optimized rate limiter * make disk cache pool allocator configurable diff --git a/bindings/python/src/torrent_handle.cpp b/bindings/python/src/torrent_handle.cpp index 638a564c6..314e8eddc 100644 --- a/bindings/python/src/torrent_handle.cpp +++ b/bindings/python/src/torrent_handle.cpp @@ -339,11 +339,11 @@ void bind_torrent_handle() int (torrent_handle::*piece_priority0)(int) const = &torrent_handle::piece_priority; void (torrent_handle::*piece_priority1)(int, int) const = &torrent_handle::piece_priority; - void (torrent_handle::*move_storage0)(std::string const&) const = &torrent_handle::move_storage; + void (torrent_handle::*move_storage0)(std::string const&, int flags) const = &torrent_handle::move_storage; void (torrent_handle::*rename_file0)(int, std::string const&) const = &torrent_handle::rename_file; #if TORRENT_USE_WSTRING - void (torrent_handle::*move_storage1)(std::wstring const&) const = &torrent_handle::move_storage; + void (torrent_handle::*move_storage1)(std::wstring const&, int flags) const = &torrent_handle::move_storage; void (torrent_handle::*rename_file1)(int, std::wstring const&) const = &torrent_handle::rename_file; #endif @@ -449,13 +449,13 @@ void bind_torrent_handle() .def("set_max_connections", _(&torrent_handle::set_max_connections)) .def("max_connections", _(&torrent_handle::max_connections)) .def("set_tracker_login", _(&torrent_handle::set_tracker_login)) - .def("move_storage", _(move_storage0)) + .def("move_storage", _(move_storage0), (arg("path"), arg("flags") = 0)) .def("info_hash", _(&torrent_handle::info_hash)) .def("force_recheck", _(&torrent_handle::force_recheck)) .def("rename_file", _(rename_file0)) .def("set_ssl_certificate", &torrent_handle::set_ssl_certificate, (arg("cert"), arg("private_key"), arg("dh_params"), arg("passphrase")="")) #if TORRENT_USE_WSTRING - .def("move_storage", _(move_storage1)) + .def("move_storage", _(move_storage1), (arg("path"), arg("flags") = 0)) .def("rename_file", _(rename_file1)) #endif ; diff --git a/docs/manual.rst b/docs/manual.rst index 0bafadcbe..16d44e874 100644 --- a/docs/manual.rst +++ b/docs/manual.rst @@ -2431,8 +2431,8 @@ Its declaration looks like this:: bool set_metadata(char const* buf, int size) const; - void move_storage(std::string const& save_path) const; - void move_storage(std::wstring const& save_path) const; + void move_storage(std::string const& save_path, int flags = 0) const; + void move_storage(std::wstring const& save_path, int flags = 0) const; void rename_file(int index, std::string) const; void rename_file(int index, std::wstring) const; storage_interface* get_storage_impl() const; @@ -2615,12 +2615,12 @@ move_storage() :: - void move_storage(std::string const& save_path) const; - void move_storage(std::wstring const& save_path) const; + void move_storage(std::string const& save_path, int flags = 0) const; + void move_storage(std::wstring const& save_path, int flags = 0) const; Moves the file(s) that this torrent are currently seeding from or downloading to. If the given ``save_path`` is not located on the same drive as the original save path, -The files will be copied to the new drive and removed from their original location. +the files will be copied to the new drive and removed from their original location. This will block all other disk IO, and other torrents download and upload rates may drop while copying the file. @@ -2629,6 +2629,29 @@ Once the operation completes, the ``storage_moved_alert`` is generated, with the path as the message. If the move fails for some reason, ``storage_moved_failed_alert`` is generated instead, containing the error message. +The ``flags`` argument determines the behavior of the copying/moving of the files +in the torrent. They are defined in ``include/libtorrent/storage.hpp``: + + * ``always_replace_files`` = 0 + * ``fail_if_exist`` = 1 + * ``dont_replace`` = 2 + +``always_replace_files`` is the default and replaces any file that exist in both the +source directory and the target directory. + +``fail_if_exist`` first check to see that none of the copy operations would cause an +overwrite. If it would, it will fail. Otherwise it will proceed as if it was in +``always_replace_files`` mode. Note that there is an inherent race condition here. +If the files in the target directory appear after the check but before the copy +or move completes, they will be overwritten. + +The intention is that a client may use this as a probe, and if it fails, ask the user +which mode to use. The client may then re-issue the ``move_storage`` call with one +of the other modes. + +``dont_replace`` always takes the existing file in the target directory, if there is +one. The source files will still be removed in that case. + rename_file() ------------- diff --git a/include/libtorrent/disk_io_thread.hpp b/include/libtorrent/disk_io_thread.hpp index 74fb5ef83..52adc87d7 100644 --- a/include/libtorrent/disk_io_thread.hpp +++ b/include/libtorrent/disk_io_thread.hpp @@ -112,6 +112,7 @@ namespace libtorrent int buffer_size; boost::intrusive_ptr storage; // arguments used for read and write + // piece is used as flags for move_storage int piece, offset; // used for move_storage and rename_file. On errors, this is set // to the error message diff --git a/include/libtorrent/storage.hpp b/include/libtorrent/storage.hpp index b7bc6f9de..23268d5b3 100644 --- a/include/libtorrent/storage.hpp +++ b/include/libtorrent/storage.hpp @@ -128,8 +128,12 @@ namespace libtorrent // is not in a sparse region, start itself is returned virtual int sparse_end(int start) const { return start; } - // non-zero return value indicates an error - virtual bool move_storage(std::string const& save_path) = 0; + // returns: + // no_error = 0, + // need_full_check = -1, + // fatal_disk_error = -2, + // file_exist = -4, + virtual int move_storage(std::string const& save_path, int flags) = 0; // verify storage dependent fast resume entries virtual bool verify_resume_data(lazy_entry const& rd, error_code& error) = 0; @@ -197,7 +201,7 @@ namespace libtorrent bool release_files(); bool delete_files(); bool initialize(bool allocate_files); - bool move_storage(std::string const& save_path); + int move_storage(std::string const& save_path, int flags); int read(char* buf, int slot, int offset, int size); int write(char const* buf, int slot, int offset, int size); int sparse_end(int start) const; @@ -271,7 +275,7 @@ namespace libtorrent bool release_files() { return false; } bool delete_files() { return false; } bool initialize(bool) { return false; } - bool move_storage(std::string const&) { return true; } + int move_storage(std::string const&, int flags) { return 0; } int read(char*, int, int, int size) { return size; } int write(char const*, int, int, int size) { return size; } size_type physical_offset(int, int) { return 0; } @@ -286,6 +290,27 @@ namespace libtorrent int m_piece_size; }; + // flags for async_move_storage + enum move_flags_t + { + // replace any files in the destination when copying + // or moving the storage + always_replace_files = 0, + + // if any files that we want to copy exist in the destination + // exist, fail the whole operation and don't perform + // any copy or move. There is an inherent race condition + // in this mode. The files are checked for existence before + // the operation starts. In between the check and performing + // the copy, the destination files may be created, in which + // case they are replaced. + fail_if_exist = 1, + + // if any file exist in the target, take those files instead + // of the ones we may have in the source. + dont_replace = 2, + }; + struct disk_io_thread; class TORRENT_EXTRA_EXPORT piece_manager @@ -356,7 +381,7 @@ namespace libtorrent boost::function const& handler = boost::function()); - void async_move_storage(std::string const& p + void async_move_storage(std::string const& p, int flags , boost::function const& handler); void async_save_resume_data( @@ -368,7 +393,8 @@ namespace libtorrent no_error = 0, need_full_check = -1, fatal_disk_error = -2, - disk_check_aborted = -3 + disk_check_aborted = -3, + file_exist = -4, }; storage_interface* get_storage_impl() { return m_storage.get(); } @@ -457,7 +483,7 @@ namespace libtorrent int rename_file_impl(int index, std::string const& new_filename) { return m_storage->rename_file(index, new_filename); } - int move_storage_impl(std::string const& save_path); + int move_storage_impl(std::string const& save_path, int flags); int allocate_slot_for_piece(int piece_index); #if defined TORRENT_DEBUG && !defined TORRENT_DISABLE_INVARIANT_CHECKS diff --git a/include/libtorrent/torrent.hpp b/include/libtorrent/torrent.hpp index 25ce46ee4..acdd68405 100644 --- a/include/libtorrent/torrent.hpp +++ b/include/libtorrent/torrent.hpp @@ -775,7 +775,8 @@ namespace libtorrent void set_max_connections(int limit, bool state_update = true); int max_connections() const { return m_max_connections; } - void move_storage(std::string const& save_path); + // flags are defined in storage.hpp + void move_storage(std::string const& save_path, int flags); // renames the file with the given index to the new name // the name may include a directory path diff --git a/include/libtorrent/torrent_handle.hpp b/include/libtorrent/torrent_handle.hpp index 17d558f18..869c7518b 100644 --- a/include/libtorrent/torrent_handle.hpp +++ b/include/libtorrent/torrent_handle.hpp @@ -398,11 +398,11 @@ namespace libtorrent , std::string const& password) const; // post condition: save_path() == save_path if true is returned - void move_storage(std::string const& save_path) const; + void move_storage(std::string const& save_path, int flags = 0) const; void rename_file(int index, std::string const& new_name) const; #if TORRENT_USE_WSTRING - void move_storage(std::wstring const& save_path) const; + void move_storage(std::wstring const& save_path, int flags = 0) const; void rename_file(int index, std::wstring const& new_name) const; #endif diff --git a/src/disk_io_thread.cpp b/src/disk_io_thread.cpp index 86dbd0383..21a1a4691 100644 --- a/src/disk_io_thread.cpp +++ b/src/disk_io_thread.cpp @@ -2240,7 +2240,7 @@ namespace libtorrent m_log << log_time() << " move" << std::endl; #endif TORRENT_ASSERT(j.buffer == 0); - ret = j.storage->move_storage_impl(j.str); + ret = j.storage->move_storage_impl(j.str, j.piece); if (ret != 0) { test_error(j); diff --git a/src/storage.cpp b/src/storage.cpp index 0871dc5a3..2b4fe6b1d 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -749,21 +749,14 @@ namespace libtorrent } // returns true on success - bool default_storage::move_storage(std::string const& sp) + int default_storage::move_storage(std::string const& sp, int flags) { + int ret = piece_manager::no_error; std::string save_path = complete(sp); + // check to see if any of the files exist error_code ec; - file_status s; - stat_file(save_path, &s, ec); - if (ec == boost::system::errc::no_such_file_or_directory) - create_directories(save_path, ec); - else if (ec) - return false; - m_pool.release(this); - - bool ret = true; std::set to_move; file_storage const& f = files(); @@ -774,6 +767,40 @@ namespace libtorrent to_move.insert(to_move.begin(), split); } + file_status s; + if (flags == fail_if_exist) + { + stat_file(save_path, &s, ec); + if (ec != boost::system::errc::no_such_file_or_directory) + { + // the directory exists, check all the files + for (std::set::const_iterator i = to_move.begin() + , end(to_move.end()); i != end; ++i) + { + std::string new_path = combine_path(save_path, *i); + stat_file(new_path, &s, ec); + if (ec != boost::system::errc::no_such_file_or_directory) + return piece_manager::file_exist; + } + } + } + + ec.clear(); + stat_file(save_path, &s, ec); + if (ec == boost::system::errc::no_such_file_or_directory) + { + ec.clear(); + create_directories(save_path, ec); + } + + if (ec) + { + set_error(save_path, ec); + return piece_manager::fatal_disk_error; + } + + m_pool.release(this); + for (std::set::const_iterator i = to_move.begin() , end(to_move.end()); i != end; ++i) { @@ -781,24 +808,34 @@ namespace libtorrent std::string new_path = combine_path(save_path, *i); rename(old_path, new_path, ec); - if (ec && ec != boost::system::errc::no_such_file_or_directory) + if (ec) { - error_code ec; - recursive_copy(old_path, new_path, ec); - if (ec) + if (flags == dont_replace && ec == boost::system::errc::file_exists) { - set_error(old_path, ec); - ret = false; + if (ret == piece_manager::no_error) ret = piece_manager::need_full_check; + continue; } - else + + if (ec != boost::system::errc::no_such_file_or_directory) { - remove_all(old_path, ec); + error_code ec; + recursive_copy(old_path, new_path, ec); + if (ec) + { + set_error(old_path, ec); + ret = piece_manager::fatal_disk_error; + } + else + { + remove_all(old_path, ec); + } + break; } - break; } } - if (ret) m_save_path = save_path; + if (ret == piece_manager::no_error || ret == piece_manager::need_full_check) + m_save_path = save_path; return ret; } @@ -1497,13 +1534,14 @@ ret: m_io_thread.add_job(j, handler); } - void piece_manager::async_move_storage(std::string const& p + void piece_manager::async_move_storage(std::string const& p, int flags , boost::function const& handler) { disk_io_job j; j.storage = this; j.action = disk_io_job::move_storage; j.str = p; + j.piece = flags; m_io_thread.add_job(j, handler); } @@ -1664,14 +1702,15 @@ ret: return ph.h.final(); } - int piece_manager::move_storage_impl(std::string const& save_path) + int piece_manager::move_storage_impl(std::string const& save_path, int flags) { - if (m_storage->move_storage(save_path)) + int ret = m_storage->move_storage(save_path, flags); + + if (ret == no_error || ret == need_full_check) { m_save_path = complete(save_path); - return 0; } - return -1; + return ret; } void piece_manager::write_resume_data(entry& rd) const diff --git a/src/torrent.cpp b/src/torrent.cpp index 187c8b897..0d05b1eec 100644 --- a/src/torrent.cpp +++ b/src/torrent.cpp @@ -6517,7 +6517,7 @@ namespace libtorrent return true; } - void torrent::move_storage(std::string const& save_path) + void torrent::move_storage(std::string const& save_path, int flags) { TORRENT_ASSERT(m_ses.is_network_thread()); INVARIANT_CHECK; @@ -6529,7 +6529,7 @@ namespace libtorrent #else std::string const& path = save_path; #endif - m_owning_storage->async_move_storage(path + m_owning_storage->async_move_storage(path, flags , boost::bind(&torrent::on_storage_moved, shared_from_this(), _1, _2)); } else @@ -6551,20 +6551,18 @@ namespace libtorrent { TORRENT_ASSERT(m_ses.is_network_thread()); - if (ret == 0) + if (ret == piece_manager::no_error || ret == piece_manager::need_full_check) { if (alerts().should_post()) - { alerts().post_alert(storage_moved_alert(get_handle(), j.str)); - } m_save_path = j.str; + if (ret == piece_manager::need_full_check) + force_recheck(); } else { if (alerts().should_post()) - { alerts().post_alert(storage_moved_failed_alert(get_handle(), j.error)); - } } } diff --git a/src/torrent_handle.cpp b/src/torrent_handle.cpp index 5b3dd586b..f41d725e0 100644 --- a/src/torrent_handle.cpp +++ b/src/torrent_handle.cpp @@ -338,20 +338,20 @@ namespace libtorrent } void torrent_handle::move_storage( - std::string const& save_path) const + std::string const& save_path, int flags) const { INVARIANT_CHECK; - TORRENT_ASYNC_CALL1(move_storage, save_path); + TORRENT_ASYNC_CALL2(move_storage, save_path, flags); } #if TORRENT_USE_WSTRING void torrent_handle::move_storage( - std::wstring const& save_path) const + std::wstring const& save_path, int flags) const { INVARIANT_CHECK; std::string utf8; wchar_utf8(save_path, utf8); - TORRENT_ASYNC_CALL1(move_storage, utf8); + TORRENT_ASYNC_CALL2(move_storage, utf8, flags); } void torrent_handle::rename_file(int index, std::wstring const& new_name) const diff --git a/test/test_storage.cpp b/test/test_storage.cpp index f66ee1d30..131b0e26a 100644 --- a/test/test_storage.cpp +++ b/test/test_storage.cpp @@ -198,8 +198,8 @@ struct test_storage : storage_interface virtual int sparse_end(int start) const { return start; } - virtual bool move_storage(std::string const& save_path) - { return false; } + virtual int move_storage(std::string const& save_path, int flags) + { return 0; } virtual bool verify_resume_data(lazy_entry const& rd, error_code& error) { return false; } @@ -572,8 +572,8 @@ void run_storage_tests(boost::intrusive_ptr info TEST_CHECK(exists(combine_path(test_path, "temp_storage"))); done = false; - pm->async_move_storage(combine_path(test_path, "temp_storage2") - , boost::bind(on_move_storage, _1, &done, _2, combine_path(test_path, "temp_storage2"))); + pm->async_move_storage(combine_path(test_path, "temp_storage2"), 0 + , boost::bind(&on_move_storage, _1, &done, _2, combine_path(test_path, "temp_storage2"))); run_until(ios, done); if (fs.num_files() > 1) @@ -584,7 +584,7 @@ void run_storage_tests(boost::intrusive_ptr info TEST_CHECK(exists(combine_path(test_path, combine_path("temp_storage2", "part0")))); done = false; - pm->async_move_storage(test_path, boost::bind(on_move_storage, _1, &done, _2, test_path)); + pm->async_move_storage(test_path, 0, boost::bind(&on_move_storage, _1, &done, _2, test_path)); run_until(ios, done); TEST_CHECK(exists(combine_path(test_path, "part0"))); diff --git a/test/test_transfer.cpp b/test/test_transfer.cpp index 8eec9796a..152386649 100644 --- a/test/test_transfer.cpp +++ b/test/test_transfer.cpp @@ -183,8 +183,8 @@ struct test_storage : storage_interface virtual int sparse_end(int start) const { return m_lower_layer->sparse_end(start); } - virtual bool move_storage(std::string const& save_path) - { return m_lower_layer->move_storage(save_path); } + virtual int move_storage(std::string const& save_path, int flags) + { return m_lower_layer->move_storage(save_path, flags); } virtual bool verify_resume_data(lazy_entry const& rd, error_code& error) { return m_lower_layer->verify_resume_data(rd, error); }