Merge lp:~sylvain-pineau/checkbox/more-udisks2-spineau into lp:~zyga/checkbox/more-udisks2

[Sylvain Pineau]

This MR basically fixes two things:

1. is_memorycard() uses the imported regexp from the udevadm parser and the Udisks1 version also benefit from it.

2. To follow the Udisks1 version behavior, memory cards detected are filtered except if --memorycard is explicitly given as argument to the script.

I'd like to see these minor changes merged in your branch before approving your version.

[Zygmunt Krynicki (zyga)]

I'll cherry-pick individual pieces and propose them separately

Unmerged revisions

1731. By Sylvain Pineau on 2012-10-01

Filter memory cards if we're only interested in usb devices (same behaviour as the Udisks1 version)

1730. By Sylvain Pineau on 2012-10-01

Reused is_memory_card() for the UDisks1StorageDeviceListener

1729. By Sylvain Pineau on 2012-10-01

Merged from trunk

=== modified file 'backend'
--- backend	2012-06-01 21:38:37 +0000
+++ backend	2012-10-01 12:36:29 +0000
@@ -35,10 +35,14 @@
     while True:
         try:
             message = reader.read_object()
+            #"unpack" the message
+            sequence, message = message
             if message == "stop":
                 break
             if message == "ping":
-                writer.write_object("pong")
+                #Build a tuple with the sequence number as
+                #received
+                writer.write_object((sequence, "pong",))
                 continue
             if isinstance(message, dict) and "command" in message:
                 job = Job(message["command"], message.get("environ"),
@@ -50,8 +54,8 @@
                     status, data, duration = (FAIL, "Decode error", 0,)
             else:
                 status, data, duration = (FAIL, "", 0,)
-
-            writer.write_object((status, data, duration,))
+            #Build a tuple with sequence number
+            writer.write_object((sequence, (status, data, duration,),))
         except IOError:
             break
 

=== modified file 'checkbox/parsers/udevadm.py'
--- checkbox/parsers/udevadm.py	2012-09-25 11:52:25 +0000
+++ checkbox/parsers/udevadm.py	2012-10-01 12:36:29 +0000
@@ -59,6 +59,9 @@
     r"^scsi:"
     r"t-0x(?P<type>[%(hex)s]{2})"
     % {"hex": string.hexdigits})
+CARD_READER_RE = re.compile(r"SD|MMC|CF|MS|SM|xD|Card", re.I)
+GENERIC_RE = re.compile(r"Generic", re.I)
+FLASH_RE = re.compile(r"Flash", re.I)
 
 
 class UdevadmDevice:
@@ -208,6 +211,22 @@
             if test_bit(Input.BTN_MOUSE, bitmask, self._bits):
                 return "MOUSE"
 
+        if self.driver:
+            if self.driver.startswith("sdhci"):
+                return "CARDREADER"
+
+            if self.driver.startswith("mmc"):
+                return "CARDREADER"
+
+            if self.driver == "sd" and self.product:
+                if any(FLASH_RE.search(k) for k in self._environment.keys()):
+                    return "CARDREADER"
+                if any(d.bus == 'usb' for d in self._stack):
+                    if CARD_READER_RE.search(self.product):
+                        return "CARDREADER"
+                    if GENERIC_RE.search(self.vendor):
+                        return "CARDREADER"
+
         if "ID_TYPE" in self._environment:
             id_type = self._environment["ID_TYPE"]
 

=== modified file 'checkbox/tests/message_files.py'
--- checkbox/tests/message_files.py	2012-09-19 16:12:11 +0000
+++ checkbox/tests/message_files.py	2012-10-01 12:36:29 +0000
@@ -32,32 +32,41 @@
             jobs_path = "./jobs"
 
         for filename in path_expand_recursive(jobs_path):
-            if not filename.endswith("~"):
-                file = open(filename, "r", encoding="utf-8")
+            basename = os.path.basename(filename)
+            if not basename.startswith(".") and not basename.endswith("~"):
                 template = TemplateI18n()
-                messages += template.load_file(file, filename)
+                messages += template.load_filename(filename)
+
         return messages
 
     def setUp(self):
         self.messages = self.read_jobs()
 
     def test_job_files_valid(self):
-        messages = self.messages
-        self.assertTrue(messages)
-        self.assertTrue(len(messages) > 0)
+        self.assertTrue(self.messages)
+        self.assertTrue(len(self.messages) > 0)
 
     def test_all_messages_have_name(self):
-        messages = self.messages
-        for message in messages:
+        for message in self.messages:
             self.assertIn("name", message)
 
     def test_all_messages_have_command_or_description(self):
-        messages = self.messages
-        for message in messages:
+        for message in self.messages:
             self.assertTrue("command" in message or "description" in message)
-    
+
     def test_shell_jobs_have_description(self):
-        messages = self.messages
-        for message in messages:
-            if message['plugin']=='shell':
+        for message in self.messages:
+            if message['plugin'] == 'shell':
                 self.assertTrue("description" in message, message['name'])
+
+    def test_jobs_comply_with_schema(self):
+        globals = {}
+        exec(open("plugins/jobs_info.py").read(), globals)
+        job_schema = globals["job_schema"]
+        for message in self.messages:
+            long_ext = "_extended"
+            for long_key in list(message.keys()):
+                if long_key.endswith(long_ext):
+                    short_key = long_key.replace(long_ext, "")
+                    message[short_key] = message.pop(long_key)
+            job_schema.coerce(message)

=== modified file 'debian/changelog'
--- debian/changelog	2012-09-27 18:10:45 +0000
+++ debian/changelog	2012-10-01 12:36:29 +0000
@@ -62,6 +62,10 @@
     that all shell jobs have descriptions (LP: #1052992).
   * [FEATURE] jobs/stress.txt.in: Updated some shell jobs that had no
     description (LP: #1052992).
+  * Added consecutive numbering to messages sent to the backend, so the
+    frontend knows to discard out-of-sequence messages. (LP: #886118)
+  * [FEATURE] Added a test to verify that jobs contain only keys
+    declared in the schema (avoid stray keys).
 
   [Alberto Milone]
   * [FEATURE] scripts/window_test, jobs/graphics.txt.in: Added script
@@ -161,6 +165,12 @@
   * checkbox/parsers/udevadm.py: Improved wireless devices detection.
     The wireless category is now set if the subsystem is equal to ieee80211 
     (LP: #855382)
+  * scripts/memorycard_resource, scripts/removable_storage_test,
+    scripts/removable_storage_watcher: Fixed the memorycard regexp flags and 
+    add the DriveVendor Udisks property to the re.search() string (LP: #1050920)
+  * scripts/display_resource, jobs/resource.txt.in: Added a new display 
+    resource script to properly handle connector names returned by proprietary
+    drivers (LP: #956139 and #992727)
 
   [Zygmunt Krynicki]
   * Fixed simple duplicate 'the' mistakes (LP: #1040022)
@@ -172,7 +182,7 @@
     the 'device' argument (bus type) and require at least one value
   * [FEATURE] scripts/removable_storage_watcher: add support for debugging
 
- -- Jeff Marcom <[e-mail]>  Thu, 20 Sep 2012 18:03:21 -0400
+ -- Daniel Manrique <[e-mail]>  Thu, 27 Sep 2012 13:02:24 -0400
 
 checkbox (0.14.5) quantal; urgency=low
 

=== modified file 'jobs/mediacard.txt.in'
--- jobs/mediacard.txt.in	2012-09-04 19:15:43 +0000
+++ jobs/mediacard.txt.in	2012-10-01 12:36:29 +0000
@@ -181,7 +181,7 @@
 user: root
 requires:
  package.name == 'udisks'
- memory_card.reader == 'detected'
+ device.category == 'CARDREADER'
 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

=== modified file 'jobs/resource.txt.in'
--- jobs/resource.txt.in	2012-09-06 09:42:43 +0000
+++ jobs/resource.txt.in	2012-10-01 12:36:29 +0000
@@ -76,7 +76,8 @@
 
 name: display
 plugin: resource
-command: for display in `xrandr | grep connected | awk '{print $1}' | grep -o ^[A-Z]* | sort | uniq`; do echo "$display: supported"; done
+command: display_resource
+description: Creates display resource info from xrandr output
 
 name: usb
 plugin: resource
@@ -100,9 +101,3 @@
  for e in `env | sed 's/=/:/g'`; do
      echo $e | awk -F':' '{print tolower($1) ": " $2}'
  done
-
-name: memory_card
-plugin: resource
-user: root
-command: memorycard_resource
-description: Create resource info for memorycard readers

=== modified file 'plugins/backend_info.py'
--- plugins/backend_info.py	2012-06-22 16:33:41 +0000
+++ plugins/backend_info.py	2012-10-01 12:36:29 +0000
@@ -26,11 +26,12 @@
 from checkbox.lib.fifo import FifoReader, FifoWriter, create_fifo
 
 from checkbox.plugin import Plugin
-from checkbox.properties import Path, Float 
+from checkbox.properties import Path, Float
 from checkbox.job import FAIL
 
 from gettext import gettext as _
 
+
 class BackendInfo(Plugin):
 
     # how long to wait for I/O from/to the backend before the call returns.
@@ -39,6 +40,37 @@
 
     command = Path(default="%(checkbox_share)s/backend")
 
+    next_sequence = 0
+    expected_sequence = 0
+
+    def write_to_parent(self, object):
+        message = (self.next_sequence, object,)
+        logging.debug("Sending message with sequence number %s to backend" %
+                       self.next_sequence)
+        self.parent_writer.write_object(message)
+        self.expected_sequence = self.next_sequence
+        self.next_sequence += 1
+
+    def read_from_parent(self):
+        correct_sequence = False
+        while not correct_sequence:
+            ro = self.parent_reader.read_object()
+            if ro:
+                sequence, result = ro
+                logging.debug("Expecting sequence number %s from backend, "
+                              "got sequence number %s" %
+                       (self.expected_sequence, sequence))
+                if (self.expected_sequence == sequence):
+                    correct_sequence = True
+                else:
+                    logging.warning("Backend sent wrong sequence number, "
+                                    "Discarding message and re-reading")
+            else:
+                #If we timed out, just return nothing, the rest of
+                #the code knows how to handle this.
+                return ro
+        return result
+
     def register(self, manager):
         super(BackendInfo, self).register(manager)
 
@@ -57,7 +89,10 @@
 
     def get_root_command(self, *args):
         uid = os.getuid()
-        password_text = _("SYSTEM TESTING: Please enter your password. Some tests require root access to run properly. Your password will never be stored and will never be submitted with test results.")
+        password_text = _("SYSTEM TESTING: Please enter your password. "
+                          "Some tests require root access to run properly. "
+                          "Your password will never be stored and will never "
+                          "be submitted with test results.")
         password_prompt = _("PASSWORD: ")
         if uid == 0:
             prefix = []
@@ -88,31 +123,35 @@
     def ping_backend(self):
         if not self.parent_reader or not self.parent_writer:
             return False
-        self.parent_writer.write_object("ping")
-        result = self.parent_reader.read_object()
+        self.write_to_parent("ping")
+        result = self.read_from_parent()
         return result == "pong"
 
-
     def gather(self):
         self.directory = mkdtemp(prefix="checkbox")
-        child_input = create_fifo(os.path.join(self.directory, "input"), 0o600)
-        child_output = create_fifo(os.path.join(self.directory, "output"), 0o600)
+        child_input = create_fifo(os.path.join(self.directory, "input"),
+                                  0o600)
+        child_output = create_fifo(os.path.join(self.directory, "output"),
+                                   0o600)
 
         self.backend_is_alive = False
-        for attempt in range(1,4):
+        for attempt in range(1, 4):
             self.spawn_backend(child_input, child_output)
-            #Only returns if I'm still the parent, so I can do parent stuff here
+            #Only returns if I'm still the parent,
+            #so I can do parent stuff here
             self.parent_writer = FifoWriter(child_input, timeout=self.timeout)
-            self.parent_reader = FifoReader(child_output, timeout=self.timeout)
+            self.parent_reader = FifoReader(child_output,
+                                            timeout=self.timeout)
             if self.ping_backend():
                 logging.debug("Backend responded, continuing execution.")
                 self.backend_is_alive = True
                 break
             else:
-                logging.debug("Backend didn't respond, trying to create again.")
+                logging.debug("Backend didn't respond, "
+                              "trying to create again.")
 
-        if not self.backend_is_alive: 
-            logging.warning("Privileged backend not responding. " + 
+        if not self.backend_is_alive:
+            logging.warning("Privileged backend not responding. " +
                             "jobs specifying user will not be run")
 
     def message_exec(self, message):
@@ -120,20 +159,20 @@
             if "environ" in message:
                 #Prepare variables to be "exported" from my environment
                 #to the backend's.
-                backend_environ=["%s=%s" % (key, os.environ[key])
-                             for key in message["environ"] 
+                backend_environ = ["%s=%s" % (key, os.environ[key])
+                             for key in message["environ"]
                              if key in os.environ]
-                message=dict(message) #so as to not wreck the
-                                      #original message
-                message["environ"]=backend_environ
+                message = dict(message)  # so as to not wreck the
+                                         # original message
+                message["environ"] = backend_environ
 
             if (self.backend_is_alive and not self.ping_backend()):
                 self.backend_is_alive = False
 
             if self.backend_is_alive:
-                self.parent_writer.write_object(message)
+                self.write_to_parent(message)
                 while True:
-                    result = self.parent_reader.read_object()
+                    result = self.read_from_parent()
                     if result:
                         break
                     else:
@@ -145,7 +184,7 @@
                 self._manager.reactor.fire("message-result", *result)
 
     def stop(self):
-        self.parent_writer.write_object("stop")
+        self.write_to_parent("stop")
         self.parent_writer.close()
         self.parent_reader.close()
         shutil.rmtree(self.directory)

=== added file 'scripts/display_resource'
--- scripts/display_resource	1970-01-01 00:00:00 +0000
+++ scripts/display_resource	2012-10-01 12:36:29 +0000
@@ -0,0 +1,84 @@
+#!/usr/bin/python3
+
+import re
+import sys
+import subprocess
+
+CONNECTOR_RE = re.compile(
+    r"\n(?P<Name>[\w\-]+)"
+    r"(?:\s+.*?"
+    r"SignalFormat:\s+(?P<SignalFormat>[\w\-_]+)\s+.*?"
+    r"ConnectorType:\s+(?P<ConnectorType>[\w\-_]+))?", re.S)
+SVIDEO_RE = re.compile(r"s\-?video|din|cv", re.I)
+DP_RE = re.compile(r"dp|displayport", re.I)
+
+
+def main():
+    try:
+        xrandr_output = subprocess.check_output(
+            ["xrandr", "-q", "--verbose"], universal_newlines=True)
+    except subprocess.CalledProcessError:
+        return 0
+
+    xrandr_output = "\n" + ''.join(xrandr_output.splitlines(True)[1:])
+    supported_connections = dict()
+
+    for m in CONNECTOR_RE.finditer(xrandr_output):
+        name = m.group('Name').lower()
+        signal_format = connector_type = ''  # RandR 1.3 only
+        if m.group('SignalFormat'):
+            signal_format = m.group('SignalFormat').lower()
+        if m.group('ConnectorType'):
+            connector_type = m.group('ConnectorType').lower()
+
+        if name.startswith('vga'):
+            supported_connections['vga'] = 'supported'
+
+        elif name.startswith('dvi'):
+            supported_connections['dvi'] = 'supported'
+
+        elif DP_RE.match(name):
+            # HDMI uses TMDS links, DisplayPort (DP) uses LVDS pairs (lanes)
+            # to transmit micro data packets.
+            # Reported by NVIDIA proprietary drivers (version >= 302.17)
+            if signal_format == 'tmds':
+                supported_connections['hdmi'] = 'supported'
+            else:
+                supported_connections['dp'] = 'supported'
+
+        elif name.startswith('hdmi'):
+            supported_connections['hdmi'] = 'supported'
+
+        elif SVIDEO_RE.match(name):
+            supported_connections['svideo'] = 'supported'
+
+        elif name.startswith('rca'):
+            supported_connections['rca'] = 'supported'
+
+        elif name.startswith('tv'):
+            if SVIDEO_RE.match(signal_format):
+                supported_connections['svideo'] = 'supported'
+            else:
+                supported_connections['tv'] = 'supported'
+
+        # ATI/AMD proprietary FGLRX graphics driver codenames:
+        elif name.startswith('crt') or name.startswith('dfp'):
+            if connector_type.startswith('dvi'):
+                supported_connections['dvi'] = 'supported'
+            elif connector_type.startswith('vga'):
+                supported_connections['vga'] = 'supported'
+            elif DP_RE.match(connector_type) and DP_RE.match(signal_format):
+                supported_connections['dp'] = 'supported'
+            else:
+                # HDMI ports may appear as unknown
+                # (for both signal format and connector type).
+                supported_connections['hdmi'] = 'supported'
+
+    for connector in supported_connections.keys():
+        print("%s: supported" % connector)
+
+    return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

=== removed file 'scripts/memorycard_resource'
--- scripts/memorycard_resource	2012-09-05 20:09:10 +0000
+++ scripts/memorycard_resource	1970-01-01 00:00:00 +0000
@@ -1,143 +0,0 @@
-#!/usr/bin/python3
-#
-# This file is part of Checkbox.
-#
-# Copyright 2012 Canonical Ltd.
-#
-# Checkbox is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Checkbox is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Checkbox.  If not, see <http://www.gnu.org/licenses/>.
-#
-
-import re
-import sys
-import fcntl
-import struct
-import ctypes
-from glob import glob
-from subprocess import call
-
-
-class sg_io_hdr(ctypes.Structure):
-    """
-    Do ioctl's using Linux generic SCSI interface.
-    """
-    SG_IO = 0x2285
-
-    _fields_ = [("interface_id", ctypes.c_int),
-                ("dxfer_direction", ctypes.c_int),
-                ("cmd_len", ctypes.c_ubyte),
-                ("mx_sb_len", ctypes.c_ubyte),
-                ("iovec_count", ctypes.c_ushort),
-                ("dxfer_len", ctypes.c_int),
-                ("dxferp", ctypes.c_char_p),
-                ("cmdp", ctypes.c_char_p),
-                ("sbp", ctypes.c_char_p),
-                ("timeout", ctypes.c_uint),
-                ("flags", ctypes.c_uint),
-                ("pack_id", ctypes.c_int),
-                ("usr_ptr", ctypes.c_char_p),
-                ("status", ctypes.c_ubyte),
-                ("masked_status", ctypes.c_ubyte),
-                ("msg_status", ctypes.c_ubyte),
-                ("sb_len_wr", ctypes.c_ubyte),
-                ("host_status", ctypes.c_ushort),
-                ("driver_status", ctypes.c_ushort),
-                ("resid", ctypes.c_int),
-                ("duration", ctypes.c_uint),
-                ("info", ctypes.c_uint)]
-
-    def __init__(self):
-        self.interface_id = ord('S')
-        self.dxfer_direction = 0
-        self.cmd_len = 0
-        self.mx_sb_len = 0
-        self.iovec_count = 0
-        self.dxfer_len = 0
-        self.dxferp = None
-        self.cmdp = None
-        self.timeout = 20000
-        self.flags = 0
-        self.pack_id = 0
-        self.usr_ptr = None
-        self.status = 0
-        self.masked_status = 0
-        self.msg_status = 0
-        self.sb_len_wr = 0
-        self.host_status = 0
-        self.driver_status = 0
-        self.resid = 0
-        self.duration = 0
-        self.info = 0
-
-
-def _get_sub_page(size, control_block, device):
-    """
-    ioctl to retrieve a sub-page from the device.
-    """
-    io_hdr = sg_io_hdr()
-    io_hdr.dxfer_direction = -3
-    io_hdr.cmd_len = len(control_block)
-    io_hdr.cmdp = control_block
-    page_buffer = ctypes.create_string_buffer(size)
-    io_hdr.dxfer_len = size
-    io_hdr.dxferp = ctypes.cast(page_buffer, ctypes.c_char_p)
-
-    i = -1
-    try:
-        with open(device, 'r') as fd:
-            i = fcntl.ioctl(fd, sg_io_hdr.SG_IO, io_hdr)
-    except IOError:
-        return None
-
-    if i < 0:
-        raise IOError("SG_IO ioctl error")
-
-    if io_hdr.status != 0x00:
-        raise IOError("io_hdr status is: %x" % io_hdr.status)
-
-    if io_hdr.resid > 0:
-        retsize = size - io_hdr.resid
-    else:
-        retsize = size
-
-    return page_buffer[0:retsize]
-
-
-def inquire(device):
-    """
-    Issue a SCSI INQUIRY command
-    """
-    fields = '>BBBBBBBB8s16s4s20sBB8HH'
-    fields_len = struct.calcsize(fields)
-    control_block = struct.pack("BBBBBB", 0x12, 0, 0, 0, fields_len, 0)
-    try:
-        return _get_sub_page(fields_len, control_block, device)[8:32].decode()
-    except TypeError:
-        return ''
-
-
-def main():
-    pattern = re.compile('SD|MMC|CF|MS|SM|xD|Card|Generic')
-    matches = [dev for dev in glob('/dev/sg*') if
-               pattern.search(inquire(dev), re.I)]
-
-    if matches or not call(
-        "grep -q 'SDHCI controller found' /var/log/kern.log*", shell=True):
-        print('reader: detected')
-    else:
-        print('reader: none')
-
-    return 0
-
-if __name__ == "__main__":
-    sys.exit(main())

=== modified file 'scripts/optical_write_test'
--- scripts/optical_write_test	2012-09-24 14:39:56 +0000
+++ scripts/optical_write_test	2012-10-01 12:36:29 +0000
@@ -68,14 +68,14 @@
     while true; do
         sleep $INTERVAL
         SLEEP_COUNT=`expr $SLEEP_COUNT + $INTERVAL`
-
+        
         mount $OPTICAL_DRIVE 2>&1 |egrep -q "already mounted"
         rt=$?
         if [ $rt -eq 0 ]; then
             echo "Drive appears to be mounted now"
             break
         fi
-
+        
         # If they exceed the timeout limit, make a best effort to load the tray
         # in the next steps
         if [ $SLEEP_COUNT -ge $TIMEOUT ]; then
@@ -84,6 +84,7 @@
         fi
     done
 
+        
     echo "Deleting original data files ..."
     rm -rf $SAMPLE_FILE
     if [ ! -z "$(mount | grep $OPTICAL_DRIVE)" ]; then
@@ -130,7 +131,7 @@
     OPTICAL_DRIVE=$(readlink -f $1)
 else
     OPTICAL_DRIVE='/dev/sr0'
-fi
+fi    
 
 create_working_dirs || failed "Failed to create working directories"
 get_sample_data || failed "Failed to copy sample data"

=== modified file 'scripts/removable_storage_test'
--- scripts/removable_storage_test	2012-09-06 09:27:13 +0000
+++ scripts/removable_storage_test	2012-10-01 12:36:29 +0000
@@ -4,7 +4,6 @@
 import sys
 import random
 import os
-import re
 import tempfile
 import hashlib
 import argparse
@@ -12,6 +11,7 @@
 import threading
 import time
 import collections
+from checkbox.parsers.udevadm import CARD_READER_RE, GENERIC_RE, FLASH_RE
 
 
 class ActionTimer():
@@ -142,7 +142,6 @@
         self.rem_disks_nm = {}
         self.rem_disks_memory_cards = {}
         self.rem_disks_memory_cards_nm = {}
-        pattern = re.compile('SD|MMC|CF|MS|SM|xD|Card|Generic')
         for dev in ud_manager.EnumerateDevices():
             device_obj = bus.get_object("org.freedesktop.UDisks", dev)
             device_props = dbus.Interface(device_obj, dbus.PROPERTIES_IFACE)
@@ -150,20 +149,26 @@
             if not device_props.Get(udisks, "DeviceIsDrive"):
                 dev_bus = device_props.Get(udisks, "DriveConnectionInterface")
                 if dev_bus in args.device:
-                    parent_model = ''
+                    parent_model = parent_vendor = ''
                     if device_props.Get(udisks, "DeviceIsPartition"):
                         parent_obj = bus.get_object(
                             "org.freedesktop.UDisks",
                             device_props.Get(udisks, "PartitionSlave"))
                         parent_props = dbus.Interface(
                             parent_obj, dbus.PROPERTIES_IFACE)
-                        parent_model=parent_props.Get(udisks, "DriveModel")
+                        parent_model = parent_props.Get(udisks, "DriveModel")
+                        parent_vendor = parent_props.Get(udisks, "DriveVendor")
+                        parent_media = parent_props.Get(udisks, "DriveMedia")
                     if args.memorycard:
-                        if dev_bus != 'sdio' and not pattern.search(
-                            parent_model, re.I):
+                        if (dev_bus != 'sdio'
+                            and not FLASH_RE.search(parent_media)
+                            and not CARD_READER_RE.search(parent_model)
+                            and not GENERIC_RE.search(parent_vendor)):
                             continue
                     else:
-                        if pattern.search(parent_model, re.I):
+                        if (FLASH_RE.search(parent_media)
+                            or CARD_READER_RE.search(parent_model)
+                            or GENERIC_RE.search(parent_vendor)):
                             continue
                     dev_file = str(device_props.Get(udisks, "DeviceFile"))
                     dev_speed = str(device_props.Get(udisks,
@@ -178,7 +183,6 @@
                         self.rem_disks_nm[dev_file] = None
                         self.rem_disks_memory_cards_nm[dev_file] = None
 
-
     def mount(self):
         passed_mount = {}
 
@@ -379,7 +383,8 @@
                     print("    %s : %s : %s bits/s" %
                             (disk, mount_point, supported_speed),
                             end="")
-                    if (args.min_speed and int(args.min_speed) > int(supported_speed)):
+                    if (args.min_speed
+                        and int(args.min_speed) > int(supported_speed)):
                         print(" (Will not test it, speed is below %s bits/s)" %
                                 args.min_speed, end="")
 
@@ -387,9 +392,10 @@
 
                 print('-' * 20)
 
-                disks_eligible = {disk: disks_all[disk] for disk in disks_all \
-                                  if not args.min_speed or \
-                                  int(test.rem_disks_speed[disk]) >= int(args.min_speed) }
+                disks_eligible = {disk: disks_all[disk] for disk in disks_all
+                                  if not args.min_speed or
+                                  int(test.rem_disks_speed[disk])
+                                  >= int(args.min_speed)}
                 write_sizes = []
                 test_files = {}
                 # Generate our data file(s)
@@ -441,8 +447,9 @@
                                 test.clean_up(file)
                             total_write_time = sum(write_times)
                             avg_write_time = total_write_time / args.count
-                            avg_write_speed = (
-                                total_write_size / total_write_time) / 1024 / 1024
+                            avg_write_speed = ((
+                                total_write_size / total_write_time)
+                                / 1024 / 1024)
                             iteration_write_times.append(total_write_time)
                             print("\t[Iteration %s] Average Speed: %0.4f"
                                   % (iteration, avg_write_speed))
@@ -470,25 +477,27 @@
                            "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" %
-                            (args.device, args.min_speed), file=sys.stderr)
+                    print("ERROR: No %s disks with speed higher than %s bits/s"
+                            % (args.device, args.min_speed), file=sys.stderr)
                     return 1
 
                 else:
                     #Pass is not assured!
-                    if not args.pass_speed or avg_write_speed >= args.pass_speed:
+                    if (not args.pass_speed or
+                        avg_write_speed >= args.pass_speed):
                         return 0
                     else:
                         print("FAIL: Average speed was lower than desired "
                               "pass speed of %s MB/s" % args.pass_speed)
                         return 1
             else:
-                print("ERROR: No device being mounted successfully for testing, "
-                      "aborting", file=sys.stderr)
+                print("ERROR: No device being mounted successfully "
+                      "for testing, aborting", file=sys.stderr)
                 return 1
 
     else:  # If we don't have removable drives attached and mounted
-        print("ERROR: No removable drives were detected, aborting", file=sys.stderr)
+        print("ERROR: No removable drives were detected, aborting",
+              file=sys.stderr)
         return 1
 
 if __name__ == '__main__':

=== modified file 'scripts/removable_storage_watcher'
--- scripts/removable_storage_watcher	2012-09-27 20:29:22 +0000
+++ scripts/removable_storage_watcher	2012-10-01 12:36:29 +0000
@@ -5,7 +5,6 @@
 import copy
 import dbus
 import logging
-import re
 import sys
 
 from dbus.exceptions import DBusException
@@ -13,6 +12,12 @@
 
 from checkbox.dbus import connect_to_system_bus
 from checkbox.dbus.udisks2 import UDisks2Model, UDisks2Observer
+from checkbox.parsers.udevadm import CARD_READER_RE, GENERIC_RE, FLASH_RE
+
+# Subset of device properties for Udisks1
+Properties = collections.namedtuple(
+    "Properties",
+    "file bus speed model vendor media")
 
 # Delta record that encapsulates difference:
 # delta_dir -- directon of the difference, either DELTA_DIR_PLUS or
@@ -78,18 +83,21 @@
     return "{}{}B".format(size // factor, prefix.strip())
 
 
-def is_memory_card(vendor, model, udisks2_media):
+def is_memory_card(vendor, model, media):
     """
     Check if the device seems to be a memory card
 
-    This is rather fuzzy, sadly udev and udisks2 don't do a very good job and
+    This is rather fuzzy, sadly udev and udisks don't do a very good job and
     mostly don't specify the "media" property (it has a few useful values, such
     as flash_cf, flash_ms, flash_sm, flash_sd, flash_sdhc, flash_sdxc and
     flash_mmc but I have yet to see a device that reports such values)
     """
+    # Treat any media that starts with 'flash' as a memory card
+    if media is not None and FLASH_RE.search(media):
+        return True
     # Treat any device that match model name to the following regular
     # expression as a memory card reader.
-    if re.search('SD|MMC|CF|MS|SM|xD|Card', model, re.I):
+    if CARD_READER_RE.search(model):
         return True
     # Treat any device that contains the word 'Generic' in the vendor string as
     # a memory card reader.
@@ -97,10 +105,7 @@
     # XXX: This seems odd but strangely enough seems to gets the job done. I
     # guess if I should start filing tons of bugs/patches on udev/udisks2 to
     # just have a few more rules and make this rule obsolete.
-    if re.match('Generic', vendor, re.I):
-        return True
-    # Treat any udisks2_media that starts with 'flash' as a memory card
-    if udisks2_media is not None and udisks2_media.startswith('flash'):
+    if GENERIC_RE.search(vendor):
         return True
     return False
 
@@ -157,33 +162,36 @@
         desired_bus_devices = [
             device
             for device in changed_devices
-            if device[1] in self._devices]
+            if device.bus in self._devices]
         logging.debug("Desired bus devices: %s", desired_bus_devices)
-        pattern = re.compile('SD|MMC|CF|MS|SM|xD|Card|Generic')
         for dev in desired_bus_devices:
+            # Ensure it is a media card reader if this was explicitly requested
+            drive_is_reader = is_memory_card(dev.vendor, dev.model, dev.media)
             if self._memorycard:
-                if dev[1] != 'sdio' and not pattern.search(dev[3], re.I):
-                    logging.debug("The device does not seem to be a memory"
-                                  " card (dev[1]: %r, dev[3]: %r), skipping",
-                                 dev[1], dev[3])
+                if not drive_is_reader:
+                    logging.warning("The device does not seem to be a memory"
+                                  " card (bus: %r, model: %r), skipping",
+                                 dev.bus, dev.model)
                     return
-                print(message % {'bus': 'memory card', 'file': dev[0]})
+                print(message % {'bus': 'memory card', 'file': dev.file})
             else:
-                if pattern.search(dev[3], re.I):
-                    logging.debug("The device seems to be a memory"
-                                  " card %s (dev[3]: %r), skipping",
-                                  pattern, dev[3])
+                if drive_is_reader:
+                    logging.warning("The device seems to be a memory"
+                                  " card (bus: %r (model: %r), skipping",
+                                  dev.bus, dev.model)
                     return
-                print(message % {'bus': dev[1], 'file': dev[0]})
+                print(message % {'bus': dev.bus, 'file': dev.file})
             if self._minimum_speed:
-                if dev[2] >= self._minimum_speed:
+                if dev.speed >= self._minimum_speed:
                     print("with speed of %(speed)s bits/s "
                           "higher than %(min_speed)s bits/s" %
-                          {'speed': dev[2], 'min_speed': self._minimum_speed})
+                          {'speed': dev.speed,
+                           'min_speed': self._minimum_speed})
                 else:
                     print("ERROR: speed of %(speed)s bits/s lower "
                           "than %(min_speed)s bits/s" %
-                          {'speed': dev[2], 'min_speed': self._minimum_speed})
+                          {'speed': dev.speed,
+                           'min_speed': self._minimum_speed})
                     self._error = True
             logging.debug("Device matches requirements, exiting event loop")
             self._loop.quit()
@@ -257,11 +265,11 @@
                 udisks = 'org.freedesktop.UDisks.Device'
                 _device_file = device_props.Get(udisks,
                                            "DeviceFile")
-                _device = device_props.Get(udisks,
+                _bus = device_props.Get(udisks,
                                            "DriveConnectionInterface")
                 _speed = device_props.Get(udisks,
                                                 "DriveConnectionSpeed")
-                _parent_model = ''
+                _parent_model = _parent_vendor = ''
 
                 if device_props.Get(udisks, "DeviceIsPartition"):
                     parent_obj = self._bus.get_object(
@@ -270,13 +278,17 @@
                     parent_props = dbus.Interface(
                         parent_obj, dbus.PROPERTIES_IFACE)
                     _parent_model = parent_props.Get(udisks, "DriveModel")
+                    _parent_vendor = parent_props.Get(udisks, "DriveVendor")
+                    _parent_media = parent_props.Get(udisks, "DriveMedia")
 
                 if not device_props.Get(udisks, "DeviceIsDrive"):
-                    device = (
+                    device = Properties(
                         str(_device_file),
-                        str(_device),
+                        str(_bus),
                         int(_speed),
-                        str(_parent_model))
+                        str(_parent_model),
+                        str(_parent_vendor),
+                        str(_parent_media))
                     existing_devices.append(device)
 
             except dbus.DBusException:
@@ -443,8 +455,8 @@
         # argument uses somewhat different strings. In particular we need to
         # map 'ieee1394' to 'firewire'
         #
-        # See: http://udisks.freedesktop.org/docs/latest/gdbus-org.freedesktop.UDisks2.Drive.html#gdbus-property-org-freedesktop-UDisks2-Drive.ConnectionBus
-
+        # See: http://goo.gl/jaUAa
+        #
         # TODO: Report old unsupported possibilities that are still offered by
         # the command-line interface. We also need samples to understand how
         # UDisks2 behaves in practice on various devices.
@@ -655,14 +667,24 @@
             drive_is_reader = is_memory_card(
                 drive_props['Vendor'], drive_props['Model'],
                 drive_props['Media'])
-            if self._desired_memory_card and not drive_is_reader:
-                logging.warning(
-                    "The object %s belongs to drive %s that does not seem to"
-                    " be a media reader", object_block_device,
-                    drive_object_path)
-                # Ignore this object so that we don't spam the user twice
-                self._ignored_objects.add(object_path)
-                continue
+            if self._desired_memory_card:
+                if not drive_is_reader:
+                    logging.warning(
+                        "The object %s belongs to drive %s that does not seem"
+                        " to be a media reader", object_block_device,
+                        drive_object_path)
+                    # Ignore this object so that we don't spam the user twice
+                    self._ignored_objects.add(object_path)
+                    continue
+            else:
+                if drive_is_reader:
+                    logging.warning(
+                        "The object %s belongs to drive %s that seems to be a"
+                        " media reader", object_block_device,
+                        drive_object_path)
+                    # Ignore this object so that we don't spam the user twice
+                    self._ignored_objects.add(object_path)
+                    continue
             # TODO: Add support for detecting USB3
             # Yay, success
             results.add(object_block_device)