qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
28.67k stars 4.01k forks source link

Lost 10GiB of data with no warning simply by changing download location #7146

Open ranolfi opened 7 years ago

ranolfi commented 7 years ago

Using qBittorrent v3.3.13 in Arch Linux (4.12 kernel) with Cinnamon 3.4.3.

libtorrent 1.1.3.0; Qt 5.8.0.

I had my torrents set for downloading in the /media/docs directory, which was a mountpoint for a secondary hard drive. I wanted to rename the mountpoint to Docs, and did so by doing:

  1. umount /media/docs
  2. rmdir /media/docs
  3. mkdir /media/Docs
  4. mount /dev/sdb2 /media/Docs

But in the process, I had forgotten to update my torrents to use the new location, and when I opened qBittorrent after the change, all torrents were with "Missing Files" status. I had a look at the files located in ~/.local/share/data/qBittorrent, hoping it would suffice to edit them with a text editor, but my editor refused to open them. So I tried:

  1. Quit qBittorrent and waited until the process had actually been destroyed
  2. mkdir /media/docs
  3. mount /dev/sdb2 /media/docs

Notice how I mounted the same device (/dev/sdb2) to a second mountpoint without unmounting it from the first one. This is safe as we can see here (https://bbs.archlinux.org/viewtopic.php?id=166505), and was never an issue for me when working with ext4 filesystems.

Then I opened qBittorrent once again, all torrents were with status "Completed". I right-click each one of them, chose "Set location", and in the file location dialog I copied the current location (which was /media/docs/some_path), and pasted it into the address bar (using Control+L), and changed docs to Docs. I clicked OK; torrent status were still "Completed". To confirm the change has been applied, I right-clicked the first torrent after changing and clicked "Open destination folder", and it opened media/Docs/whatever correctly. Since it worked, I didn't find it necessary to confirm the others.

Then I closed qBittorrent once more, made sure it was finalized, and:

  1. umount /media/docs
  2. rmdir /media/docs

Now when I opened qBittorrent again, all torrents are status "Missing Files". I tried right-click and Force recheck, it didn't work. Then I navigated to /media/Docs in my file browser (Nemo), only to find that, to my horror, all my previously downloaded files are gone.

I'm still not absolutely sure if this is a qBittorrent bug, and I'll now try running some file recovery software... I can try to reproduce this later. But anyway, I felt this was worth reporting because if some action has the potential to delete files permanently (I checked and they're not in the Trash), the user should be warned.

ranolfi commented 7 years ago

Updated description.

Seeker2 commented 7 years ago

I've heard of something similar to this before... Qbittorrent deletes dowloaded torrent instead of checking it https://github.com/qbittorrent/qBittorrent/issues/6690

...But your case may not have a file name+path length issue.

Another way to lose a LOT of torrents: https://qbforums.shiki.hu/index.php/topic,5096.0.html Another or a duplicate of last: https://qbforums.shiki.hu/index.php/topic,5097.0.html

sledgehammer999 commented 7 years ago

Hmm, from the description I can think one non-obvious possibility. It depends on how "smart" libtorrent is. @arvidn can you take a look at the OP? It depends on how libtorrent did the move in this case. Would it recognize that is the same partition(although different mountpoints) and did a simple move or did it think it was a different partition and started copying and deleting-after-success?

It seems that after the move you were able to open the location and locate the files before closing the client though.

FYI, writes a log to disk now. Locate it and examine the last starts/shutdowns of the client. It reports stuff about fastresumes and location move. Does something weird appear?

Lastly, there's a slight possibility that qbittorrent didn't actually remove the files but somehow placed them under some other path. So can you do a filename search on your disks for some files to see if they exist?

Under the General tab does the save path: label point to /media/Docs?

arvidn commented 7 years ago

libtorrent first attempts to move the files (i.e. rename). If that fails, it falls back to copying the files. Every file that is successfully copied will then be deleted.

what qbittorrent could do is to ask the user whether to replace the existing files or not. I take it qBittorrent defaults to always replacing (which would explain the behavior OP experienced).

See here: http://libtorrent.org/reference-Core.html#move_storage()

if you pass in torrent_handle::fail_if_exist you'll get an error if the files at the destination already exists. You can then ask the user to replace existing files or not and re-issue the move_storage() call.

arvidn commented 7 years ago

libtorrent could have a check for this case. It would not be trivial, since every file would have to be resolved to a volume and inode and checked. I would be a bit worried that it would cause problems in more scenarios than it fixes.

ghost commented 2 years ago

libtorrent first attempts to move the files (i.e. rename). If that fails, it falls back to copying the files. Every file that is successfully copied will then be deleted.

what qbittorrent could do is to ask the user whether to replace the existing files or not. I take it qBittorrent defaults to always replacing (which would explain the behavior OP experienced).

See here: http://libtorrent.org/reference-Core.html#move_storage()

if you pass in torrent_handle::fail_if_exist you'll get an error if the files at the destination already exists. You can then ask the user to replace existing files or not and re-issue the move_storage() call.

@glassez could you please implement this method of checking if files exist before moving them? I guess this has been a major issue since many old versions and has been neglected for unknown reasons. Other clients like utorrent ask the user before replacing files. I don't think it'll be very difficult to implement. You just have to check the move_storage_failed_alert for boost::system::errc::file_exists and show a warning dialogue to user if they want to replace the files or cancel the operation. https://libtorrent.org/reference-Torrent_Handle.html#move-storage

ghost commented 2 years ago

Actually there should be 3 options given if files already exist in download location or move storage location.

@qbittorrent/frequent-contributors @qbittorrent/bug-handlers what are you thoughts about this?

glassez commented 2 years ago

Other clients like utorrent ask the user before replacing files. I don't think it'll be very difficult to implement.

Not so difficult, if we talk about GUI-only application. But qBittorrent is not like that. Therefore, our core should still be designed with the expectation of non-interactive use.

I guess this has been a major issue since many old versions and has been neglected for unknown reasons.

It looks like you're trying to resurrect an ancient Issue without fully understanding it. If I'm not confusing anything, then in the current code qBittorrent behaves in such a way that it does not cause problems in the vast majority of cases (at least, the author of this Issue should not have problems with the new version):

  1. it uses existing files if the move is initiated by the user,
  2. it overwrites the existing files with movable ones if it is moving the completed torrent from the download folder to the target folder.

So if you think that interactivity in this matter is really important, you could open a new Issue where you could outline the shortcomings of the current implementation/behavior, and offer (in more detail than here) ways out of the fix, without forgetting about non-interactive usage scenarios, as well as the fact that there are many users who are satisfied with the current behavior, so they do not need any unnecessary annoying interactivity, so it will have to be at least optional. Then someone will be able to assess the likelihood of a problem and the costs of eliminating it in order to conclude whether it is worth the effort.

Chocobo1 commented 2 years ago

it uses existing files if the move is initiated by the user,

What if the existing files doesn't have the same hash? Is it being overwritten later?

glassez commented 2 years ago

it uses existing files if the move is initiated by the user,

What if the existing files doesn't have the same hash? Is it being overwritten later?

I believe it is.

The problem is that the hash check is not smart enough (not smart at all, to be precise). All we have at the output is only information about the matching of the piece hashes of torrent files with the hashes of the corresponding pieces of the files on the disk. Moreover, if some piece does not pass the hash check, no additional analysis is performed, and it is simply marked by libtorrent as not downloaded.

  1. it uses existing files if the move is initiated by the user,

We could change the current behavior, but this is not a solution to the problem, since the user often needs to use existing files. So all we can do so far in this matter is to delegate responsibility for user actions to the users themselves (as it is now done). The only improvement I can think of is to allow the user to choose the behavior of the move operation for those cases where the move is started directly by the user.

2. it overwrites the existing files with movable ones if it is moving the completed torrent from the download folder to the target folder.

Now I believe that the lesser evil would be to use "fail if exists" behavior in this case.

glassez commented 2 years ago

The problem is that the hash check is not smart enough (not smart at all, to be precise).

The same problem affects the case of added torrents.

Chocobo1 commented 2 years ago

The only improvement I can think of is to allow the user to choose the behavior of the move operation for those cases where the move is started directly by the user.

I'm thinking some thing alike (not sure it is the same). IMO conceptually there should be 2 separate 'move' operation for a torrent:

Now I believe that the lesser evil would be to use "fail if exists" behavior in this case.

👍 I partially handled it #17853, can you handle the rest?

glassez commented 2 years ago

Now I believe that the lesser evil would be to use "fail if exists" behavior in this case.

👍 I partially handled it #17853, can you handle the rest?

I will.

glassez commented 2 years ago
  • Another is for user to locate existing files

It looks like an interesting idea to make this a separate action.

The question is how is it expected to behave with existing files (I mean the files that are currently handled by torrent)? There are three main cases (consider a single-file torrent for simplicity):

  1. Only destination file exists. Just do what you suggested above, i.e. try to check it.
  2. Only source file exists. What should we do? Move existing file to target location (IIRC, libtorrent does it if use_existing_files is set)? Or just drop current file?
  3. Both source and target files exist.
glassez commented 2 years ago

In any case, it would be nice to have file checking smarter. At least it could distinguish the cases of non-existing pieces and pieces that have inappropriate data and (optionally) stop torrent from being downloaded if there is inappropriate data found during files checking.

Another useful feature that libtorrent could provide to applications is ability to check some arbitrary files for matching the torrent metadata without adding the torrent to the session. I.e. some function that accepts torrent_info and map of filenames indexed by file_index_t as parameters and returns info about corresponding pieces matches (of course, distinguishing zero and unsuitable pieces, like suggested above).

@arvidn Have you any thoughts about it?

arvidn commented 2 years ago

I'm not quite following what "smarter" file checking means or how it relates to this ticket. iiuc, this ticket describes a scenario where the source and the destination directories point to the same physical directory. When libtorrent were done copying, it deleted the source files, which turned out to also be the destination files. Admittedly not great.

Once this has happened, I don't see what checking could do to help. The files are gone.

I suppose the scenario you're considering is where there's an unrelated file (but with the same filename) in the download directory, and you'd want to distinguish an unrelated file from a file from the previous session.

So, is the file unrelated if a single piece has the wrong hash? - probably not, that would be likely to happen if just one small file is deleted, or if there's actually a corrupt piece received from the network.

Perhaps the file is considered unrelated if all pieces have the wrong hash? Presumably then pieces that are just full of zeroes wouldn't count, since that's the default after fallocate()ing the file.

Admittedly a fringe feature, but the SetFileValidData() would probably have to be considered too, since that's a back-door to allocate a file without clearing it to zero. So it might not be "unrelated" even though it's full of noise.

Chocobo1 commented 2 years ago

Only source file exists. What should we do? Move existing file to target location (IIRC, libtorrent does it if use_existing_files is set)?

IMO don't move/delete any file, only adjust the torrent folder/file pointer. Since user pointed the torrent directory to somewhere where the file doesn't exists then the file has to be redownloaded.

Or just drop current file? Both source and target files exist.

Like above, leave the current/original file as is.

glassez commented 2 years ago

iiuc, this ticket describes a scenario where the source and the destination directories point to the same physical directory.

Apparently, the tail of this thread is offtopic. Nevertheless, it touches on a very important problem for many users.

I suppose the scenario you're considering is where there's an unrelated file (but with the same filename) in the download directory

Exactly. So adding new torrent (#127) or moving torrent storage can cause overwriting of unrelated files.

TomArrow commented 2 years ago

qbittorrent just loves deleting your files. Been an issue forever and nobody seems to care. Mind-boggling.

Not 100% the same situation but this happened to me too. I moved a perfectly fine downloaded torrent first on my hard drive and then adjusted the location in qbit, resulting in all the data being wiped without any prompt or question.

glassez commented 2 years ago

Apparently, the tail of this thread is offtopic. Nevertheless, it touches on a very important problem for many users.

I suppose the scenario you're considering is where there's an unrelated file (but with the same filename) in the download directory

Exactly.

@arvidn Let's continue in separate Issue: https://github.com/arvidn/libtorrent/issues/7138.