Merge lp:~sylvain-pineau/checkbox/story597_removable_test_for_SD into lp:checkbox

[Sylvain Pineau]

This MR tries to offer a way to properly identify memory cards plugged in the system under test.
Udisks correctly manages this type of devices but only shows detailed information if they are available on the sdio interface.

Memory cards plugged in a usb card reader are just seen as usb devices an we can't get the media type.

I've started to look into the ouput of the following commands, but the SCSI spec (2 or 3) doesn't provide enough information
to clearly identify such devices (My sd card is mounted as /dev/sdb):

sudo sg_scan -i (see http://pastebin.ubuntu.com/1173366/)
sudo sg_inq /dev/sdb (see http://pastebin.ubuntu.com/1173368/)

But we can see the reader model name in the sg_scan ouput and this data is also available
to udev and part of the ancestors path:

udevadm info -a -p /sys/block/sdb (see http://pastebin.ubuntu.com/1173369/)

Finally udisks can access the model name because it's part of the unique id name given to the device and can be retrieved with the DeviceFileById property (http://hal.freedesktop.org/docs/udisks/Device.html#Device:DeviceFileById)

udisks --show-info /dev/sdb (see http://pastebin.ubuntu.com/1173370/)

This is an extract of /dev/disk/by-id/

/dev/disk/by-id/ata-HL-DT-ST_DVD+_-RW_GS20N_KZ5A1S70602 -> ../../sr0
/dev/disk/by-id/mmc-SD08G_0x074e5fa1 -> ../../mmcblk0
/dev/disk/by-id/mmc-SD08G_0x074e5fa1-part1 -> ../../mmcblk0p1
/dev/disk/by-id/mmc-SD08G_0x074e5fa1-part2 -> ../../mmcblk0p2
/dev/disk/by-id/usb-Generic_Ultra_HS-SD_MMC_000000264001-0:0 -> ../../sdb
/dev/disk/by-id/usb-JetFlash_Transcend_4GB_16DLP0JADLM1F6WM-0:0 -> ../../sdc
/dev/disk/by-id/usb-JetFlash_Transcend_4GB_16DLP0JADLM1F6WM-0:0-part1 -> ../../sdc1

So the proposal is an attempt to guess the media type by looking into the parent model/name and using the following pattern: 'SD|MMC|CF|MS|SM|xD|Card' to match a memory card reader.

Both removable_storage_watcher and removable_storage_test offer a new parameter (--memorycard) to help the identification but only removable_storage_test is used for the preinserted SD test.
I've also left other manual test definition as they were for the moment, keeping this experimental guess for SRU.

[lp:~sylvain-pineau/checkbox/story597_removable_test_for_SD]

1621. By Sylvain Pineau on 2012-08-30

Update search pattern with in removable_storage scripts (+Generic)

[Jeff Lane  (bladernr)]

Nice! Great enhancement! Here's the before and after testing I just did:

bladernr@klaatu:~/development/sylvain-mmc-test-enhancement$ ./scripts/removable_storage_test -l sdio usb scsi
--------------------
Removable devices currently mounted:
/dev/mmcblk0p1 : /media/8765-4321
/dev/sdb1 : /media/1AE8-8249
Removable devices currently not mounted:
None
--------------------
bladernr@klaatu:~/development/sylvain-mmc-test-enhancement$ ./scripts/removable_storage_test --memorycard -l sdio usb scsi
--------------------
Removable devices currently mounted:
/dev/mmcblk0p1 : /media/8765-4321
Removable devices currently not mounted:
None
--------------------

Note that sdb on my machine is a usb key... and it properly ignores the usb key!

Approve

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-08-29 20:52:33 +0000
+++ debian/changelog	2012-08-30 09:35:22 +0000
@@ -7,6 +7,9 @@
     properly with KVM.
   * jobs/suspend.txt.in, scripts/gpu_test: Update the job description and the
     script docstrings from Flash to HTML5 video playback.
+  * [FEATURE] scripts/removable_storage_test, scripts/removable_storage_watcher,
+    jobs/mediacard.txt.in: Added memory cards detection (on bus other than sdio)
+    and a new automated (based on usb/storage-pre-inserted) for SD cards.
 
   [Jeff Marcom]
   * scripts/accelerometer_test, jobs/input.txt.in: Improved ability to detect 

=== modified file 'jobs/mediacard.txt.in'
--- jobs/mediacard.txt.in	2012-07-11 17:28:09 +0000
+++ jobs/mediacard.txt.in	2012-08-30 09:35:22 +0000
@@ -176,6 +176,17 @@
      The verification of this test is automated. Do not change the
      automatically selected result.
 
+plugin: shell
+name: mediacard/sd-preinserted
+user: root
+requires:
+ package.name == 'udisks'
+command: removable_storage_test --memorycard -l sdio usb scsi && removable_storage_test --memorycard sdio usb scsi
+_description:
+ This is a fully automated version of mediacard/sd-automated and assumes that the
+ system under test has a memory card device plugged in prior to checkbox execution.
+ It is intended for SRU automated testing.
+
 plugin: manual
 name: mediacard/sdhc-insert
 requires:

=== modified file 'scripts/removable_storage_test'
--- scripts/removable_storage_test	2012-08-21 19:44:07 +0000
+++ scripts/removable_storage_test	2012-08-30 09:35:22 +0000
@@ -4,6 +4,7 @@
 import sys
 import random
 import os
+import re
 import tempfile
 import hashlib
 import argparse
@@ -64,7 +65,7 @@
 
 
 class DiskTest():
-    ''' Class to contain various methods for testing USB disks '''
+    ''' Class to contain various methods for testing removable disks '''
     def __init__(self):
         self.process = None
         self.cmd = None
@@ -130,7 +131,7 @@
             print("ERROR: Unable to remove tempfile %s" % target,
                 file=sys.stderr)
 
-    def get_disk_info(self, device):
+    def get_disk_info(self, args):
         ''' Probes dbus to find any attached/mounted devices'''
         bus = dbus.SystemBus()
         ud_manager_obj = bus.get_object("org.freedesktop.UDisks",
@@ -139,23 +140,36 @@
         self.rem_disks = {}
         self.rem_disks_speed = {}
         self.rem_disks_nm = {}
+        self.rem_disks_memory_cards = {}
+        self.rem_disks_memory_cards_nm = {}
         for dev in ud_manager.EnumerateDevices():
             device_obj = bus.get_object("org.freedesktop.UDisks", dev)
             device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
             udisks = 'org.freedesktop.UDisks.Device'
             if not device_props.Get(udisks, "DeviceIsDrive"):
-                if device_props.Get(udisks,
-                                    "DriveConnectionInterface") in device:
+                dev_bus = device_props.Get(udisks, "DriveConnectionInterface")
+                if dev_bus in args.device:
+                    if args.memorycard and dev_bus != 'sdio':
+                        device_file_by_id = device_props.Get(udisks,
+                            "DeviceFileById")[0]
+                        if not re.search('SD|MMC|CF|MS|SM|xD|Card|Generic',
+                            device_file_by_id, re.I):
+                            continue
+                        else:
+                            self.rem_disks_memory_cards = {}
                     dev_file = str(device_props.Get(udisks, "DeviceFile"))
-                    dev_speed = str(device_props.Get(udisks, 
+                    dev_speed = str(device_props.Get(udisks,
                                                      "DriveConnectionSpeed"))
                     self.rem_disks_speed[dev_file] = dev_speed
                     if len(device_props.Get(udisks, "DeviceMountPaths")) > 0:
                         devPath = str(device_props.Get(udisks,
                                                        "DeviceMountPaths")[0])
                         self.rem_disks[dev_file] = devPath
+                        self.rem_disks_memory_cards[dev_file] = devPath
                     else:
                         self.rem_disks_nm[dev_file] = None
+                        self.rem_disks_memory_cards_nm[dev_file] = None
+
 
     def mount(self):
         passed_mount = {}
@@ -193,6 +207,8 @@
 
         for disk in self.rem_disks_nm:
             result = False
+            if not self.rem_disks_nm[disk]:
+                continue
             try:
                 result = self.make_thread(self._umount(disk))
             except:
@@ -213,6 +229,8 @@
 
     def clean_tmp_dir(self):
         for disk in self.rem_disks_nm:
+            if not self.rem_disks_nm[disk]:
+                continue
             if not os.path.ismount(self.rem_disks_nm[disk]):
                 os.rmdir(self.rem_disks_nm[disk])
 
@@ -281,12 +299,16 @@
                         default=False,
                         help=("skip the removable devices "
                               "which haven't been mounted before the test."))
+    parser.add_argument('--memorycard', action="store_true",
+                        help=("Memory cards devices on bus other than sdio "
+                              "require this parameter to identify "
+                              "them as such"))
 
     args = parser.parse_args()
 
     test = DiskTest()
 
-    test.get_disk_info(args.device)
+    test.get_disk_info(args)
 
     errors = 0
     # If we do have removable drives attached and mounted
@@ -294,24 +316,38 @@
         if args.list:  # Simply output a list of drives detected
             print('-' * 20)
             print("Removable devices currently mounted:")
-            if len(test.rem_disks) > 0:
-                for disk, mount_point in test.rem_disks.items():
-                    print("%s : %s" % (disk, mount_point))
-            else:
-                print("None")
-
-            print("Removable devices currently not mounted:")
-            if len(test.rem_disks_nm) > 0:
-                for disk, dummy in test.rem_disks_nm.items():
-                    print(disk)
-            else:
-                print("None")
+            if args.memorycard:
+                if len(test.rem_disks_memory_cards) > 0:
+                    for disk, mnt_point in test.rem_disks_memory_cards.items():
+                        print("%s : %s" % (disk, mnt_point))
+                else:
+                    print("None")
+
+                print("Removable devices currently not mounted:")
+                if len(test.rem_disks_memory_cards_nm) > 0:
+                    for disk, dummy in test.rem_disks_memory_cards_nm.items():
+                        print(disk)
+                else:
+                    print("None")
+            else:
+                if len(test.rem_disks) > 0:
+                    for disk, mnt_point in test.rem_disks.items():
+                        print("%s : %s" % (disk, mnt_point))
+                else:
+                    print("None")
+
+                print("Removable devices currently not mounted:")
+                if len(test.rem_disks_nm) > 0:
+                    for disk, dummy in test.rem_disks_nm.items():
+                        print(disk)
+                else:
+                    print("None")
 
             print('-' * 20)
 
             return 0
 
-        else:  # Create a file, copy to USB and compare hashes
+        else:  # Create a file, copy to disk and compare hashes
             if args.skip_not_mount:
                 disks_all = test.rem_disks
             else:
@@ -332,7 +368,7 @@
 
                 for disk, mount_point in disks_all.items():
                     supported_speed = test.rem_disks_speed[disk]
-                    print("    %s : %s : %s bits/s" % 
+                    print("    %s : %s : %s bits/s" %
                             (disk, mount_point, supported_speed),
                             end="")
                     if (args.min_speed and int(args.min_speed) > int(supported_speed)):
@@ -426,7 +462,7 @@
                            "but there were errors" % args.count)
                     return 1
                 elif len(disks_eligible) == 0:
-                    print("ERROR: No %s disks with speed higher than %s bits/s" % 
+                    print("ERROR: No %s disks with speed higher than %s bits/s" %
                             (args.device, args.min_speed), file=sys.stderr)
                     return 1
 

=== modified file 'scripts/removable_storage_watcher'
--- scripts/removable_storage_watcher	2012-08-21 19:30:10 +0000
+++ scripts/removable_storage_watcher	2012-08-30 09:35:22 +0000
@@ -1,5 +1,6 @@
 #!/usr/bin/env python3
 
+import re
 import sys
 import dbus
 import argparse
@@ -10,13 +11,15 @@
 
 class StorageDeviceListener:
 
-    def __init__(self, action, devices, minimum_speed ):
+    def __init__(self, action, devices, minimum_speed, memorycard ):
         self._action = action
         self._devices = devices
         self._minimum_speed = minimum_speed
+        self._memorycard = memorycard
         self._bus = dbus.SystemBus(mainloop=DBusGMainLoop())
         self._loop = GObject.MainLoop()
         self._error = False
+        self._change_cache = []
 
     def check(self, timeout):
         udisks = 'org.freedesktop.UDisks'
@@ -46,7 +49,13 @@
         desired_bus_devices = [device for device in changed_devices \
                               if device[1] in self._devices]
         for dev in desired_bus_devices:
-            print(message % {'bus':dev[1], 'file': dev[0]})
+            if self._memorycard and dev[1] != 'sdio':
+                if not re.search('SD|MMC|CF|MS|SM|xD|Card|Generic',
+                    dev[3], re.I):
+                    return
+                print(message % {'bus':'memory card', 'file': dev[0]})
+            else:
+                print(message % {'bus':dev[1], 'file': dev[0]})
             if self._minimum_speed:
                 if dev[2] >= self._minimum_speed:
                     print("with speed of %(speed)s bits/s "
@@ -63,12 +72,16 @@
                             job_num_tasks, job_cur_task_id,
                             job_cur_task_percentage):
         if job_id == "FilesystemMount":
+            if devices in self._change_cache:
+                return
+            self._change_cache.append(devices)
             inserted_devices = list(set(self.get_existing_devices()) -
                                     set(self._starting_devices))
             self.verify_device_change(inserted_devices,
                       message="Expected %(bus)s device %(file)s inserted")
 
     def add_detected(self, added_path):
+        self._change_cache = []
         self._bus.add_signal_receiver(self.job_change_detected,
                                       signal_name='DeviceJobChanged',
                                       dbus_interface='org.freedesktop.UDisks')
@@ -93,12 +106,18 @@
                 udisks = 'org.freedesktop.UDisks.Device'
                 _device_file = device_props.Get(udisks,
                                            "DeviceFile")
+                _device_file_by_id = device_props.Get(udisks,
+                                           "DeviceFileById")[0]
                 _device = device_props.Get(udisks,
                                            "DriveConnectionInterface")
                 _speed = device_props.Get(udisks,
                                                 "DriveConnectionSpeed")
                 if not device_props.Get(udisks, "DeviceIsDrive"):
-                    device=(str(_device_file),str(_device), int(_speed))
+                    device=(
+                        str(_device_file),
+                        str(_device),
+                        int(_speed),
+                        str(_device_file_by_id))
                     existing_devices.append(device)
 
             except dbus.DBusException:
@@ -115,6 +134,10 @@
                         choices=['usb', 'sdio', 'firewire',
                         'scsi', 'ata_serial_esata'],
                         nargs=argparse.REMAINDER)
+    memorycard_help = ("Memory cards devices on bus other than sdio require "
+                       "this parameter to identify them as such")
+    parser.add_argument('--memorycard', action="store_true",
+                        help=memorycard_help)
     parser.add_argument('--timeout', type=int, default=20)
     min_speed_help = ("Will only accept a device if its connection speed "
                       "attribute is higher than this value "
@@ -122,8 +145,8 @@
     parser.add_argument('--minimum_speed', '-m', help=min_speed_help,
                         type=int, default=0)
     args = parser.parse_args()
-    listener = StorageDeviceListener(args.action, args.device, 
-                                     args.minimum_speed)
+    listener = StorageDeviceListener(args.action, args.device,
+                                     args.minimum_speed, args.memorycard)
     return(listener.check(args.timeout))
 
 if __name__ == "__main__":