usb_test fails on non-writable filesystems

Note that this was verified too with the new usb_test from checkbox trunk (r1160), so this complements bug 887049 that happened on a USB stick with multiple partitions, some of which were unwritable.

I took the liberty of subscribing François as he was also experiencing this problem.

"The test needs to verify that the drive is writable, so if I can't write to it, it should still fail, but do so in a graceful manner."

Are you sure ? What is this test supposed to do ? I thought it was just to check if Ubuntu can mount a partition from a USB stick/harddrive. Why should it necessarily be writable ? I know that most partitions should and will be. But let's say you are doing in the test with an external harddrive used on a Mac ? Or maybe an NTFS partition not closed properly. Does it mean that this test is failing ? I don't think so.

But then it's a bit complicated to perform this test. It should react accordingly to the partition format. And maybe just do a read test on partitions Ubuntu can't write on.

We want to check the USB subsystem is working properly. Just mounting a partition is not really good enough (would you test Wifi by just checking that it pairs with an access point?). I would suggest that we should try to write to every partition which is writable, and just ignore the ones that aren't. If no partitions are writable then maybe we need to fail the test (not enough info?)

Hi François,

the usb/disk_detect test checks if the partitions are mounted (usb_test -l).

usb_test -t is used by usb/storage_transfer, and although the purpose is a bit vague ("This test will check your USB connection."), as Brendan mentions, we want to test that the drive can be written to without the data getting corrupted. If we're unable to write to it, we don't know either way, so the safe thing to do is assume something is amiss and fail the test. Either that, or we add a long list of heuristics to catch, for instance, the cases you mention, among others.

I agree with your idea about doing a read-only test, if we can do that without i/o errors we could take the drive to be good. But yes, detecting whether a partition is writable and doing read/write or read-only tests accordingly would be the most thorough way to test this.

Status changed to 'Confirmed' because the bug affects multiple users.

I'm recreating this and wondering just how big a case this is... I think the best solution is brendan's, just write to what's writeable... plus I think I'll add to the description suggesting that usb keys are used in preference over usb HDDs.

I'm just concerned, given the comments here, that someone is going to plug in their 2TB NTFS hard disk full of photos and personal data, and have this test accidentally destroy their filesystem... Granted there's always a slight chance of that happening (and I get that we're doing essentially the same thing on the onboard HDD) but I'd rather lose whats on my laptops HDD than what I have stored on USB disks.

Hi!

Well, maybe the description should be changed from "test fails" to "test fails with a nasty trace" :)

I agree that it we can't write, then we can't write, and just changing this so that the test exits gracefully with "no writable partitions on the device" would be quite acceptable.

However, in that case, the test should at least try reading an existing file from the device. If we can read, we know the USB bus is transferring data correctly.

The point is to test the USB bus's storage and transfer capability any way we can, otherwise this will become one of those "I don't know either way, so I'll just ignore it" tests.

>I'd rather lose whats on my laptops HDD than what I have stored on USB disks.
Your not every one :)

Well, now the test doesn't fail at all... well it does. I've modified the script to behave better, Instead of a nasty traceback, all you get is an overall fail of the test and output that tells you which partition failed. I tried in on my own systems with 2 usb keys:

1: Common use case, 8GB USB key with a single vfat partition.
2: Non-common case, 8GB USB key with 1 ext2, 1 vfat and 1 NTFS partition.

2 was set so that partition 1 (ext2) was mounted/owned by root, and partition 3 (NTFS) was mounted read-only

On testing key 1, test passed and returned 0.
On testing key 2, test passed on 1 partition, failed on 2, test returned a 1.

I'm not sure I quite understand. With the changes you made, does the test still fail if it has any writable partitions? In my opinion it shouldn't - it just makes things confusing for the tester. As Daniel mentioned before, a good idea might be to test reading at a minimum for read-only partitions. As long as reading and/or writing works when attempted then the USB port should be fine.

So if it encounters a usb device with say... 3 partitions and two of those partitions are not writable and one IS, it will complete testing ALL partitions, then fail overall by saying "Completed X iterations of testing, but there were errors."

And the script output will tell you which partitions failed.

The problem is, you can't necessarily determine a non-writable partition. For example, this is my test drive

bladernr@klaatu:~/development/fix-usb-time-limit/scripts$ mount
...
/dev/sdb1 on /media/6e2ec127-1f1e-4035-8b9e-9b9d31b31425 type ext2 (rw,nosuid,nodev,uhelper=udisks)
/dev/sdb3 on /media/69C4EBBA125CC3B7 type fuseblk (rw,nosuid,nodev,allow_other,blksize=4096,default_permissions)
/dev/sdb2 on /media/1297-8992_ type vfat (rw,nosuid,nodev,uid=1000,gid=1000,shortname=mixed,dmask=0077,utf8=1,showexec,flush,uhelper=udisks)

Three partitions... one was ext2, one vfat and one ntfs... they're all mounted RW automatically by udev, however, default udev rules apparently differ by filesystem type... here's what the user sees:

drwx------ 2 bladernr bladernr 4096 2012-01-19 09:52 1297-8992_
drwx------ 1 bladernr bladernr 4096 2012-01-19 09:52 69C4EBBA125CC3B7
drwxr-xr-x 3 root root 4096 2012-01-18 17:01 6e2ec127-1f1e-4035-8b9e-9b9d31b31425

So even though 9b9d31b31425 is moutned as read-write, the non-rot user can not write to it at all. but how do you determine that at run time, without creating overly complicated hueristics that read the permissions on the directory or something? No way that I know of...

So the simpler, cleaner method was to simply create a fail if we can't write data to the drive, note that and move on. That way, if we notice that 3 of 4 partitions pass, but one did not, it's probably a false positive, but if the device has only one partition and that failed, we may be more interested.

Another way around it would simply be to run the test as root, but I don't like running potentially destructive tests as the root user...

Also, having a usb drive with multiple partitions is not a common use case... I'd guess that probably 85 - 90% of all people would never format their usb key, or usb hard disk into multiple partitions. Consider how long this test has been around and used and no one mentioned this issue before... it's taken a good while for someone to hit the right combo to hit this bug...

As fare as i remember all i did was run the test from a live USB, posibly the reason was that the drive was filled.

Yeah, that would have triggered the bug since the live USB would be mounted read-only.

I actually tested this initially using my stick mentioned above with one of the partitions remounted read-only, so I had one mounted read-write, one read-only and one read-write but no read permissions for non-root users, and the bug appeared on both.

Jeff,

Have you tried using the 'DeviceIsReadOnly' property of org.freedesktop.UDisks.Device? That might be a straightforward way of telling if a partition is read-only.

On 01/21/2012 12:53 PM, Brendan Donegan wrote:
> Jeff,
>
> Have you tried using the 'DeviceIsReadOnly' property of
> org.freedesktop.UDisks.Device? That might be a straightforward way of
> telling if a partition is read-only.
>

Was looking at that the other day, but haven't tried it just yet...

That could fix the problem when a partition is specifically mounted
read-only (as in when running from a live image) but doesn't address the
weird default behaviour where a mount point is only readable by root by
default...

it's certainly something worth looking at more closely though.

--
Jeff Lane - Hardware Certification Engineer and Test Tools Developer
Ubuntu Ham: W4KDH
Freenode IRC: bladernr or bladernr_
gpg: 1024D/3A14B2DD 8C88 B076 0DD7 B404 1417 C466 4ABD 3635 3A14 B2DD

This bug was fixed in the package checkbox - 0.13.1

---------------
checkbox (0.13.1) precise; urgency=low

  New upstream release (LP: #925090):

  [Brendan Donegan]
  * Fixed the cpu_topology script so that it doesn't mistake the word
    'processor' in the value of another field for the field 'processor'
    (LP: #882161)
  * Added create_connection script and jobs to automatically create/test a
    wireless network connection.
  * Updated wireless job dependencies.
  * Add wireless performance data collecting tests.
  * Changed is_laptop test to a shell test and implemented a check_is_laptop
    script to check automatically for a systems 'laptopness' (LP: #886668)
  * Fixed connect_wireless script which continued failing to correctly
    identify wireless connections.
  * Don't fail the sleep_test if the wake alarm is still set (LP: #911161)
  * Add requirement for mem sleep state to be supported to the
    suspend_advanced_auto job (LP: #804190)
  * Fixed the camera/display test and removed the camera/video one.
  * Added display resource and matching requirements to external video
    output tests.
  * Added removable_storage_watcher script to replace watch_command to make
    testing USB, FireWire and MMC devices easier and more cohesive.
  * Added memory_compare script to automate the memory/info job
  * Switch audio settings to correct device before running audio tests
    (LP: #916859)
  * Nixed graphics/xorg-version-output job and updated other job dependencies,
    since it is redundant with graphics/xorg-version. (LP: #671144)

  [Gabor Kelemen]
  * Fixed last two remaining strings with backslashes (LP: #868571)
  * Fix misplaced parentheses, so translation can work (LP: #904876)

  [Marc Tardif]
  * Refactored install scripts to be agnostic of variant name:
    install/postinst, install/config and debian/*.postinst.
  * Using title defined in user_interface plugin in GTK interface.
  * Updated default.whitelist to reflect renamed jobs.
  * Removed files with non-printable characters from submission.xml.
  * Fixed parser for submission files with empty question comments
    and context info (LP: #912546)
  * Added support for skipping tests when the depends don't pass
    (LP: #509598)
  * Removed extraneous code from the sleep_test.
  * Refactored logic to check for network after suspend.
  * Removed deprecated hwtest package.
  * cpu_offlining was incorrectly using return instead of exit.

  [Daniel Manrique]
  * Update control files under debian/ to eliminate (most) lintian warnings
    (LP: #352986)
  * Environment variables specified with environ: in a job description will be
    passed to the backend for it to add to its environment. (LP: #897889)
  * Handle malformed LANGUAGE environment variable values (LP: #912946)
  * Added interactive media_keys_test script.
  * Make creation of wireless connection files more robust (LP: #923836)
  * Recommend gstreamer-gconf to enable media tests on kubuntu (LP: #898641)
  * Add bluetooth device requirement to obex jobs (LP: #921128)
  * Add a plugin conf variable for the welcome string (shown on the first
    screen when checkbox runs), so it can be changed without much effort.
  *...