Launchpad itself

Code review comment for lp:~adeuring/launchpad/hwdb-class-udev-device-5

  1. hwdb-class-udev-device-5
  2. Merge into db-devel
Revision history for this message
Gavin Panella (allenap) wrote :

This all looks good.

As discussed on IRC, this is really meant for devel, not db-devel, so
I've attached the diff I actually reviewed.

Gavin.

review.diff
1=== modified file 'lib/canonical/launchpad/scripts/hwdbsubmissions.py'
2--- lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-13 21:03:31 +0000
3+++ lib/canonical/launchpad/scripts/hwdbsubmissions.py 2009-10-14 16:00:26 +0000
4@@ -87,12 +87,14 @@
5 'pci': '0x%04x',
6 'usb_device': '0x%04x',
7 'scsi': '%-8s',
8+ 'scsi_device': '%-8s',
9 }
10
11 DB_FORMAT_FOR_PRODUCT_ID = {
12 'pci': '0x%04x',
13 'usb_device': '0x%04x',
14 'scsi': '%-16s',
15+ 'scsi_device': '%-16s',
16 }
17
18 UDEV_USB_DEVICE_PROPERTIES = set(('DEVTYPE', 'PRODUCT', 'TYPE'))
19@@ -1609,6 +1611,11 @@
20 """The name of the driver contolling this device. May be None."""
21 raise NotImplementedError
22
23+ @property
24+ def scsi_controller(self):
25+ """Return the SCSI host controller for this device."""
26+ raise NotImplementedError
27+
28 def translateScsiBus(self):
29 """Return the real bus of a device where raw_bus=='scsi'.
30
31@@ -1617,37 +1624,27 @@
32 for more details. This method determines the real bus
33 of a device accessed via the kernel's SCSI subsystem.
34 """
35- # While SCSI devices from valid submissions should have a
36- # parent and a grandparent, we can't be sure for bogus or
37- # broken submissions.
38- parent = self.parent
39- if parent is None:
40- self.parser._logWarning(
41- 'Found SCSI device without a parent: %s.' % self.device_id)
42- return None
43- grandparent = parent.parent
44- if grandparent is None:
45- self.parser._logWarning(
46- 'Found SCSI device without a grandparent: %s.'
47- % self.device_id)
48+ scsi_controller = self.scsi_controller
49+ if scsi_controller is None:
50 return None
51
52- grandparent_bus = grandparent.raw_bus
53- if grandparent_bus == 'pci':
54- if (grandparent.pci_class != PCI_CLASS_STORAGE):
55+ scsi_controller_bus = scsi_controller.raw_bus
56+ if scsi_controller_bus == 'pci':
57+ if (scsi_controller.pci_class != PCI_CLASS_STORAGE):
58 # This is not a storage class PCI device? This
59 # indicates a bug somewhere in HAL or in the hwdb
60 # client, or a fake submission.
61- device_class = grandparent.pci_class
62+ device_class = scsi_controller.pci_class
63 self.parser._logWarning(
64 'A (possibly fake) SCSI device %s is connected to '
65 'PCI device %s that has the PCI device class %s; '
66 'expected class 1 (storage).'
67- % (self.device_id, grandparent.device_id, device_class))
68+ % (self.device_id, scsi_controller.device_id,
69+ device_class))
70 return None
71- pci_subclass = grandparent.pci_subclass
72+ pci_subclass = scsi_controller.pci_subclass
73 return self.pci_storage_subclass_hwbus.get(pci_subclass)
74- elif grandparent_bus == 'usb':
75+ elif scsi_controller_bus == 'usb':
76 # USB storage devices have the following HAL device hierarchy:
77 # - HAL node for the USB device. info.bus == 'usb_device',
78 # device class == 0, device subclass == 0
79@@ -2357,6 +2354,27 @@
80 """See `BaseDevice`."""
81 return self.getVendorOrProductID('product')
82
83+ @property
84+ def scsi_controller(self):
85+ """See `BaseDevice`."""
86+ # While SCSI devices from valid submissions should have a
87+ # parent and a grandparent, we can't be sure for bogus or
88+ # broken submissions.
89+ if self.raw_bus != 'scsi':
90+ return None
91+ parent = self.parent
92+ if parent is None:
93+ self.parser._logWarning(
94+ 'Found SCSI device without a parent: %s.' % self.device_id)
95+ return None
96+ grandparent = parent.parent
97+ if grandparent is None:
98+ self.parser._logWarning(
99+ 'Found SCSI device without a grandparent: %s.'
100+ % self.device_id)
101+ return None
102+ return grandparent
103+
104
105 class UdevDevice(BaseDevice):
106 """The representation of a udev device node."""
107@@ -2617,6 +2635,11 @@
108 """See `BaseDevice`."""
109 return self.getVendorOrProductID('product')
110
111+ @property
112+ def driver_name(self):
113+ """See `BaseDevice`."""
114+ return self.udev['E'].get('DRIVER')
115+
116
117 class ProcessingLoop(object):
118 """An `ITunableLoop` for processing HWDB submissions."""
119
120=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py'
121--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-13 21:02:35 +0000
122+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2009-10-14 16:00:26 +0000
123@@ -539,6 +539,214 @@
124 'HAL property info.bus.')
125
126
127+ def test_HALDevice_scsi_controller_usb_storage_device(self):
128+ """test of HALDevice.scsi_controller.
129+
130+ The physical device is a USB storage device.
131+ """
132+ devices = [
133+ # The main node of the USB storage device.
134+ {
135+ 'id': 1,
136+ 'udi': self.UDI_USB_STORAGE,
137+ 'properties': {
138+ 'info.bus': ('usb_device', 'str'),
139+ },
140+ },
141+ # The storage interface of the USB device.
142+ {
143+ 'id': 2,
144+ 'udi': self.UDI_USB_STORAGE_IF0,
145+ 'properties': {
146+ 'info.bus': ('usb', 'str'),
147+ 'info.parent': (self.UDI_USB_STORAGE, 'str'),
148+ },
149+ },
150+ # The fake SCSI host of the storage device. Note that HAL does
151+ # _not_ provide the info.bus property.
152+ {
153+ 'id': 3,
154+ 'udi': self.UDI_USB_STORAGE_SCSI_HOST,
155+ 'properties': {
156+ 'info.parent': (self.UDI_USB_STORAGE_IF0, 'str'),
157+ },
158+ },
159+ # The fake SCSI disk.
160+ {
161+ 'id': 3,
162+ 'udi': self.UDI_USB_STORAGE_SCSI_DEVICE,
163+ 'properties': {
164+ 'info.bus': ('scsi', 'str'),
165+ 'info.parent': (self.UDI_USB_STORAGE_SCSI_HOST, 'str'),
166+ },
167+ },
168+ ]
169+ parsed_data = {
170+ 'hardware': {
171+ 'hal': {
172+ 'devices': devices,
173+ },
174+ },
175+ }
176+
177+ parser = SubmissionParser()
178+ parser.buildDeviceList(parsed_data)
179+
180+ usb_fake_scsi_disk = parser.hal_devices[
181+ self.UDI_USB_STORAGE_SCSI_DEVICE]
182+ usb_main_device = parser.hal_devices[self.UDI_USB_STORAGE_IF0]
183+ self.assertEqual(usb_main_device, usb_fake_scsi_disk.scsi_controller)
184+
185+ def test_HALDevice_scsi_controller_pci_controller(self):
186+ """test of HALDevice.scsi_controller.
187+
188+ Variant for a SCSI device connected to a PCI controller.
189+ """
190+ devices = [
191+ # The PCI host controller.
192+ {
193+ 'id': 1,
194+ 'udi': self.UDI_SATA_CONTROLLER,
195+ 'properties': {
196+ 'info.bus': ('pci', 'str'),
197+ 'pci.device_class': (PCI_CLASS_STORAGE, 'int'),
198+ 'pci.device_subclass': (PCI_SUBCLASS_STORAGE_SATA,
199+ 'int'),
200+ },
201+ },
202+ # The (fake or real) SCSI host of the storage device.
203+ {
204+ 'id': 2,
205+ 'udi': self.UDI_SATA_CONTROLLER_SCSI,
206+ 'properties': {
207+ 'info.parent': (self.UDI_SATA_CONTROLLER, 'str'),
208+ },
209+ },
210+ # The (possibly fake) SCSI disk.
211+ {
212+ 'id': 3,
213+ 'udi': self.UDI_SATA_DISK,
214+ 'properties': {
215+ 'info.bus': ('scsi', 'str'),
216+ 'info.parent': (self.UDI_SATA_CONTROLLER_SCSI, 'str'),
217+ },
218+ },
219+ ]
220+ parsed_data = {
221+ 'hardware': {
222+ 'hal': {
223+ 'devices': devices,
224+ },
225+ },
226+ }
227+
228+ parser = SubmissionParser()
229+ parser.buildDeviceList(parsed_data)
230+
231+ scsi_device = parser.hal_devices[self.UDI_SATA_DISK]
232+ controller = parser.hal_devices[self.UDI_SATA_CONTROLLER]
233+ self.assertEqual(controller, scsi_device.scsi_controller)
234+
235+ def test_HALDevice_scsi_controller_non_scsi_device(self):
236+ """test of HALDevice.scsi_controller.
237+
238+ Variant for non-SCSI devices.
239+ """
240+ devices = [
241+ {
242+ 'id': 1,
243+ 'udi': self.UDI_COMPUTER,
244+ 'properties': {},
245+ },
246+ ]
247+ parsed_data = {
248+ 'hardware': {
249+ 'hal': {
250+ 'devices': devices,
251+ },
252+ },
253+ }
254+
255+ parser = SubmissionParser()
256+ parser.buildDeviceList(parsed_data)
257+
258+ device = parser.hal_devices[self.UDI_COMPUTER]
259+ self.assertEqual(None, device.scsi_controller)
260+
261+ def test_HALDevice_scsi_controller_no_grandparent(self):
262+ """test of HALDevice.scsi_controller.
263+
264+ Variant for a SCSI device without a grandparent device.
265+ """
266+ devices = [
267+ # The (fake or real) SCSI host of the storage device.
268+ {
269+ 'id': 1,
270+ 'udi': self.UDI_SATA_CONTROLLER_SCSI,
271+ 'properties': {},
272+ },
273+ # The (possibly fake) SCSI disk.
274+ {
275+ 'id': 2,
276+ 'udi': self.UDI_SATA_DISK,
277+ 'properties': {
278+ 'info.bus': ('scsi', 'str'),
279+ 'info.parent': (self.UDI_SATA_CONTROLLER_SCSI, 'str'),
280+ },
281+ },
282+ ]
283+ parsed_data = {
284+ 'hardware': {
285+ 'hal': {
286+ 'devices': devices,
287+ },
288+ },
289+ }
290+
291+ parser = SubmissionParser(self.log)
292+ parser.submission_key = 'SCSI device without grandparent device'
293+ parser.buildDeviceList(parsed_data)
294+
295+ scsi_device = parser.hal_devices[self.UDI_SATA_DISK]
296+ self.assertEqual(None, scsi_device.scsi_controller)
297+ self.assertWarningMessage(
298+ parser.submission_key,
299+ "Found SCSI device without a grandparent: %s."
300+ % self.UDI_SATA_DISK)
301+
302+ def test_HALDevice_scsi_controller_no_parent(self):
303+ """test of HALDevice.scsi_controller.
304+
305+ Variant for a SCSI device without a parent device.
306+ """
307+ devices = [
308+ # The (possibly fake) SCSI disk.
309+ {
310+ 'id': 1,
311+ 'udi': self.UDI_SATA_DISK,
312+ 'properties': {
313+ 'info.bus': ('scsi', 'str'),
314+ },
315+ },
316+ ]
317+ parsed_data = {
318+ 'hardware': {
319+ 'hal': {
320+ 'devices': devices,
321+ },
322+ },
323+ }
324+
325+ parser = SubmissionParser(self.log)
326+ parser.submission_key = 'SCSI device without parent device'
327+ parser.buildDeviceList(parsed_data)
328+
329+ scsi_device = parser.hal_devices[self.UDI_SATA_DISK]
330+ self.assertEqual(None, scsi_device.scsi_controller)
331+ self.assertWarningMessage(
332+ parser.submission_key,
333+ "Found SCSI device without a parent: %s." % self.UDI_SATA_DISK)
334+
335 def testHALDeviceGetRealBus(self):
336 """Test of HALDevice.real_bus, generic case.
337
338@@ -2613,6 +2821,7 @@
339 'PCI_SUBSYS_ID': '10CF:1387',
340 'PCI_SLOT_NAME': '0000:00:1f.2',
341 'SUBSYSTEM': 'pci',
342+ 'DRIVER': 'ahci',
343 }
344 }
345
346@@ -2623,21 +2832,23 @@
347 'DEVTYPE': 'usb_device',
348 'PRODUCT': '46d/a01/1013',
349 'TYPE': '0/0/0',
350+ 'DRIVER': 'usb',
351 },
352 }
353
354- scsi_device_data = {
355- 'P': '/devices/pci0000:00/0000:00:1f.1/host4/target4:0:0/4:0:0:0',
356+ scsi_scanner_device_data = {
357+ 'P': ('/devices/pci0000:00/0000:00:1e.0/0000:08:03.0/0000:09:00.0/'
358+ 'host6/target6:0:1/6:0:1:0'),
359 'E': {
360+ 'DEVTYPE': 'scsi_device',
361 'SUBSYSTEM': 'scsi',
362- 'DEVTYPE': 'scsi_device',
363 },
364 }
365
366- scsi_device_sysfs_data = {
367- 'vendor': 'MATSHITA',
368- 'model': 'DVD-RAM UJ-841S',
369- 'type': '5',
370+ scsi_scanner_device_sysfs_data = {
371+ 'vendor': 'FUJITSU',
372+ 'model': 'fi-5120Cdj',
373+ 'type': '6',
374 }
375
376 no_subsystem_device_data = {
377@@ -2790,7 +3001,8 @@
378 def test_is_scsi_device(self):
379 """Test of UdevDevice.is_scsi_device."""
380 device = UdevDevice(
381- None, self.scsi_device_data, self.scsi_device_sysfs_data)
382+ None, self.scsi_scanner_device_data,
383+ self.scsi_scanner_device_sysfs_data)
384 self.assertTrue(device.is_scsi_device)
385
386 device = UdevDevice(None, self.root_device)
387@@ -2799,16 +3011,18 @@
388 def test_scsi_vendor(self):
389 """Test of UdevDevice.scsi_vendor."""
390 device = UdevDevice(
391- None, self.scsi_device_data, self.scsi_device_sysfs_data, None)
392- self.assertEqual('MATSHITA', device.scsi_vendor)
393+ None, self.scsi_scanner_device_data,
394+ self.scsi_scanner_device_sysfs_data)
395+ self.assertEqual('FUJITSU', device.scsi_vendor)
396 device = UdevDevice(None, self.root_device)
397 self.assertEqual(None, device.scsi_vendor)
398
399 def test_scsi_model(self):
400 """Test of UdevDevice.scsi_model."""
401 device = UdevDevice(
402- None, self.scsi_device_data, self.scsi_device_sysfs_data)
403- self.assertEqual('DVD-RAM UJ-841S', device.scsi_model)
404+ None, self.scsi_scanner_device_data,
405+ self.scsi_scanner_device_sysfs_data)
406+ self.assertEqual('fi-5120Cdj', device.scsi_model)
407
408 device = UdevDevice(None, self.root_device)
409 self.assertEqual(None, device.scsi_model)
410@@ -2847,10 +3061,10 @@
411 self.assertEqual('Unknown', device.getVendorOrProduct('product'))
412
413 device = UdevDevice(
414- None, self.scsi_device_data, self.scsi_device_sysfs_data)
415- self.assertEqual('MATSHITA', device.getVendorOrProduct('vendor'))
416- self.assertEqual(
417- 'DVD-RAM UJ-841S', device.getVendorOrProduct('product'))
418+ None, self.scsi_scanner_device_data,
419+ self.scsi_scanner_device_sysfs_data)
420+ self.assertEqual('FUJITSU', device.getVendorOrProduct('vendor'))
421+ self.assertEqual('fi-5120Cdj', device.getVendorOrProduct('product'))
422
423 device = UdevDevice(None, self.no_subsystem_device_data)
424 self.assertEqual(None, device.getVendorOrProduct('vendor'))
425@@ -2888,10 +3102,10 @@
426 self.assertEqual(0xa01, device.getVendorOrProductID('product'))
427
428 device = UdevDevice(
429- None, self.scsi_device_data, self.scsi_device_sysfs_data)
430- self.assertEqual('MATSHITA', device.getVendorOrProductID('vendor'))
431- self.assertEqual(
432- 'DVD-RAM UJ-841S', device.getVendorOrProductID('product'))
433+ None, self.scsi_scanner_device_data,
434+ self.scsi_scanner_device_sysfs_data)
435+ self.assertEqual('FUJITSU', device.getVendorOrProductID('vendor'))
436+ self.assertEqual('fi-5120Cdj', device.getVendorOrProductID('product'))
437
438 device = UdevDevice(
439 None, self.no_subsystem_device_data)
440@@ -2910,6 +3124,49 @@
441 None, self.root_device, None, self.root_device_dmi_data)
442 self.assertEqual('LIFEBOOK E8210', device.product_id)
443
444+ def test_vendor_id_for_db(self):
445+ """Test of UdevDevice.vendor_id_for_db."""
446+ device = UdevDevice(
447+ None, self.root_device, None, self.root_device_dmi_data)
448+ self.assertEqual('FUJITSU SIEMENS', device.vendor_id_for_db)
449+
450+ device = UdevDevice(None, self.pci_device_data)
451+ self.assertEqual('0x8086', device.vendor_id_for_db)
452+
453+ device = UdevDevice(None, self.usb_device_data)
454+ self.assertEqual('0x046d', device.vendor_id_for_db)
455+
456+ device = UdevDevice(
457+ None, self.scsi_scanner_device_data,
458+ self.scsi_scanner_device_sysfs_data)
459+ self.assertEqual('FUJITSU ', device.vendor_id_for_db)
460+
461+ def test_product_id_for_db(self):
462+ """Test of UdevDevice.product_id_for_db."""
463+ device = UdevDevice(
464+ None, self.root_device, None, self.root_device_dmi_data)
465+ self.assertEqual('LIFEBOOK E8210', device.product_id_for_db)
466+
467+ device = UdevDevice(None, self.pci_device_data)
468+ self.assertEqual('0x27c5', device.product_id_for_db)
469+
470+ device = UdevDevice(None, self.usb_device_data)
471+ self.assertEqual('0x0a01', device.product_id_for_db)
472+
473+ device = UdevDevice(
474+ None, self.scsi_scanner_device_data,
475+ self.scsi_scanner_device_sysfs_data)
476+ self.assertEqual('fi-5120Cdj ', device.product_id_for_db)
477+
478+ def test_driver_name(self):
479+ """Test of UdevDevice.driver_name."""
480+ device = UdevDevice(None, self.pci_device_data)
481+ self.assertEqual('ahci', device.driver_name)
482+
483+ device = UdevDevice(
484+ None, self.root_device, None, self.root_device_dmi_data)
485+ self.assertEqual(None, device.driver_name)
486+
487
488 class TestHWDBSubmissionTablePopulation(TestCaseHWDB):
489 """Tests of the HWDB popoluation with submitted data."""

« Back to merge proposal