Ticket #5719 (closed defect: fixed)

Opened 6 years ago

Last modified 5 years ago

Deleting a large file from a USB stick copies it into NAND (often filling NAND)

Reported by: gnu Owned by: tomeu
Priority: blocker Milestone: Update.1
Component: journal-activity Version: Build 650
Keywords: review? Cc: wmb@…, jg, mstone
Action Needed: Verified: no
Deployments affected: Blocked By:
Blocking:

Description

WARNING: If your file is larger than the amount of free NAND space you have left, this will leave your laptop unbootable, due to #5317. It can be recovered by a full wipe, so it's not a total brick, but you lose all your data.

Put a 200+MB file onto a FAT-formatted USB key on a real Linux machine (I used a copy of os542.img called "deleteme.pls"). Unmount it and move it to an XO. Go to the terminal. Do "df -h". Note the free space in your NAND.

Now go to the Journal, click on the USB logo, find the file, go into its detail view, and press the "-" in a circle to "Erase" the file. Watch it take a long time; watch the USB access light blink a lot.

Go back to the terminal while it's doing this. Run "df -h" again. Note that the NAND is filling up.

I did strace on the sugar-datastore process; it was in a read/write copy loop. J'accuse!

After it was done, I noticed:

  • The file was still there on the USB key, undeleted.
  • There were TWO copies of it in /home/olpc/.sugar/default/data, one complete copy called "deleteme.pls", and one partial copy called "deleteme(1).pls".
  • I had only 11 Mbytes free in my NAND.
  • The Journal showed no change from before I tried deleting the file - neither in the USB page nor in the Journal page.

I was VERY CAREFUL to remove those two files from where Sugar copied them, and check "df -h" again, before rebooting my system.

Bugs:

Deleting a file should never INCREASE the amount of space occupied. (Other than possibly adding tiny housekeeping records.)

Deleting a file from one filesystem (e.g. USB) should never result in copying that file to another filesystem (e.g. NAND).

Deleting a file should actually remove the file from the place that it was deleted from.

This bug was reported by donor Pamela Dallas as a suspicion (rt# 2962), and verified by me (John Gilmore) on build 650 on a B4.

Change History

Changed 6 years ago by cscott

I have a patched copy of olpcrd which will let you recover the system as follows:

  • First, get a developer key for the machine.
  • Go to the OFW prompt.
  • Type: (the 'ok' is the prompt, don't type that)
    ok " nand:\boot\vmlinuz" to boot-device
    ok " disk:\olpcrd" to ramdisk
    ok boot
    
  • Now you will boot into linux. Sugar won't start, because the filesystem is full, but after about 30s it will give up and you will be at a console.
  • Log in as root (or 'olpc' on joyride/update.1 builds).
  • Type:
    cd ~olpc/.sugar/default/data
    ls -s | sort -n
    
  • The last file listed is the biggest file in your journal, and likely the culprit. Remove it:
    rm FILENAME
    sync
    
  • Power off and reboot normally.

In addition to now being idiotic and copying files to NAND before trying to delete them, the datastore *should* be careful to always clean up after failed operations and delete orphaned files. In theory, there should be a datastore-fsck which removes orphans.

Changed 6 years ago by cscott

You may have to press Ctrl-Alt-"3 dots" (fourth key in from the top-left) to get a console before you can log in as root or olpc.

Changed 6 years ago by cscott

  • cc wmb@…, jg added
  • keywords update.1? added

You will also have to hold the power button down for ~10s to "power off" at the very end. Short presses may simply suspend your laptop.

The olpcrd file referenced above can be downloaded from http://dev.laptop.org/~cscott/trac5719/olpcrd Right-click and use 'save as'.

Put it in the top-level of your FAT-formatted USB key, as a file named 'olpcrd' (no suffix).

Changed 6 years ago by cscott

  • cc mstone added

Changed 6 years ago by wmb@…

OFW q2d07c and later have the ability to delete files from the JFFS2 filesystem, so long as there is at least one empty page for storing the deletion node.

  ok dir n:\home\olpc\.sugar\default\data\
  ok rm n:\home\olpc\.sugar\default\data\XXX

where XXX is the name of the file you want to delete.

Changed 6 years ago by jg

  • keywords update.1? removed
  • verified unset
  • milestone changed from Never Assigned to Update.1

gnu: verified is intended to be used that something is "Verified fixed" rather than verified to be a bug. coderanger is working on a better workflow system for us we'll deploy in the next month.

Changed 6 years ago by tomeu

  • keywords review? added
  • component changed from datastore to journal-activity

The journal was asking the file to the DS in order to check if it was a bundle, as we need to uninstall the bundles before deleting them.

This patch will copy only the bundles, limiting this problem. If we want to avoid copying the bundles at all, we need to fix #5744.

diff --git a/misc.py b/misc.py
index b1b8144..ae53137 100644
--- a/misc.py
+++ b/misc.py
@@ -131,16 +131,13 @@ def get_date(jobject):
         return _('No date')
 
 def get_bundle(jobject):
-    if jobject.file_path == '':
-        # Probably a download-in-progress
-        return None
-
     try:
-        if jobject.is_activity_bundle():
+        if jobject.is_activity_bundle() and jobject.file_path:
             return ActivityBundle(jobject.file_path)
-        elif jobject.is_content_bundle():
+        elif jobject.is_content_bundle() and jobject.file_path:
             return ContentBundle(jobject.file_path)
-        elif jobject.metadata['mime_type'] == JournalEntryBundle.MIME_TYPE:
+        elif jobject.metadata['mime_type'] == JournalEntryBundle.MIME_TYPE and \
+                jobject.file_path:
             return JournalEntryBundle(jobject.file_path)
         else:
             return None

Changed 6 years ago by tomeu

Add a comment as suggested by Michael:

diff --git a/misc.py b/misc.py
index b1b8144..25d99fb 100644
--- a/misc.py
+++ b/misc.py
@@ -131,16 +131,16 @@ def get_date(jobject):
         return _('No date')
 
 def get_bundle(jobject):
-    if jobject.file_path == '':
-        # Probably a download-in-progress
-        return None
-
     try:
-        if jobject.is_activity_bundle():
+        # Check that the entry has a file only if it is a bundle, as
+        # jobject.file_path would cause the file to be copied if it is in an
+        # usb stick. See https://dev.laptop.org/ticket/5744
+        if jobject.is_activity_bundle() and jobject.file_path:
             return ActivityBundle(jobject.file_path)
-        elif jobject.is_content_bundle():
+        elif jobject.is_content_bundle() and jobject.file_path:
             return ContentBundle(jobject.file_path)
-        elif jobject.metadata['mime_type'] == JournalEntryBundle.MIME_TYPE:
+        elif jobject.metadata['mime_type'] == JournalEntryBundle.MIME_TYPE and \
+                jobject.file_path:
             return JournalEntryBundle(jobject.file_path)
         else:
             return None

Changed 6 years ago by tomeu

Note that this issue would be solved by a fix for #5744.

Changed 6 years ago by tomeu

  • status changed from new to closed
  • resolution set to fixed

Verified fixed in joyride 1522. See #5920 for the approval request.

Note: See TracTickets for help on using tickets.