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

[Sylvain Pineau]

This MR fixed the memorycard regexp flags and added the DriveVendor Udisks property to the re.search() string in removable_storage* scripts and the memorycard_resource job.

[Marc Tardif (cr3)]

Out of curiosity, what's the difference between the memorycard_resource and the existing udev_resource script? It looks like I can achieve pretty much the same thing with this shell script:

export PATH=/usr/share/checkbox/scripts:$PATH
run_templates -st "udev_resource | filter_templates -w 'driver=sd'" <<'EOF' \
  | (egrep -vqi '(SD|MMC|CF|MS|SM|xD|Card|Generic)' || !grep -q 'SDHCI controller found' /var/log/kern.log*) \
    && echo 'reader: detected' \
    || echo 'reader: none'
$product
EOF

[Marc Tardif (cr3)]

Note that one advantage to the shell script is that it doesn't require any crazy call to ioctl, and another advantage is that it doesn't need to run as root since udev_resource leverages udev that exposes some useful information.

[Sylvain Pineau (sylvain-pineau)]

udev seems to offer the same results but to fully replace my crazy ioctl I would change your proposal like this:

export PATH=/usr/share/checkbox/scripts:$PATH
run_templates -st "udev_resource | filter_templates -w 'driver=sd'" <<'EOF' \
  | (egrep -qi '(SD|MMC|CF|MS|SM|xD|Card|Generic)' || grep -q 'SDHCI controller found' /var/log/kern.log*) \
    && echo 'reader: detected' \
    || echo 'reader: none'
$product $vendor
EOF

I will change my MR to reflect this very nice improvement, Thx

[Marc Tardif (cr3)]

We might want to discuss ading the MEDIACARD category in the output of the udev_resource script. However, that really depends on how confident you are about this regular expression to distinguish disk devices from multicard devices: '(SD|MMC|CF|MS|SM|xD|Card|Generic)'

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

1676. By Sylvain Pineau on 2012-09-21

the memory_card resource job now uses udev_resource output

1677. By Sylvain Pineau on 2012-09-21

Merged from trunk

[Sylvain Pineau (sylvain-pineau)]

I prefer to keep the memory_card resource job separated from udev_resource. We don't have yet enough feedback on how reliable the regexp is.

review: Needs Resubmitting

[Zygmunt Krynicki (zyga)]

24 + run_templates -st "udev_resource | filter_templates -w 'driver=sd'" <<'EOF' \
25 + | (egrep -qi '(SD|MMC|CF|MS|SM|xD|Card|Generic)' || grep -q 'SDHCI controller found' /var/log/kern.log*) \
26 + && echo 'reader: detected' \
27 + || echo 'reader: none'

This is the kind of thing that deserves an API as it needs to be reused in various places

[Zygmunt Krynicki (zyga)]

/me puts syntax nazi hat on

I'm pretty sure (just by looking at a few lines) that this is not PEP8 clean, would you mind fixing it please?

review: Needs Fixing

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

1678. By Sylvain Pineau on 2012-09-25

Move the card reader detection logic to udevadm.py

1679. By Sylvain Pineau on 2012-09-25

Merged from trunk

1680. By Sylvain Pineau on 2012-09-25

Revert unwanted changes to optical_write_test

1681. By Sylvain Pineau on 2012-09-25

PEP8 check

[Sylvain Pineau (sylvain-pineau)]

/lib/udev/rules.d/80-udisks.rules also matches devices using regexp on device model names, so it's not a bad idea after all. So now the logic is part of the udevadm.py parser with a new category: CARDREADER.

The following drivers are supported: sdhci, sdhci-pci, mmc* and sd.
For sd, the parser checks the model name (aka product in udevadm.py) and the vendor name.

Nota: The code contains several pep8 fixes in removable_storage*

review: Needs Resubmitting

[Zygmunt Krynicki (zyga)]

No objections to merge.

Looking at the delta I see two things that need fixing (but are unrelated to your change) - the one with namedtuple instead of dev[x] and the one where args.speed is not a integer (removable_storage_test) - it should be marked as such in argparse to make both command line parsing and in-test logic more reliable

You may want to get a second opinion though as I'm temporarily out of precise systems that I can test this on

review: Approve

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

1682. By Sylvain Pineau on 2012-09-25

Check if the product field exists in _environment before checking sd driver devices

[Marc Tardif (cr3)]

This looks very nice, thanks for taking the time to improve udev_resource, I now get this on my system:

path: /devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0
bus: scsi
category: CARDREADER
driver: sd
product: SAMSUNG MMCRE64G8MPP-0VA

Do you think we could import CARD_READER_RE from checkbox.parsers.udevadm in scripts/removable_storage_test and scripts/removable_storage_watcher, instead of defining the same regular expression in both places? It's not that much code reuse but that pattern seems error prone.

review: Needs Information

[Sylvain Pineau (sylvain-pineau)]

@Marc, Matching an internal SSD drive will prevent it to be used for disks tests.

Could you please pastebin the output of udisks --show-info /dev/sdx (where x is your drive letter). Don't provide the output of an existing partition like sdb2, just sdb to have the drive properties.

I think we could use the rotational media udisks property to avoid them to be detected as card readers. My system reports the following info for a usb reader:

  drive:
    vendor: Generic
    model: Ultra HS-SD/MMC
    revision: 1.82
    serial: 000000264001
    WWN:
    detachable: 1
    can spindown: 0
    rotational media: Yes, unknown rate

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

1683. By Sylvain Pineau on 2012-09-28

Card reader detection improved in udev parser, relying on udev properties first and finally and the regexp if not matches found.

1684. By Sylvain Pineau on 2012-09-29

Updated removable_storage* scripts with udevadm regexp
pep8 fixes
device property is now a namedtuple

1685. By Sylvain Pineau on 2012-10-01

Merged from trunk

1686. By Sylvain Pineau on 2012-10-01

Change removable_storage_watcher permissions (+x)

1687. By Sylvain Pineau on 2012-10-01

lint fixes

[Sylvain Pineau (sylvain-pineau)]

I reworked a bit the udevadm parser section about card readers, to follow this schema: give priority to udev rules. First identify memorycard readers with ID_DRIVE_FLASH_SD and if not matches fallback to the regexp identification. When upstream patches will identify crazy usb readers, we could safely remove those lines.

Regexp used in removable_storage* scripts ar all imported from the udevadm parser.

The bug identifying SSD drives is fixed.

=== modified file 'checkbox/parsers/udevadm.py'
--- checkbox/parsers/udevadm.py	2012-09-25 11:52:25 +0000
+++ checkbox/parsers/udevadm.py	2012-10-01 07:54:21 +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 'debian/changelog'
--- debian/changelog	2012-09-28 16:29:41 +0000
+++ debian/changelog	2012-10-01 07:54:21 +0000
@@ -165,6 +165,9 @@
   * 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)
 
   [Zygmunt Krynicki]
   * Fixed simple duplicate 'the' mistakes (LP: #1040022)

=== modified file 'jobs/mediacard.txt.in'
--- jobs/mediacard.txt.in	2012-09-04 19:15:43 +0000
+++ jobs/mediacard.txt.in	2012-10-01 07:54:21 +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 07:54:21 +0000
@@ -100,9 +100,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

=== 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 07:54:21 +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 07:54:21 +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-25 09:20:00 +0000
+++ scripts/removable_storage_watcher	2012-10-01 07:54:21 +0000
@@ -3,14 +3,17 @@
 import argparse
 import dbus
 import logging
-import re
 import sys
 
 from dbus.exceptions import DBusException
 from gi.repository import GObject
 
+from collections import namedtuple
 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
+
+Properties = namedtuple('Properties', 'file bus speed model vendor media')
 
 
 def is_udisks2_supported(system_bus):
@@ -89,33 +92,39 @@
         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:
             if self._memorycard:
-                if dev[1] != 'sdio' and not pattern.search(dev[3], re.I):
+                if (dev.bus != 'sdio'
+                    and not FLASH_RE.search(dev.media)
+                    and not CARD_READER_RE.search(dev.model)
+                    and not GENERIC_RE.search(dev.vendor)):
                     logging.debug("The device does not seem to be a memory"
-                                  " card (dev[1]: %r, dev[3]: %r), skipping",
-                                 dev[1], dev[3])
+                                  " 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):
+                if (FLASH_RE.search(dev.media)
+                    or CARD_READER_RE.search(dev.model)
+                    or GENERIC_RE.search(dev.vendor)):
                     logging.debug("The device seems to be a memory"
-                                  " card %s (dev[3]: %r), skipping",
-                                  pattern, dev[3])
+                                  " 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()
@@ -189,11 +198,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(
@@ -202,13 +211,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:
@@ -242,7 +255,7 @@
         # 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