Ticket #2829 (closed defect: fixed)

Opened 12 years ago

Last modified 12 years ago

Loss of data on copy to full partition

Reported by: onlyjob Owned by: andrew_b
Priority: critical Milestone: 4.8.4
Component: mc-core Version: 4.8.3
Keywords: Cc:
Blocked By: Blocking:
Branch state: merged Votes for changeset: committed-master committed-stable

Description

To reproduce:

  1. Copy file(s) to ext4 partition until there is no more free space left.
  2. When no more space left choose small file around 7K in size and try to copy it.
  3. When dialog "No space left on device (28)" appears, choose "Skip".
  4. The same dialog may appear again - choose "Skip" one more time.
  5. Observe empty (zero-size) file being created in destination.

If copying many files to partition with no free space, choosing "Skip All" may lead to weird condition when copying seemingly continue after filling all the space in partition but leaving zero-size files behind. (Consequences of move may be devastating)

Another manifestation of this problem is when you're editing the file on full partition with mcedit and add text to the beginning of file it may be trimmed to 4096 bytes and loose its tail on save.

See

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677038
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677309

Please advise.

Attachments

fix_nospace.patch (2.8 KB) - added by onlyjob 12 years ago.
patch for 4.8.3

Change History

comment:1 in reply to: ↑ description Changed 12 years ago by andrew_b

Replying to onlyjob:

Another manifestation of this problem is when you're editing the file on full partition with mcedit and add text to the beginning of file it may be trimmed to 4096 bytes and loose its tail on save.

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=677309

This is another bug that isn't related with copy/move file operation. See #2470 and #15.

comment:2 Changed 12 years ago by onlyjob

I see, thank you (somehow I didn't know about mcedit' save modes).

To avoid reporting two issues as one I should be better searching for corresponding ticket next time.

Cheers,
Dmitry.

comment:3 Changed 12 years ago by andrew_b

  • Owner set to andrew_b
  • Keywords stable-candidate added
  • Status changed from new to accepted
  • Branch state changed from no branch to on review
  • Milestone changed from Future Releases to 4.8.4

Branch: 2829_copy_to_full_partition (parent: master).
changeset:6c59cff328bc0728cd9a4105b9b19bfd397422e8

comment:4 follow-up: ↓ 6 Changed 12 years ago by onlyjob

Thanks for the patch.
I tried it and here is some feedback:

When copying one file to full partition a dialog "No space left on device (28)" appears.

If I choose [Skip] the same dialog returns (as if I choose [Retry]);

[Skip All] or [Abort] lead to another dialog "Incomplete file retrieved. Keep it?"

When copying many files [Skip] also behaves like [Retry].

But the most unexpected thing happened when I tried to copy small file just about 12 KB:
first dialog "Cannot write target file... No space left on device (28)" appears
but when I choose [Skip] few times either empty file is created in destination
or (if I'm overwriting existing file) target file is trimmed down to exactly 4096 bytes.

comment:5 Changed 12 years ago by andrew_b

  • Branch state changed from on review to on rework

comment:6 in reply to: ↑ 4 ; follow-up: ↓ 7 Changed 12 years ago by andrew_b

Replying to onlyjob:

Thanks for the patch.
I tried it and here is some feedback:
[...]

Thanks for the feedback!

I don't have an unallocated free space on my hard drive to create small partition and test with it. By i tested my patch with two ways:
a) I created a small partition in the file and mount that via loop device;
b) I installed Linux in VirtualBox? and created small partition within that.

Unfortunately, I'm unable to reproduce and confirm none of you feedback issues.

When copying one file to full partition a dialog "No space left on device (28)" appears.

If I choose [Skip] the same dialog returns (as if I choose [Retry]);

Works as expected for me. "No space left on device (28)" error isn't appears again.

[Skip All] or [Abort] lead to another dialog "Incomplete file retrieved. Keep it?"

Cannot confirm.

When copying many files [Skip] also behaves like [Retry].

But the most unexpected thing happened when I tried to copy small file just about 12 KB:
first dialog "Cannot write target file... No space left on device (28)" appears
but when I choose [Skip] few times either empty file is created in destination
or (if I'm overwriting existing file) target file is trimmed down to exactly 4096 bytes.

Cannot confirm too.

Version 0, edited 12 years ago by andrew_b (next)

comment:7 in reply to: ↑ 6 ; follow-up: ↓ 8 Changed 12 years ago by onlyjob

Replying to andrew_b:

a) I created a small partition in the file and mount that via loop device;

That's what I did as well. I used ext4 partition of 80M.

Unfortunately, I'm unable to reproduce and confirm none of you feedback issues.

Perhaps we're doing something in a different way. I noticed that some of the problems getting worse when there is no free space in partition whatsoever. If there are few KB still available errors may be harder to reproduce.

Also I was using 3.8.3 with patch I've made of your changeset.

I hope this may be of help.

comment:8 in reply to: ↑ 7 ; follow-up: ↓ 9 Changed 12 years ago by andrew_b

Do you use the "Preallocate space" option (Menu->Options->Configuration->File operation options)?

comment:9 in reply to: ↑ 8 ; follow-up: ↓ 10 Changed 12 years ago by onlyjob

Replying to andrew_b:

Do you use the "Preallocate space" option (Menu->Options->Configuration->File operation options)?

No, it is not in use. Sorry for not mentioning it.

comment:10 in reply to: ↑ 9 Changed 12 years ago by andrew_b

OK. I added the tweak commit [fddfb7bf96a2f76c28ac85352b7d96e58fddae21]. Please test it together with [6c59cff328bc0728cd9a4105b9b19bfd397422e8].

Changed 12 years ago by onlyjob

patch for 4.8.3

comment:11 Changed 12 years ago by onlyjob

Andrew, it is perfect. Thank you.
I tested with and without [Preallocate space].

Patch for 4.8.3 is attached.

Fantastic work. :)

comment:12 Changed 12 years ago by andrew_b

  • Branch state changed from on rework to on review

Thanks for test and feedback. My initial mistake was to use errno regardless of what mc_write() returned. If some function returns success value, it does't changes errno (and doesn't set it to 0). errno should be used if and only if function returns error.

comment:13 Changed 12 years ago by slavazanko

  • Votes for changeset set to slavazanko

comment:14 Changed 12 years ago by angel_il

  • Votes for changeset changed from slavazanko to slavazanko angel_il
  • Branch state changed from on review to approved

comment:15 Changed 12 years ago by andrew_b

  • Status changed from accepted to testing
  • Keywords stable-candidate removed
  • Votes for changeset changed from slavazanko angel_il to committed-master committed-stable
  • Resolution set to fixed
  • Branch state changed from approved to merged

comment:16 Changed 12 years ago by andrew_b

  • Status changed from testing to closed
Note: See TracTickets for help on using tickets.