Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loss of data on copy to full partition #2829

Closed
mc-butler opened this issue Jun 17, 2012 · 17 comments
Closed

Loss of data on copy to full partition #2829

mc-butler opened this issue Jun 17, 2012 · 17 comments
Assignees
Labels
area: core Issues not related to a specific subsystem prio: high Serious problem that could block progress ver: 4.8.3 Reproducible in version 4.8.3
Milestone

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/2829
Reporter onlyjob (@onlyjob)

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.

Note

Original attachments:

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 17, 2012 at 9:10 UTC

Replying to onlyjob (#2829):

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.

@mc-butler
Copy link
Author

Changed by onlyjob (@onlyjob) on Jun 17, 2012 at 9:50 UTC (comment 2)

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.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 17, 2012 at 11:40 UTC (comment 3)

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

Branch: 2829_copy_to_full_partition (parent: master).
[6c59cff328bc0728cd9a4105b9b19bfd397422e8]

@mc-butler
Copy link
Author

Changed by onlyjob (@onlyjob) on Jun 18, 2012 at 12:19 UTC (comment 4)

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.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 22, 2012 at 11:32 UTC (comment 5)

  • Branch state changed from on review to on rework

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 23, 2012 at 13:14 UTC (comment 4.6)

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. But 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.

@mc-butler
Copy link
Author

Changed by onlyjob (@onlyjob) on Jun 23, 2012 at 13:40 UTC (comment 6.7)

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.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 23, 2012 at 15:55 UTC (comment 7.8)

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

@mc-butler
Copy link
Author

Changed by onlyjob (@onlyjob) on Jun 23, 2012 at 16:00 UTC (comment 8.9)

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.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 23, 2012 at 16:59 UTC (comment 9.10)

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

@mc-butler
Copy link
Author

Changed by onlyjob (@onlyjob) on Jun 24, 2012 at 10:02 UTC

patch for 4.8.3

@mc-butler
Copy link
Author

Changed by onlyjob (@onlyjob) on Jun 24, 2012 at 10:05 UTC (comment 11)

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

Patch for 4.8.3 is attached.

Fantastic work. :)

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 24, 2012 at 10:36 UTC (comment 12)

  • 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.

@mc-butler
Copy link
Author

Changed by slavazanko (@slavaz) on Jun 25, 2012 at 10:24 UTC (comment 13)

  • Votes set to slavazanko

@mc-butler
Copy link
Author

Changed by angel_il (@ilia-maslakov) on Jun 26, 2012 at 11:26 UTC (comment 14)

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

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 26, 2012 at 12:21 UTC (comment 15)

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

Merged to master: [af2fe31].
Merged to 4.8.1-stable: [5737723]

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Jun 26, 2012 at 12:23 UTC (comment 16)

  • Status changed from testing to closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues not related to a specific subsystem prio: high Serious problem that could block progress ver: 4.8.3 Reproducible in version 4.8.3
Development

No branches or pull requests

2 participants