extend move_storage functionality to have more flexible behavior

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

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
         ;

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()
 -------------
 

include/libtorrent/disk_io_thread.hpp

@@ -112,6 +112,7 @@ namespace libtorrent
 		int buffer_size;
 		boost::intrusive_ptr<piece_manager> 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

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<void(int, disk_io_job const&)> const& handler
 			= boost::function<void(int, disk_io_job const&)>());
 
-		void async_move_storage(std::string const& p
+		void async_move_storage(std::string const& p, int flags
 			, boost::function<void(int, disk_io_job const&)> 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

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

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
 

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);

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<std::string> 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<std::string>::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<std::string>::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<void(int, disk_io_job const&)> 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

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<storage_moved_alert>())
-			{
 				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<storage_moved_failed_alert>())
-			{
 				alerts().post_alert(storage_moved_failed_alert(get_handle(), j.error));
-			}
 		}
 	}
 

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

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<torrent_info> 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<torrent_info> 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")));	

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); }