Merge lp:~taihsiangho/checkbox/fix-1313581 into lp:checkbox

Proposed by Taihsiang Ho on 2014-04-30
Status: Rejected
Rejected by: Zygmunt Krynicki on 2014-05-02
Proposed branch: lp:~taihsiangho/checkbox/fix-1313581
Merge into: lp:checkbox
Diff against target: 232 lines (+77/-28)
4 files modified
checkbox-support/checkbox_support/udev.py (+17/-0)
providers/plainbox-provider-checkbox/bin/removable_storage_test (+54/-20)
providers/plainbox-provider-checkbox/jobs/suspend.txt.in (+3/-4)
providers/plainbox-provider-checkbox/jobs/usb.txt.in (+3/-4)
To merge this branch: bzr merge lp:~taihsiangho/checkbox/fix-1313581
Reviewer Review Type Date Requested Status
Zygmunt Krynicki (community) 2014-04-30 Needs Fixing on 2014-04-30
Review via email: mp+217783@code.launchpad.net

Commit message

Change the way we test usb 3.0 superspeed (LP: #1313581)

Description of the change

fixing LP: #1313581 Change the way we test usb 3.0 superspeed

in this case, we are going to confirm two things:
1. the usb storage is detected as Superspeed USB device
2. xhci_hcd module is used to control the usb device

The idea is,
for the 1st item,
the value of supported_speed is used to confirm the device is recognized as superspeed device because of the spec. 5GB.

For the 2nd item,
compare the pci slot name of the devices using xhci and
the pci slot name where the usb device is on,
to make sure the usb device does be on the bus using xhci.

checkbox-gui was run and
the testing items related to media cards and usb storages
which using the modified script removable_storage_test
were run to confirm there is no regression.

To post a comment you must log in.
Zygmunt Krynicki (zyga) wrote :

Hey.

A few comments, can you make the code a tiny bit more pythonic?

81 + if (args.driver == 'xhci_hcd'):
82 + if(5000000000 == test.rem_disks_speed[disk] and
83 + 'xhci' == test.rem_disks_xhci[disk]):
84 + print("\t\tDevice Detected: SuperSpeed USB")
85 + print("\t\tDriver Detected: xhci_hcd")

Fix PEP-8 issues, run flake8 on that file and see what shows up. Fix issues affecting the things you've changed if more are reported.

And squash fixes/comments that belong in older patches.

If you need help with any of that just ask. Thanks

review: Needs Fixing
Taihsiang Ho (taihsiangho) wrote :

thanks for the comment, I am working on that.

flake8 scanning as the following:

flake8 checkbox-support/checkbox_support/udev.py
udev.py:95:1: E302 expected 2 blank lines, found 1

flake8 providers/plainbox-provider-checkbox/bin/removable_storage_test
removable_storage_test:25:80: E501 line too long (81 > 79 characters)
removable_storage_test:103:33: E261 at least two spaces before inline comment
removable_storage_test:103:80: E501 line too long (84 > 79 characters)
removable_storage_test:180:21: E125 continuation line does not distinguish itself from next logical line
removable_storage_test:254:80: E501 line too long (82 > 79 characters)
removable_storage_test:262:80: E501 line too long (92 > 79 characters)
removable_storage_test:263:80: E501 line too long (89 > 79 characters)
removable_storage_test:296:29: E125 continuation line does not distinguish itself from next logical line
removable_storage_test:301:29: E125 continuation line does not distinguish itself from next logical line
removable_storage_test:415:78: W291 trailing whitespace
removable_storage_test:467:28: E127 continuation line over-indented for visual indent
removable_storage_test:475:24: E127 continuation line over-indented for visual indent
removable_storage_test:480:29: E127 continuation line over-indented for visual indent
removable_storage_test:481:29: E127 continuation line over-indented for visual indent
removable_storage_test:481:33: E901 SyntaxError: invalid syntax
removable_storage_test:483:25: E125 continuation line does not distinguish itself from next logical line
removable_storage_test:485:33: E127 continuation line over-indented for visual indent
removable_storage_test:501:41: E126 continuation line over-indented for hanging indent
removable_storage_test:570:47: E128 continuation line under-indented for visual indent
removable_storage_test:599:29: E125 continuation line does not distinguish itself from next logical line
removable_storage_test:606:25: E125 continuation line does not distinguish itself from next logical line

lp:~taihsiangho/checkbox/fix-1313581 updated on 2014-05-02
2962. By Taihsiang Ho on 2014-05-02

fix pep8 coding style scanned by flake8

2963. By Taihsiang Ho on 2014-05-02

pep8 coding style, line too long (81 > 79 characters)

2964. By Taihsiang Ho on 2014-05-02

pep8 coding style, at least two spaces before inline comment, line too long

2965. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line does not distinguish itself from next logical line

2966. By Taihsiang Ho on 2014-05-02

pep8 coding style, line too long

2967. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line does not distinguish itself from next logical line

2968. By Taihsiang Ho on 2014-05-02

trailing whitespace

2969. By Taihsiang Ho on 2014-05-02

line over-indented for visual indent

2970. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line does not distinguish itself from next logical line

2971. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line over-indented for visual indent

2972. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line over-indented for hanging indent

2973. By Taihsiang Ho on 2014-05-02

pep8 coding style, E128 continuation line under-indented for visual indent

2974. By Taihsiang Ho on 2014-05-02

pep8 coding style, E125 continuation line does not distinguish itself from next logical line

2975. By Taihsiang Ho on 2014-05-02

pep8 coding style, E125 continuation line does not distinguish itself from next logical line

2976. By Taihsiang Ho on 2014-05-02

F841 local variable 'avg_write_time' is assigned to but never used

2977. By Taihsiang Ho on 2014-05-02

E265 block comment should start with '# '

Taihsiang Ho (taihsiangho) wrote :

comment 2 used flake8 under python 2 (this is incorrect)

flake8 for python3 shows there is no syntax error and two more highlights:

removable_storage_test:554:29: F841 local variable 'avg_write_time' is assigned to but never used
removable_storage_test:610:21: E265 block comment should start with '# '

the rest of scanning result is the same as those in comment 2

----------------------------------
scanned result by flake8 (python3)
----------------------------------

udev.py:95:1: E302 expected 2 blank lines, found 1

removable_storage_test:25:80: E501 line too long (81 > 79 characters)
removable_storage_test:103:33: E261 at least two spaces before inline comment
removable_storage_test:103:80: E501 line too long (84 > 79 characters)
removable_storage_test:180:21: E129 visually indented line with same indent as next logical line
removable_storage_test:254:80: E501 line too long (82 > 79 characters)
removable_storage_test:262:80: E501 line too long (92 > 79 characters)
removable_storage_test:263:80: E501 line too long (89 > 79 characters)
removable_storage_test:296:29: E129 visually indented line with same indent as next logical line
removable_storage_test:301:29: E129 visually indented line with same indent as next logical line
removable_storage_test:415:78: W291 trailing whitespace
removable_storage_test:467:28: E127 continuation line over-indented for visual indent
removable_storage_test:475:24: E127 continuation line over-indented for visual indent
removable_storage_test:480:29: E127 continuation line over-indented for visual indent
removable_storage_test:481:29: E127 continuation line over-indented for visual indent
removable_storage_test:483:25: E129 visually indented line with same indent as next logical line
removable_storage_test:485:33: E127 continuation line over-indented for visual indent
removable_storage_test:501:41: E126 continuation line over-indented for hanging indent
removable_storage_test:548:29: F841 local variable 'avg_write_time' is assigned to but never used
removable_storage_test:570:47: E128 continuation line under-indented for visual indent
removable_storage_test:599:29: E125 continuation line with same indent as next logical line
removable_storage_test:604:21: E265 block comment should start with '# '
removable_storage_test:606:25: E129 visually indented line with same indent as next logical line

Taihsiang Ho (taihsiangho) wrote :

test media card and usb storage items on 201304-13404 with 14.04.
Looks good.

Taihsiang Ho (taihsiangho) wrote :

revno 2961 is code for the usb3 testing logic.

modification after revno 2961 just for the pep8 coding style.

Unmerged revisions

2977. By Taihsiang Ho on 2014-05-02

E265 block comment should start with '# '

2976. By Taihsiang Ho on 2014-05-02

F841 local variable 'avg_write_time' is assigned to but never used

2975. By Taihsiang Ho on 2014-05-02

pep8 coding style, E125 continuation line does not distinguish itself from next logical line

2974. By Taihsiang Ho on 2014-05-02

pep8 coding style, E125 continuation line does not distinguish itself from next logical line

2973. By Taihsiang Ho on 2014-05-02

pep8 coding style, E128 continuation line under-indented for visual indent

2972. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line over-indented for hanging indent

2971. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line over-indented for visual indent

2970. By Taihsiang Ho on 2014-05-02

pep8 coding style, continuation line does not distinguish itself from next logical line

2969. By Taihsiang Ho on 2014-05-02

line over-indented for visual indent

2968. By Taihsiang Ho on 2014-05-02

trailing whitespace

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'checkbox-support/checkbox_support/udev.py'
2--- checkbox-support/checkbox_support/udev.py 2014-01-07 13:39:44 +0000
3+++ checkbox-support/checkbox_support/udev.py 2014-05-02 07:49:00 +0000
4@@ -91,3 +91,20 @@
5 # look better.
6 devices.sort(key=lambda device: device.get_device_file())
7 return devices
8+
9+
10+def get_udev_xhci_devices(udev_client):
11+ """
12+ Get a list of all devices on pci slots using xhci drivers
13+ """
14+ # setup an enumerator so that we can list devices
15+ enumerator = GUdev.Enumerator(client=udev_client)
16+ # Iterate over pci devices only
17+ enumerator.add_match_subsystem('pci')
18+ devices = [
19+ device for device in enumerator.execute()
20+ if (device.get_driver() == 'xhci_hcd')]
21+ # Sort the list, this is not needed but makes various debugging dumps
22+ # look better.
23+ devices.sort(key=lambda device: device.get_property('PCI_SLOT_NAME'))
24+ return devices
25
26=== modified file 'providers/plainbox-provider-checkbox/bin/removable_storage_test'
27--- providers/plainbox-provider-checkbox/bin/removable_storage_test 2014-01-07 13:43:22 +0000
28+++ providers/plainbox-provider-checkbox/bin/removable_storage_test 2014-05-02 07:49:00 +0000
29@@ -22,9 +22,12 @@
30 from checkbox_support.dbus.udisks2 import lookup_udev_device
31 from checkbox_support.dbus.udisks2 import map_udisks1_connection_bus
32 from checkbox_support.heuristics.udisks2 import is_memory_card
33-from checkbox_support.parsers.udevadm import CARD_READER_RE, GENERIC_RE, FLASH_RE
34+from checkbox_support.parsers.udevadm import CARD_READER_RE
35+from checkbox_support.parsers.udevadm import GENERIC_RE
36+from checkbox_support.parsers.udevadm import FLASH_RE
37 from checkbox_support.udev import get_interconnect_speed
38 from checkbox_support.udev import get_udev_block_devices
39+from checkbox_support.udev import get_udev_xhci_devices
40
41
42 class ActionTimer():
43@@ -99,6 +102,8 @@
44 self.rem_disks_memory_cards = {}
45 self.rem_disks_memory_cards_nm = {}
46 self.rem_disks_speed = {}
47+ # LP: #1313581, TODO: extend to be rem_disks_driver
48+ self.rem_disks_xhci = {}
49 self.data = ''
50 self.device = device
51 self.memorycard = memorycard
52@@ -174,8 +179,8 @@
53 have both the filesystem and block device interfaces
54 """
55 for udisks2_object_path, interfaces in udisks2_objects.items():
56- if (UDISKS2_FILESYSTEM_INTERFACE in interfaces
57- and UDISKS2_BLOCK_INTERFACE in interfaces):
58+ if (UDISKS2_FILESYSTEM_INTERFACE in interfaces and
59+ UDISKS2_BLOCK_INTERFACE in interfaces):
60 yield udisks2_object_path
61 # We need to know about all IO candidates,
62 # let's iterate over all the block devices reported by udisks2
63@@ -247,6 +252,23 @@
64 else:
65 self.rem_disks_memory_cards_nm[dev_file] = None
66 self.rem_disks_nm[dev_file] = None
67+ # LP: #1313581
68+ # Compare the pci slot name of the devices using xhci and
69+ # the pci slot name of the disks,
70+ # which is usb3 disks in this case so far,
71+ # to make sure the usb3 disk does be on the bus using xhci
72+ # TODO: it will be better to extend to be all kinds of drivers.
73+ try:
74+ udev_devices_xhci = get_udev_xhci_devices(udev_client)
75+ for udev_device_xhci in udev_devices_xhci:
76+ pci_slot_name = udev_device_xhci.get_property('PCI_SLOT_NAME')
77+ for udev_device in udev_devices:
78+ if (pci_slot_name ==
79+ udev_device.get_property('DEVPATH').split('/')[3]):
80+ self.rem_disks_xhci[
81+ udev_device.get_property('DEVNAME')] = 'xhci'
82+ except:
83+ logging.error("Failed to get driver information.")
84
85 def _probe_disks_udisks1(self, bus):
86 """
87@@ -275,14 +297,14 @@
88 parent_media = parent_props.Get(udisks, "DriveMedia")
89 if self.memorycard:
90 if (dev_bus != 'sdio'
91- and not FLASH_RE.search(parent_media)
92- and not CARD_READER_RE.search(parent_model)
93- and not GENERIC_RE.search(parent_vendor)):
94+ and not FLASH_RE.search(parent_media)
95+ and not CARD_READER_RE.search(parent_model)
96+ and not GENERIC_RE.search(parent_vendor)):
97 continue
98 else:
99 if (FLASH_RE.search(parent_media)
100- or CARD_READER_RE.search(parent_model)
101- or GENERIC_RE.search(parent_vendor)):
102+ or CARD_READER_RE.search(parent_model)
103+ or GENERIC_RE.search(parent_vendor)):
104 continue
105 dev_file = str(device_props.Get(udisks, "DeviceFile"))
106 dev_speed = str(device_props.Get(udisks,
107@@ -393,6 +415,10 @@
108 help=("Memory cards devices on bus other than sdio "
109 "require this parameter to identify "
110 "them as such"))
111+ parser.add_argument('--driver',
112+ choices=['xhci_hcd'],
113+ help=("Detect the driver of the host controller."
114+ "Only xhci_hcd for usb3 is supported so far."))
115
116 args = parser.parse_args()
117
118@@ -444,7 +470,7 @@
119
120 if errors_mount > 0:
121 print("There're total %d device(s) failed at mounting."
122- % errors_mount)
123+ % errors_mount)
124 errors += errors_mount
125
126 disks_all = dict(list(test.rem_disks.items())
127@@ -452,17 +478,17 @@
128
129 if len(disks_all) > 0:
130 print("Found the following mounted %s partitions:"
131- % ', '.join(args.device))
132+ % ', '.join(args.device))
133
134 for disk, mount_point in disks_all.items():
135 supported_speed = test.rem_disks_speed[disk]
136 print(" %s : %s : %s bits/s" %
137- (disk, mount_point, supported_speed),
138- end="")
139- if (args.min_speed
140- and int(args.min_speed) > int(supported_speed)):
141+ (disk, mount_point, supported_speed),
142+ end="")
143+ if (args.min_speed and
144+ int(args.min_speed) > int(supported_speed)):
145 print(" (Will not test it, speed is below %s bits/s)" %
146- args.min_speed, end="")
147+ args.min_speed, end="")
148
149 print("")
150
151@@ -478,7 +504,7 @@
152 for count in range(args.count):
153 test_files[count] = RandomData(args.size)
154 write_sizes.append(os.path.getsize(
155- test_files[count].tfile.name))
156+ test_files[count].tfile.name))
157 total_write_size = sum(write_sizes)
158
159 try:
160@@ -525,7 +551,7 @@
161 for file in target_file_list:
162 test.clean_up(file)
163 total_write_time = sum(write_times)
164- avg_write_time = total_write_time / args.count
165+ # avg_write_time = total_write_time / args.count
166 try:
167 avg_write_speed = ((
168 total_write_size / total_write_time)
169@@ -547,7 +573,7 @@
170 (iteration_write_time / args.iterations))
171 try:
172 avg_write_speed = (iteration_write_size /
173- iteration_write_time)
174+ iteration_write_time)
175 except ZeroDivisionError:
176 avg_write_speed = 0.00
177 finally:
178@@ -573,9 +599,17 @@
179 return 1
180
181 else:
182- #Pass is not assured!
183+ # LP: 1313581
184+ if (args.driver == 'xhci_hcd'):
185+ if(5000000000 == test.rem_disks_speed[disk] and
186+ 'xhci' == test.rem_disks_xhci[disk]):
187+ print("\t\tDevice Detected: SuperSpeed USB")
188+ print("\t\tDriver Detected: xhci_hcd")
189+ else:
190+ return 1
191+ # Pass is not assured
192 if (not args.pass_speed or
193- avg_write_speed >= args.pass_speed):
194+ avg_write_speed >= args.pass_speed):
195 return 0
196 else:
197 print("FAIL: Average speed was lower than desired "
198
199=== modified file 'providers/plainbox-provider-checkbox/jobs/suspend.txt.in'
200--- providers/plainbox-provider-checkbox/jobs/suspend.txt.in 2014-04-29 21:10:14 +0000
201+++ providers/plainbox-provider-checkbox/jobs/suspend.txt.in 2014-05-02 07:49:00 +0000
202@@ -1667,11 +1667,10 @@
203 depends: suspend/usb3_insert_after_suspend
204 user: root
205 estimated_duration: 45.00
206-command: removable_storage_test -s 268400000 -m 500000000 -p 60 usb
207+command: removable_storage_test -s 268400000 -m 500000000 usb --driver xhci_hcd
208 _description:
209- This test will check that your USB 3.0 port transfers data at a
210- minimum expected speed in accordance with the specification of
211- USB 3.0 SuperSpeed mode.
212+ This test will check that your USB 3.0 port could be recognized
213+ as SuperSpeed USB device using xhci_hcd driver and transfers data correctly.
214
215 plugin: user-interact
216 id: suspend/mmc-insert-after-suspend
217
218=== modified file 'providers/plainbox-provider-checkbox/jobs/usb.txt.in'
219--- providers/plainbox-provider-checkbox/jobs/usb.txt.in 2014-04-10 21:51:48 +0000
220+++ providers/plainbox-provider-checkbox/jobs/usb.txt.in 2014-05-02 07:49:00 +0000
221@@ -217,8 +217,7 @@
222 depends: usb3/insert
223 user: root
224 estimated_duration: 45.00
225-command: removable_storage_test -s 268400000 -m 500000000 -p 60 usb
226+command: removable_storage_test -s 268400000 -m 500000000 usb --driver xhci_hcd
227 _description:
228- This test will check that your USB 3.0 port transfers data at a
229- minimum expected speed in accordance with the specification of
230- USB 3.0 SuperSpeed mode.
231+ This test will check that your USB 3.0 port could be recognized
232+ as SuperSpeed USB device using xhci_hcd driver and transfers data correctly.

Subscribers

People subscribed via source and target branches