Merge lp:~cr3/checkbox/removable_resource into lp:checkbox

[Marc Tardif]

Minor improvement suggestions.

[Daniel Manrique (roadmr)]

Looks OK, I tested at least the resource script and it works as advertised. Merging, thanks!

review: Approve

[Sylvain Pineau (sylvain-pineau)]

OK for me as well, a bit late though.

review: Approve

=== modified file 'data/whitelists/default.whitelist'
--- data/whitelists/default.whitelist	2012-06-13 14:13:01 +0000
+++ data/whitelists/default.whitelist	2012-06-13 14:40:31 +0000
@@ -12,7 +12,7 @@
 sleep
 display
 optical_drive
-block_devices
+block_device
 __audio__
 audio/alsa_record_playback_external
 audio/alsa_record_playback_internal

=== modified file 'jobs/disk.txt.in'
--- jobs/disk.txt.in	2012-06-13 12:33:46 +0000
+++ jobs/disk.txt.in	2012-06-13 14:40:31 +0000
@@ -13,7 +13,7 @@
  name: disk/benchmark_`ls /sys$path/block`
  requires: 
   device.path == "$path"
-  block_devices.`ls /sys$path/block` != 'removable'
+  block_device.name == `ls /sys$path/block` and block_device.state != 'removable'
   package.name == 'linux'
  user: root
  command: hdparm -tT /dev/`ls /sys$path/block | sed 's|!|/|'` | sed 's/:.*= */ = /' | grep -v "^$"
@@ -30,7 +30,7 @@
  name: disk/stats_`ls /sys$path/block`
  requires: 
   device.path == "$path"
-  block_devices.`ls /sys$path/block` != 'removable'
+  block_device.name == `ls /sys$path/block` and block_device.state != 'removable'
   package.name == 'linux'
  user: root
  command: disk_stats_test `ls /sys$path/block | sed 's|!|/|'`
@@ -50,7 +50,7 @@
  name: disk/smart_`ls /sys$path/block`
  requires: 
   device.path == "$path"
-  block_devices.`ls /sys$path/block` != 'removable' 
+  block_device.name == `ls /sys$path/block` and block_device.state != 'removable'
  description:
   This tests the SMART capabilities for $product (Note that this test will not work against hardware RAID)
  user: root
@@ -68,7 +68,7 @@
  name: disk/max_diskspace_used_`ls /sys$path/block`
  requires: 
   device.path == "$path"
-  block_devices.`ls /sys$path/block` != 'removable' 
+  block_device.name == `ls /sys$path/block` and block_device.state != 'removable'
  description: Maximum disk space test for $product
  user: root
  command: max_diskspace_used `ls /sys$path/block | sed 's|!|/|'`
@@ -85,7 +85,7 @@
  name: disk/read_performance_`ls /sys$path/block`
  requires:
   device.path == "$path"
-  block_devices.`ls /sys$path/block` != 'removable' 
+  block_device.name == `ls /sys$path/block` and block_device.state != 'removable'
   package.name == 'linux'
  description: Disk performance test for $product
  user: root
@@ -104,7 +104,7 @@
  user: root
  requires:
   device.path == "$path"
-  block_devices.`ls /sys$path/block` != 'removable' 
+  block_device.name == `ls /sys$path/block` and block_device.state != 'removable'
   package.name == 'linux'
  description: Disk I/O stress test for $product
  command: storage_test `ls /sys$path/block | sed 's|!|/|'`

=== modified file 'jobs/resource.txt.in'
--- jobs/resource.txt.in	2012-06-11 07:24:31 +0000
+++ jobs/resource.txt.in	2012-06-13 14:40:31 +0000
@@ -64,9 +64,9 @@
 command: for media in CD DVD; do wodim -prcap 2>/dev/null | grep -q -i "Does write $media" && echo "$media: writable" || echo "$media: readonly"; done
 description: Create resource info for supported optical actions
 
-name: block_devices
+name: block_device
 plugin: resource
-command: removable_resource
+command: block_device_resource
 description: Create resource info for removable block devices 
 
 name: display

=== renamed file 'scripts/removable_resource' => 'scripts/block_device_resource'
--- scripts/removable_resource	2012-06-11 07:24:31 +0000
+++ scripts/block_device_resource	2012-06-13 14:40:31 +0000
@@ -5,37 +5,32 @@
 from glob import glob
 
 
-def device_removable(d):
-    """
-    Follow pmount policy to find removable devices
-    """
+def device_state(name):
+    """
+    Follow pmount policy to determine whether a device is removable or internal.
+    """
+    with open('/sys/block/%s/device/block/%s/removable' % (name, name)) as f:
+        if f.read(1) == '1':
+            return 'removable'
 
-    removable_property = 'internal'
     rootdir_pattern = re.compile('^.*?/devices')
-    with open('/sys/block/%s/device/block/%s/removable' % (d, d)) as f:
-        removable_property = f.read(1)
-
-    if removable_property == '1':
-        return 'removable'
-    else:
-        removable_property = 'internal'
-
-    path = rootdir_pattern.sub('', os.readlink('/sys/block/%s' % d))
+    path = rootdir_pattern.sub('', os.readlink('/sys/block/%s' % name))
     hotplug_buses = ("usb", "ieee1394", "mmc", "pcmcia", "firewire")
     for bus in hotplug_buses:
-        if not os.path.exists('/sys/bus/%s' % bus):
-            continue
-        for device_bus in os.listdir('/sys/bus/%s/devices' % bus):
-            device_link = rootdir_pattern.sub('', os.readlink(
-                '/sys/bus/%s/devices/%s' % (bus, device_bus)))
-            if re.search(device_link, path):
-                return 'removable'
-
-    return removable_property
-
-
-devices = [re.sub('.*/(.*?)/device', '\g<1>', d)
-    for d in glob('/sys/block/*/device')]
-
-for d in devices:
-    print('%s: %s' % (d, device_removable(d)))
+        if os.path.exists('/sys/bus/%s' % bus):
+            for device_bus in os.listdir('/sys/bus/%s/devices' % bus):
+                device_link = rootdir_pattern.sub('', os.readlink(
+                    '/sys/bus/%s/devices/%s' % (bus, device_bus)))
+                if re.search(device_link, path):
+                    return 'removable'
+
+    return 'internal'
+
+
+for path in glob('/sys/block/*/device'):
+    name = re.sub('.*/(.*?)/device', '\g<1>', path)
+    state = device_state(name)
+    print("""\
+name: %s
+state: %s
+""" % (name, state))