Ticket #10312 (closed defect: fixed)

Opened 4 years ago

Last modified 20 months ago

Journal does not provide any error message on write errors

Reported by: erikos Owned by: erikos
Priority: normal Milestone: 10.1.3
Component: sugar Version: Development build as of this date
Keywords: Cc: greenfeld, godiard, humitos
Action Needed: no action Verified: no
Deployments affected: Blocked By:
Blocking:

Description

http://bugs.sugarlabs.org/ticket/1842

I want to track the back port of the patch to 0.84 here. Landed in 0.90. The master patch does depend on new API in sugar-toolkit, the 0.84 should just create the Alert inside the patch.

Attachments

0001-Journal-add-error-message-on-copy-to-device-errors.patch (7.1 kB) - added by erikos 4 years ago.
Journal: add error message on copy to device errors (patch for 0.84)

Change History

Changed 4 years ago by erikos

Journal: add error message on copy to device errors (patch for 0.84)

  Changed 4 years ago by erikos

  • status changed from new to assigned
  • next_action changed from code to package

Refined the master patch and attached the patch for 0.84. I would to include it in 10.1.3 as it has been originally requested from the field. The only issue is that it introduces strings which we need the translations for.

  Changed 4 years ago by erikos

  • priority changed from normal to high

|TestCase|

- insert an usb device
- drag an entry with no file associated (e.g. log entry) on the device (main view)
---> Alert that you can not copy an entry that has no file.
- go to the detail view of a log entry and copy onto the device
---> Alert that you can not copy an entry that has no file.

- insert an usb device that is write protected
- drag an entry with a file associated (e.g. screenshot) on the device (main view)
---> Alert: Permission denied
- go to the detail view of a screenshot and copy onto the device
---> Alert: Permission denied

  Changed 4 years ago by Quozl

  • priority changed from high to normal
  • milestone changed from Not Triaged to 10.1.3

  Changed 4 years ago by greenfeld

  • next_action changed from package to review

Test RPMs used:

  • sugar-0.84.22-2.fc11.i586.rpm
  • sugar-artwork-0.84.2-2.fc11.i586.rpm
  • sugar-presence-service-0.84.1-2.fc11.noarch.rpm
  • sugar-toolkit-0.84.12-2.fc11.i586.rpm

If the disk is full, these RPMs detect it and display an error message.

However I could NOT get these RPMs to display any errors if Linux had USB or SD media mounted as read-only, either via remounting USB media that lacked a write switch (as root: /sbin/mount /mount/your_usb_stick -o ro,remount), or by using an SD card with a write switch on it (which the OS detects and will mount media as ro/rw as appropriate). Any error message caught is not displayed to the user when this happens.

More investigation may be needed.

  Changed 4 years ago by erikos

Good catch! We do only listen for IOError, but OSError is raised when we do not have enough permissions. Will do a new patch.

  Changed 4 years ago by erikos

I have updated the rpm: http://dev.laptop.org/~erikos/sugar-0.84.22-2.fc11.i586.rpm

Can you give it another try?

  Changed 4 years ago by erikos

Hmm, interesting on 0.90 we get the IOError when we do not have write permissions. That is why the code did work when i tested it for me. The RPM above does catch as well the OSError so it will work - will have a look if there were some changes.

  Changed 4 years ago by erikos

  • cc greenfeld added

  Changed 3 years ago by erikos

  • next_action changed from review to test in build

follow-up: ↓ 11   Changed 3 years ago by greenfeld

  • next_action changed from test in build to diagnose

The Test cases mentioned above work in 10.1.3 os351 on both an XO-1 and a XO-1.5.

However no error message appears if the disk is full. But this is not a test case mentioned above. I tried almost filling up the internal flash drive and then copying things from the USB stick and no error message appeared.

Is a check for the latter behavior supposed to be in this patch?

in reply to: ↑ 10 ; follow-up: ↓ 12   Changed 3 years ago by erikos

Replying to greenfeld:

The Test cases mentioned above work in 10.1.3 os351 on both an XO-1 and a XO-1.5. However no error message appears if the disk is full. But this is not a test case mentioned above. I tried almost filling up the internal flash drive and then copying things from the USB stick and no error message appeared. Is a check for the latter behavior supposed to be in this patch?

Yes, the case you are mentioning is indeed not handled by the fixes in here. This case does handle errors when copying from the Journal to an external device.

So if I read your lines correct, you would expect an error message appear when your disk is full and you copy things from an external device to the Journal, correct? And being proactive like we do with the "Your Journal is full message" that appears when the remaining size is below 50 mb? (this should go into a separate bug if)

in reply to: ↑ 11   Changed 3 years ago by erikos

Replying to erikos:

Replying to greenfeld:

The Test cases mentioned above work in 10.1.3 os351 on both an XO-1 and a XO-1.5. However no error message appears if the disk is full. But this is not a test case mentioned above. I tried almost filling up the internal flash drive and then copying things from the USB stick and no error message appeared. Is a check for the latter behavior supposed to be in this patch?

Yes, the case you are mentioning is indeed not handled by the fixes in here. This case does handle errors when copying from the Journal to an external device. So if I read your lines correct, you would expect an error message appear when your disk is full and you copy things from an external device to the Journal, correct? And being proactive like we do with the "Your Journal is full message" that appears when the remaining size is below 50 mb? (this should go into a separate bug if)

Sam, can we close this ticket (and open a new ticket if necessary), given my response from above?

  Changed 3 years ago by greenfeld

  • next_action changed from diagnose to test in build

Since it is just comment confusion suggesting this fix does more taking it back for testing/closing.

  Changed 3 years ago by greenfeld

  • status changed from assigned to closed
  • next_action changed from test in build to no action
  • resolution set to fixed

We now get an error message if the file system we are writing to is read-only or lacks the proper permissions for us to write in 10.1.3 os354.

  Changed 20 months ago by humitos

  • cc godiard, humitos added
Note: See TracTickets for help on using tickets.