Opened 4 years ago

Closed 4 years ago

Last modified 2 years ago

#10312 closed defect (fixed)

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
Blocked By: Blocking:
Deployments affected: Action Needed: no action
Verified: no

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 (1)

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)

Download all attachments as: .zip

Change History (16)

Changed 4 years ago by erikos

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

comment:1 Changed 4 years ago by erikos

  • Action Needed changed from code to package
  • Status changed from new to assigned

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.

comment:2 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

comment:3 Changed 4 years ago by Quozl

  • Milestone changed from Not Triaged to 10.1.3
  • Priority changed from high to normal

comment:4 Changed 4 years ago by greenfeld

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

comment:5 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.

comment:6 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?

comment:7 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.

comment:8 Changed 4 years ago by erikos

  • Cc greenfeld added

comment:9 Changed 4 years ago by erikos

  • Action Needed changed from review to test in build

comment:10 follow-up: Changed 4 years ago by greenfeld

  • Action Needed 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?

comment:11 in reply to: ↑ 10 ; follow-up: Changed 4 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)

comment:12 in reply to: ↑ 11 Changed 4 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?

comment:13 Changed 4 years ago by greenfeld

  • Action Needed changed from diagnose to test in build

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

comment:14 Changed 4 years ago by greenfeld

  • Action Needed changed from test in build to no action
  • Resolution set to fixed
  • Status changed from assigned to closed

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.

comment:15 Changed 2 years ago by humitos

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