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

This branch "adjusts" BaseDevice.is_real_device for udev data.

The HWDB submission processing script creates instances of class UdevDevice (derived from class BaseDevice) for each udev node found in the submitted data. (run "udevadm info --export-db" to see how this data looks in real like. But note that the output differs in many small details between the udev versions in Jaunty and Karmic.) In many cases udev has more than one node for a given physical device. SCSI devices, for example, have a "main" node, one sub-node for the the "disk aspect" of the device, another sub-node for the generic SCSI driver (allowing you to control the device with "raw" SCSI commands from a user-space program) etc.

We do not want to store these fine details in the HWDB -- we want to store data about a physical device just once. The decision if a given UdevDevice node represents a physical device is made by the property BaseDevice.is_real_device.

The code of this property accessed the property HALDevice.udi which is not available for class UdevDevice; this is replaced by is_root_device.

The core idea of is_real_device is to base the decision on the value of the property raw_bus. This does not work very well for the root device, where raw_bus is None or "Unknown" for submissions with HAL data, and "acpi" for udev submissions. So I moved the case for the root device to the top of the method, to keep the remaining code a bit more clean.

There are some differences in the values of raw_bus between HAL and udev submissions: For udev data, we can have raw_bus set to "partition", "scsi_disk", "scsi_target", "spi_transport". Such nodes desribe "aspects" of SCSI devices, but not real devices, so I added these values to "if bus in (None..."

These values do not appear in submissions with HAL data, so there is no bad side effect of this change.

The raw_bus is "scsi_device" for the udev node describing the "physical" device, while it is "scsi" for submissions with HAL data, so I had to adjust "elif bus == 'scsi':" too. Again, "scsi_device" is not used in submissions with HAL data, so no bad side effect.

Finally, I had to adjust UdevDevice.raw_bus (diff @@ -2564,7 +2590,17 @@). The core idea if the property is to access the HAL property "info.bus" or "info.subsystem" (depending on the HAL version), and to access "something correspondent" for udev devices, where we use the udev properties SUBSYSTEM and DEVTYPE. The name is intended to show a "contrast" to another property real_bus, which may differ for PCI devices (where real_bus may be "PC Card") and for SCSI devices, which may in fact be IDE, ATA or USB devices.

The idea to use DEVTYPE, if this udev property is available, or SUBSYSTEM otherwsie, works mostly fine, except for a certain sub-node of a real SCSI device, where SUBSYSTEM is "scsi_device" and DEVTYPE is None. This leads to conflicts with the main node of a SCSI device, where SUBSYSTEM is "scsi", and DEVTYPE is "scsi_device". The former node should not be treated as a real device, while the latter should be... So I introduced the "if subsystem != 'scsi_device':" clause. This makes the property raw_bus, let's say, "less raw", but I could not come up with a better name...

Then there are a lot of tests for BaseDevice.is_real_device for udev data. The test data is taken from a real-world set up, and it contained data called self.pci_device_data. This describes a SATA controller. I wanted to test is_real_device with different types of SCSI devices, so I added stuff that describes a SATA controller and a disk connected to it. As a part of this, I renamed self.pci_device_data to self.pci_sata_controller and I moved the definition down in the the method TestUdevDevice.setUp() to a location where other SCSI devices and their "environment" are defined.

test: ./bin/test --test=test_hwdb_submission_processing

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/canonical/launchpad/scripts/hwdbsubmissions.py
  lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py

== Pyflakes notices ==

lib/canonical/launchpad/scripts/hwdbsubmissions.py
    22: redefinition of unused 'etree' from line 20

== Pylint notices ==

lib/canonical/launchpad/scripts/hwdbsubmissions.py
    20: [F0401] Unable to import 'xml.etree.cElementTree' (No module named etree)