launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04809
[Merge] lp:~adeuring/launchpad/test-hwsubm into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/test-hwsubm into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/test-hwsubm/+merge/73294
This branch allows to parse HWDB reports that come from the Maverick and Natty versions of checkbox. These versions do not provide information we need to store data about SCSI devices in the hardwrae database: the reports have no <sysfs-qttributes> node. This node is supposed to be the source for the vendor annd model name of SCSI devcies.
We could still get the vendor/model name for block devices, by looking into sub-nodes of the "main" udev node for these devcies, but:
- we urgently need a fix
- Data about SCSI block devcies is probably the least interesting data; more intersting are other devices, like tape robots or scanners: support tends to be more fragile that support for disks or CD drives.
This branch changes the RelaxNG schema so that raw submissions without a <sysfs-attributes> node pass; it ensures that the further porcessing of the data works.
The main change is in is_scsi_device (@@ -2731,6 +2747,12 @@): This property is used by the proeprty scsi_vendor and scsi_model to check if the vendor/model name exist; if the latter properties are None, the "potential scsi device" is not considered to be a SCSI devcie, an no data about is stored.
("Potential SCSI devices": Many non-SCSI devices, like ATA disks, USB disks or USB card readers, use the SCSI command set to communicate with the host computer. These devices are treated like real SCSI devcies, but they don't have the sysfs attributes vendor/model; this is already used by the HWDB submission processing script to decide, if data about a given "potentail SCSI device" should be stored.
--
https://code.launchpad.net/~adeuring/launchpad/test-hwsubm/+merge/73294
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/test-hwsubm into lp:launchpad.
=== modified file 'lib/canonical/launchpad/scripts/hardware-1_0.rng'
--- lib/canonical/launchpad/scripts/hardware-1_0.rng 2009-09-14 09:16:45 +0000
+++ lib/canonical/launchpad/scripts/hardware-1_0.rng 2011-08-29 20:28:27 +0000
@@ -148,11 +148,17 @@
<element name="dmi">
<text/>
</element>
- <element name="sysfs-attributes">
- <zeroOrMore>
- <text/>
- </zeroOrMore>
- </element>
+ <optional>
+ <!-- Unfortunately, the checkbox version in
+ Maverick and Natty do not provide sysfs
+ data.
+ -->
+ <element name="sysfs-attributes">
+ <zeroOrMore>
+ <text/>
+ </zeroOrMore>
+ </element>
+ </optional>
</interleave>
</group>
</choice>
=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py'
--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py 2011-08-12 11:37:08 +0000
+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_parser.py 2011-08-29 20:28:27 +0000
@@ -986,6 +986,33 @@
result,
'Invalid parsing result for <hardware>')
+ def testHardware_no_sysfs_node(self):
+ """If teh <sysfs-attributes> node is missing, parseHardware()
+ returns a dicitionary where the entry for this node is None.
+ """
+ parser = self.MockSubmissionParserParseHardwareTest(self.log)
+
+ node = etree.fromstring("""
+ <hardware>
+ <hal/>
+ <processors/>
+ <aliases/>
+ <udev/>
+ <dmi/>
+ </hardware>
+ """)
+ result = parser._parseHardware(node)
+ self.assertEqual({
+ 'hal': 'parsed HAL data',
+ 'processors': 'parsed processor data',
+ 'aliases': 'parsed alias data',
+ 'udev': 'parsed udev data',
+ 'dmi': 'parsed DMI data',
+ 'sysfs-attributes': None,
+ },
+ result,
+ 'Invalid parsing result for <hardware>')
+
def test_parseHardware_sub_parsers_fail(self):
"""Test of SubmissionParser._parseHardware().
@@ -2162,6 +2189,17 @@
parser.checkUdevScsiProperties(
[self.udev_root_device, self.udev_scsi_device], sysfs_data))
+ def testCheckUdevScsiProperties_data_is_none(self):
+ """Test of SubmissionParser.checkUdevScsiProperties().
+
+ checkUdevScsiProperties() even if no sysfs properties are
+ available.
+ """
+ parser = SubmissionParser()
+ self.assertTrue(
+ parser.checkUdevScsiProperties(
+ [self.udev_root_device, self.udev_scsi_device], None))
+
def testCheckUdevScsiProperties_missing_devtype(self):
"""Test of SubmissionParser.checkUdevScsiProperties().
=== modified file 'lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py'
--- lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2011-08-13 04:07:10 +0000
+++ lib/canonical/launchpad/scripts/tests/test_hwdb_submission_processing.py 2011-08-29 20:28:27 +0000
@@ -349,6 +349,29 @@
self.assertIs(
None, parser.devices[pci_ethernet_controller_path].sysfs)
+ def test_buildUdevDeviceList_no_sysfs_data(self):
+ """Sysfs data is not required (maverick and natty submissions)."""
+ root_device_path = '/devices/LNXSYSTM:00'
+ pci_pci_bridge_path = '/devices/pci0000:00/0000:00:1c.0'
+ pci_ethernet_controller_path = (
+ '/devices/pci0000:00/0000:00:1c.0/0000:02:00.0')
+
+ udev_paths = (
+ root_device_path, pci_pci_bridge_path,
+ pci_ethernet_controller_path)
+
+ sysfs_data = None
+
+ parsed_data = self.makeUdevDeviceParsedData(udev_paths, sysfs_data)
+ parser = SubmissionParser()
+ parser.buildUdevDeviceList(parsed_data)
+
+ self.assertIs(
+ None, parser.devices[pci_pci_bridge_path].sysfs)
+
+ self.assertIs(
+ None, parser.devices[pci_ethernet_controller_path].sysfs)
+
def test_buildUdevDeviceList_invalid_device_path(self):
"""Test the creation of UdevDevice instances for a submission.
@@ -3875,6 +3898,20 @@
device = UdevDevice(None, self.root_device)
self.assertFalse(device.is_scsi_device)
+ def test_is_scsi_device__no_sysfs_data(self):
+ """Test of UdevDevice.is_scsi_device.
+
+ If there is no sysfs data for a real SCSI device, is it not
+ considered as a real SCSI device.
+
+ Reason: Without sysfs data, we don't know the vendor and
+ model name, making it impossible to store data about the
+ device in the database.
+ """
+ device = UdevDevice(
+ None, self.scsi_scanner_device_data, None)
+ self.assertFalse(device.is_scsi_device)
+
def test_scsi_vendor(self):
"""Test of UdevDevice.scsi_vendor."""
device = UdevDevice(
=== modified file 'lib/lp/hardwaredb/scripts/hwdbsubmissions.py'
--- lib/lp/hardwaredb/scripts/hwdbsubmissions.py 2011-05-27 19:53:20 +0000
+++ lib/lp/hardwaredb/scripts/hwdbsubmissions.py 2011-08-29 20:28:27 +0000
@@ -680,7 +680,12 @@
where the values are the parsing results of _parseHAL,
_parseProcessors, _parseAliases.
"""
- hardware_data = {}
+ # Submissions from checkbox for Maverick and Natty unfortunately do
+ # not contain a <sysfs-attributes> node. A default value here
+ # allows us to mark these submissions.
+ hardware_data = {
+ 'sysfs-attributes': None,
+ }
for node in hardware_node.getchildren():
parser = self._parse_hardware_section[node.tag]
result = parser(node)
@@ -1328,6 +1333,12 @@
should have a corresponding sysfs node, and this node should
define the attributes 'vendor', 'model', 'type'.
"""
+ # Broken submissions from Maverick and Natty. We'll have to deal
+ # with incomplete data for SCSI devices in this case if we don't
+ # want to drop the entire submission, so just pretend that things
+ # are fine.
+ if sysfs_data is None:
+ return True
for device in udev_data:
subsystem = device['E'].get('SUBSYSTEM')
if subsystem != 'scsi':
@@ -1456,13 +1467,18 @@
dmi_data = parsed_data['hardware']['dmi']
for udev_data in parsed_data['hardware']['udev']:
device_path = udev_data['P']
+ if sysfs_data is not None:
+ sysfs_data_for_device = sysfs_data.get(device_path)
+ else:
+ # broken maverick and natty submissions.
+ sysfs_data_for_device = None
if device_path == UDEV_ROOT_PATH:
device = UdevDevice(
- self, udev_data, sysfs_data=sysfs_data.get(device_path),
+ self, udev_data, sysfs_data=sysfs_data_for_device,
dmi_data=dmi_data)
else:
device = UdevDevice(
- self, udev_data, sysfs_data=sysfs_data.get(device_path))
+ self, udev_data, sysfs_data=sysfs_data_for_device)
self.devices[device_path] = device
# The parent-child relations are derived from the path names of
@@ -2731,6 +2747,12 @@
# udev sets the property SUBSYSTEM to "scsi" for a number of
# different nodes: SCSI hosts, SCSI targets and SCSI devices.
# They are distiguished by the property DEVTYPE.
+
+ # Hack for broken submissions from maverick and natty:
+ # If we don't have sysfs information, pretend that no SCSI
+ # related node corresponds to a real device.
+ if self.sysfs is None:
+ return False
properties = self.udev['E']
return (properties.get('SUBSYSTEM') == 'scsi' and
properties.get('DEVTYPE') == 'scsi_device')
=== modified file 'lib/lp/hardwaredb/tests/test_hwdb_submission_validation.py'
--- lib/lp/hardwaredb/tests/test_hwdb_submission_validation.py 2011-08-12 11:37:08 +0000
+++ lib/lp/hardwaredb/tests/test_hwdb_submission_validation.py 2011-08-29 20:28:27 +0000
@@ -697,8 +697,8 @@
is required, and either <hal> or all three tags <udev>, <dmi>,
<sysfs-attributes> must be present.
"""
- # Omitting any of the three tags <udev>, <dmi>, <sysfs-attributes>
- # makes the data invalid.
+ # Omitting one of the tags <udev>, <dmi> makes the data invalid.
+ # Omitting <sysfs-attributes> is tolerated.
all_tags = ['udev', 'dmi', 'sysfs-attributes']
for index, missing_tag in enumerate(all_tags):
test_tags = all_tags[:]
@@ -712,10 +712,13 @@
from_text='<hal',
to_text='</hal>')
result, submission_id = self.runValidator(sample_data)
- self.assertErrorMessage(
- submission_id, result,
- 'Expecting an element %s, got nothing' % missing_tag,
- 'missing tag <%s> in <hardware>' % missing_tag)
+ if missing_tag != 'sysfs-attributes':
+ self.assertErrorMessage(
+ submission_id, result,
+ 'Expecting an element %s, got nothing' % missing_tag,
+ 'missing tag <%s> in <hardware>' % missing_tag)
+ else:
+ self.assertFalse(result is None)
def testHardwareSubTagHalMixedWithUdev(self):
"""Mixing <hal> with <udev>, <dmi>, <sysfs-attributes> is impossible.