Remove the HAL dependency from Launchpad HWDB and checkbox

After comparing the output of "lshal" and "udevadm info --export-db" for a few machines and peripherals, I think we get nearly enough data from udevadm and the DMI related sysfs files. (we need mainly vendor/product IDs or vendor/model strings for SCSI devices; the device class/subclass for PCI and USB devices)

 On the positive side, udevadm returns vendor/product data for bluetooth devices, which is missing in HAL.

The only major problem I found is related to some SCSI devices: While the udevadm output contains the SCSI vendor and model strings for disks and CD drives which use the kernel's SCSI layer (.i.e., not only real SCSI devices, but also ATA drives), this data is missing for SCSI scanners. I assume that it is as well missing for the SCSI device type "processor" (used for some Epson and HP scanners, and for example for tape robots.)

But sysfs provides this data: The main directory of a SCSI device, for example /sys/devices/pci0000:00/0000:00:1e.0/0000:15:00.0/0000:16:00.0/host5/target5:0:6/5:0:6:0/, contains the files "vendor", "model" and "type", which provide exactly the data we need.

These directories are easy to find: If a udev path name ends with "/scsi_generic/sgN" (where N is a number), replace this string by "vendor", "model", "type".

Abel Deuring [2009-07-27 16:30 -0000]:
> The only major problem I found is related to some SCSI devices: While
> the udevadm output contains the SCSI vendor and model strings for disks
> and CD drives which use the kernel's SCSI layer (.i.e., not only real
> SCSI devices, but also ATA drives), this data is missing for SCSI
> scanners. I assume that it is as well missing for the SCSI device type
> "processor" (used for some Epson and HP scanners, and for example for
> tape robots.)

Some clarification is in order here, I think. There are two kinds of
information you can process with udev: (1) data from sysfs, which
rules can match on, but aren't stored in udev itself (example:
"vendor", "idProduct", etc.), and (2) additional properties which you
can attach to devices in rules with ENV{VAR}="value". Properties are
stored in udev itself, and that's what you get with --export-db".
export-db will _not_ dump the sysfs properties!

Properties are mainly used to attach additional information to devices
which aren't directly exported by/related to the hardware, such as
"this is a device supported by libgphoto" (ID_GPHOTO2), or the
partition type of a drive (DKD_PARTITION_TYPE).

> But sysfs provides this data: The main directory of a SCSI device, for
> example
> /sys/devices/pci0000:00/0000:00:1e.0/0000:15:00.0/0000:16:00.0/host5/target5:0:6/5:0:6:0/,
> contains the files "vendor", "model" and "type", which provide exactly
> the data we need.
>
> These directories are easy to find: If a udev path name ends with
> "/scsi_generic/sgN" (where N is a number), replace this string by
> "vendor", "model", "type".

It's certainly possible to directly read sysfs for the information you
need. Another option is to iterate over the device paths and use

  udevadm info --query=all --path=/devices/...

which will give you both the properties as well as the sysfs
attributes. For performance reasons you might want to use libudev
instead of calling udevadm 200 times, though.

On 28.07.2009 15:00, Martin Pitt wrote:
> Abel Deuring [2009-07-27 16:30 -0000]:
>> The only major problem I found is related to some SCSI devices: While
>> the udevadm output contains the SCSI vendor and model strings for disks
>> and CD drives which use the kernel's SCSI layer (.i.e., not only real
>> SCSI devices, but also ATA drives), this data is missing for SCSI
>> scanners. I assume that it is as well missing for the SCSI device type
>> "processor" (used for some Epson and HP scanners, and for example for
>> tape robots.)
>
> Some clarification is in order here, I think. There are two kinds of
> information you can process with udev: (1) data from sysfs, which
> rules can match on, but aren't stored in udev itself (example:
> "vendor", "idProduct", etc.), and (2) additional properties which you
> can attach to devices in rules with ENV{VAR}="value". Properties are
> stored in udev itself, and that's what you get with --export-db".
> export-db will _not_ dump the sysfs properties!
>
> Properties are mainly used to attach additional information to devices
> which aren't directly exported by/related to the hardware, such as
> "this is a device supported by libgphoto" (ID_GPHOTO2), or the
> partition type of a drive (DKD_PARTITION_TYPE).

Thanks for the clarification!

>
>> But sysfs provides this data: The main directory of a SCSI device, for
>> example
>> /sys/devices/pci0000:00/0000:00:1e.0/0000:15:00.0/0000:16:00.0/host5/target5:0:6/5:0:6:0/,
>> contains the files "vendor", "model" and "type", which provide exactly
>> the data we need.
>>
>> These directories are easy to find: If a udev path name ends with
>> "/scsi_generic/sgN" (where N is a number), replace this string by
>> "vendor", "model", "type".
>
> It's certainly possible to directly read sysfs for the information you
> need. Another option is to iterate over the device paths and use
>
> udevadm info --query=all --path=/devices/...

Just tried that: It returns exactly the same data for the given device
as --export-db. --attribute-walk returns all attributes, but includes
parent devices too, so we would get a lot of redundant stuff.

>
> which will give you both the properties as well as the sysfs
> attributes. For performance reasons you might want to use libudev
> instead of calling udevadm 200 times, though.
>

Well, we would need to read sysfs attributes only for SCSI devices, and
I think the average system will have less than 10 devices that use the
SCSI subsystem, including disks, CD drivers, USB memory sticks etc.

But: Since checkbox is written in Python: Do we have Python bindings for
libudev?

Abel

I changed the Relax-NG schema file used by launchpad for the
"formal sanity check" of submitted dat so that data from udev,
the DMI stuff and additional data for SCSI devices can replace
the HAL data. See the attached file.

The changes are:

 - the node <hal> is no longer unconditionally required; instead,
   the nodes <udev>, <dmi> and <scsi-devices> may appear.
   (But either <hal> or <udev>, <dmi> and <scsi-devices>
   must appear)
 - <udev> contains the plain output from
        udevadm info --export-db
 - <dmi> contains the output from
        grep -r . /sys/class/dmi/id/
 - <scsi-devices> contains the content of the sysfs file "vendor",
   "model", "type" of those sysfs directories where the udev
   property DEVTYPE has the value "scsi_device". The format is
   the same as that for the dmi data.
 - In theory, we can link questions to devices, with the sub-node
   <target id="...">. (Yes, I know that Matt wants to remove the
   questions completely form the submitted data, but I don't
   want to omit this option completely from the schema.) For
   submissions with HAL data, the value of the attribute "id" was
   the ID of a HAL node, which was an integer. The schema now
   allows arbitrary text; for submissions with udev data, the
   value should be the sysfs path of a device as given in the
   <udev> node.
- The same change applies to the (also unused)
   <alias target="..."> node.

...and here is a sample submission file

If you are worried about the attribute-walk behavior being recursive, here's essentially what udevadm does given the path returned by udevadm info --export-db:

def get_attributes(path):
    sys_path = "/sys%s" % path
    try:
        names = os.listdir(sys_path)
    except OSError:
        return None

    attributes = {}
    for name in names:
        name_path = posixpath.join(sys_path, name)
        if name[0] == "." \
           or name in ["dev", "uevent"] \
           or posixpath.isdir(name_path) \
           or posixpath.islink(name_path):
            continue

        try:
            value = open(name_path, "r").read().strip()
        except IOError:
            continue

        if [c for c in value if not isprint(c)]:
            continue

        attributes[name] = value

    return attributes

Instead of dumping raw text files which will change as new udev rules are introduced, I would propose that Launchpad exposes an API defining what it defines as a device. The current implementation is obviously coupled with what HAL defines as a device. Only a fraction of this information is actually used by Launchpad, so there seems to be a divide between both definitions.

Without being intimately familiar with Launchpad internal requirements for devices, this is what I might propose as a schema to express each device:

<define name="device">
    <interleave>
        <element name="bus">
            <attribute name="value">
                <text/>
            </attribute>
        </element>
        <optional>
            <element name="driver">
                <attribute name="value">
                    <text/>
                </attribute>
            </element>
        </optional>
        <optional>
            <element name="vendor_id">
                <attribute name="value">
                    <ref name="integer"/>
                </attribute>
            </element>
            <element name="product_id">
                <attribute name="value">
                    <ref name="integer"/>
                </attribute>
            </element>
        </optional>
        <optional>
            <element name="subvendor_id">
                <attribute name="value">
                    <ref name="integer"/>
                </attribute>
            </element>
            <element name="subproduct_id">
                <attribute name="value">
                    <ref name="integer"/>
                </attribute>
            </element>
        </optional>
        <optional>
            <element name="vendor">
                <attribute name="value">
                    <text/>
                </attribute>
            </element>
        </optional>
        <optional>
            <element name="product">
                <attribute name="value">
                    <text/>
                </attribute>
            </element>
        </optional>
    </interleave>
</define>

To illustrate the above schema, I have attached a script which parses the output from the command "udevadm info --export-db" and prints a simplified representation of each device on separate lines. The format for this representation should be simple enough to understand considering there are so few attributes.

Furthermore, I have taken this opportunity to introduce another element to the schema called "type". The motivation for this element is to generalize the concept of PCI class across all devices, so that devices can be categorized in accordance with enduser expectations.

Note that the script does not resolve names for pci and usb ids. However, this can be easily compensated by parsing the pci.ids and usb.ids file. At far as names are concerned, the important part at this point is to concentrate on other devices such as scsi for example which don't have such ids files.

For completeness purposes, I have attached the output from running the above script on my own system.

On 23.08.2009 00:59, Marc Tardif wrote:
> If you are worried about the attribute-walk behavior being recursive,
> here's essentially what udevadm does given the path returned by udevadm
> info --export-db:

Right; that's a nice way to add all attribute data. I thin k it would
make sense to include it in the submitted data.

>
> def get_attributes(path):
> sys_path = "/sys%s" % path
> try:
> names = os.listdir(sys_path)
> except OSError:
> return None
>
> attributes = {}
> for name in names:
> name_path = posixpath.join(sys_path, name)
> if name[0] == "." \
> or name in ["dev", "uevent"] \
> or posixpath.isdir(name_path) \
> or posixpath.islink(name_path):
> continue
>
> try:
> value = open(name_path, "r").read().strip()
> except IOError:
> continue
>
> if [c for c in value if not isprint(c)]:
> continue
>
> attributes[name] = value
>
> return attributes
>

On 23.08.2009 01:14, Marc Tardif wrote:
> Instead of dumping raw text files which will change as new udev rules
> are introduced, I would propose that Launchpad exposes an API defining
> what it defines as a device. The current implementation is obviously
> coupled with what HAL defines as a device. Only a fraction of this
> information is actually used by Launchpad, so there seems to be a divide
> between both definitions.

Right, Launchpad uses only a relatively small part of that data.

But I am not sure if the client should do too much processing of the
data. If we notice an error, it is hard to fix, and we might need to
throw away lots of submissions. Errors during server processing are much
easier to track and to fix.

To give an example where things become a bit difficult: The kernel
controls most storage devices via the SCSI layer, meaning that we have
for nearly each IDE disk and nearly each USB stick a "virtual" SCSI
device, controlled by a "virtual" SCSI controller. If you have a look
into the LP file lib/canonical/launchpad/scripte/hwdb-submissions.py,
you'll see a number of other "special cases". The script may need
sometimes some additional data from parent devices. Hence I think it is
better to store all the data we get from udev (and at least some
additional data from the "attribute files" in sysfs.

>
> Without being intimately familiar with Launchpad internal requirements
> for devices, this is what I might propose as a schema to express each
> device:
>
> <define name="device">
> <interleave>
> <element name="bus">
> <attribute name="value">
> <text/>
> </attribute>
> </element>
> <optional>
> <element name="driver">
> <attribute name="value">
> <text/>
> </attribute>
> </element>
> </optional>
> <optional>
> <element name="vendor_id">
> <attribute name="value">
> <ref name="integer"/>
> </attribute>
> </element>
> <element name="product_id">
> <attribute name="value">
> <ref name="integer"/>
> </attribute>
> </element>
> </optional>

bus, vendor_id and product_id are all required attributes -- that's what
the LP database needs to identify a device.

> <optional>
> <element name="subvendor_id">
> <attribute name="value">
> <ref name="integer"/>
> </attribute>
> </element>
> <element name="subproduct_id">
> <attribute name="value">
> <ref name="integer"/>
> </attribute>
> </element>
> </optional>
> <optional>
> <element name="vendor">
> <attribute name="value">
> <text/>
> </attribute>
> </element>
> </optional>
> <optional>
> <element name="product">
> <attribute name="value">
> <text/>
> ...

On 23.08.2009 06:52, Marc Tardif wrote:
> To illustrate the above schema, I have attached a script which parses
> the output from the command "udevadm info --export-db" and prints a
> simplified representation of each device on separate lines. The format
> for this representation should be simple enough to understand
> considering there are so few attributes.
>
> Furthermore, I have taken this opportunity to introduce another element
> to the schema called "type". The motivation for this element is to
> generalize the concept of PCI class across all devices, so that devices
> can be categorized in accordance with enduser expectations.

Right, we need some "type information" for devcies. I had a closer look
into the class and subclass data of PCI and USB devices in existing
submissions -- and they turned out to be quite messy.

My original intention was to represent the devcies classes roughly in a
similar way as you suggest (technically, I intended to use Launchpad's
DBEnum type to "normalize" device classes, but the difference to a set
of fixed strings as your script is using is not inportant). But after a
closer look at the data we really get I concluded that it is better to
store the raw calls data for now and to do some clean up manually. For
example, we have a PCI SCSI controller which claims to be a video
device, and we have all kind of completely nonsense class data in the
submissions. (I can send you a list of all PCI and USB class IDs I found
and some very rough data about how often they apper)

We can later "normalize" this data so that we can do runy DB query like
"show me all USB printers" or "show me all graphics cards from NVidia",

>
> Note that the script does not resolve names for pci and usb ids.
> However, this can be easily compensated by parsing the pci.ids and
> usb.ids file. At far as names are concerned, the important part at this
> point is to concentrate on other devices such as scsi for example which
> don't have such ids files.

As I wrote in another comment for this bug, SCSI devcies have the string
attributes "vendor" and "model" (they have exactly the same meaning as
the vendor and product IDs of PCI and USB devices) and they have the
attribute "type", telling us if the given device is aa disk, tape
driver, CDROM driver etc.

Very good points, and I would like to add another motivation for using raw files instead of structuring the data. If the Launchpad hardware database only relies on specific raw data, we could make sure that apport gathers that same data when reporting bugs. So, the hardware database could even start polling for bug reports instead of being limited to Checkbox submissions. This could potentially make the hardware database even greater!

The only problem I see is with the target element in questions. If devices are expressed as raw data, it is no longer possible to uniquely identify the related device based on a numeric reference. However, since questions should eventually disappear from the hardware database, perhaps this is not an issue. If we could adopt the schema you proposed in this bug shortly, could we consider removing the concept of questions at the same time?

On 23.08.2009 23:16, Marc Tardif wrote:
> Very good points, and I would like to add another motivation for using
> raw files instead of structuring the data. If the Launchpad hardware
> database only relies on specific raw data, we could make sure that
> apport gathers that same data when reporting bugs. So, the hardware
> database could even start polling for bug reports instead of being
> limited to Checkbox submissions. This could potentially make the
> hardware database even greater!

Well, apport could also simply invoke checkbox, if we think this makes
sense ;) As I understand it, checkbox uploads a bunch of attachments to
bugs report. And walking through all bug report to find some hardware
data is quite inefficient.

>
> The only problem I see is with the target element in questions. If
> devices are expressed as raw data, it is no longer possible to uniquely
> identify the related device based on a numeric reference.

I already changed the RelaxNG specification for the "target" attribute
to allow string values. We can simply use the sysfs path. See the new
schema variant and the sample file I uploaded earlier.

> However, since
> questions should eventually disappear from the hardware database,
> perhaps this is not an issue. If we could adopt the schema you proposed
> in this bug shortly, could we consider removing the concept of questions
> at the same time?

Well, you can simply decide not to ask questions at all, and not to send
related data, even if the schema allows questions ;)

I think we need to discuss again what questions to ask and why. Most of
the currently asked questions are IMHO quite pointless:

"Testing your connection to the Internet:"

If a machine does not have an internet connection, you can't send any
data to Launchpad. So, the only thing we can deduce from answers to this
question is how careful the user answered it -- submissions having the
answer "No" should simply not exist.

"Is your keyboard working properly?"

If the keyboard does not work at all, people can't submit the data: you
need the keyboard to type an email address before submitting the report.
So , in the best case we would see reports where people can say that for
example additional buttons for things like changing the sound volume
don't work, or that the keyborad layout is wrong.

If we ask questions about the keyboard at all, they should be more specific.

"Moving the mouse should move the cursor on the screen."

Sure. But how many people will find checkbox without clicking around in
the menus, ie.e, without using a mouse? (More generally, how many people
are able to use a computer without a pointing device?)

A slightly more interesting variant for laptops might be this: "Checkbox
found these pointing devices: A trackpoint, a touchpad and a USB mouse.
Is this list correct? If not, is some device missing, or is a device
listed that does not exist? Does the trackpoint work? Does the touchpad
work? Does the USB mouse work?"

"Detecting your network controller(s)"

That's a somewhat better question, but if the list is incorrect, it
would be good if a follow-up qiuestion would allow people to say that
some device is not listed or that a listed...

I have linked a branch to this bug which replaces the hal dependency with udev instead. Here are some of the considerations taken:

- The nodes <udev>, <dmi> and <scsi-devices> now appear instead of <hal> with the output requested above (note that the dmi information is currently empty because of an unrelated bug which will be fixed before beta).
- The generated submission.xml file passes validation given the schema provided earlier in this bug report (I have attached the file generated on my system for completeness purposes).
- The system_id which used to be generated from hal information still results in the same value.

I believe this meets all the requirements mentionned so far. Abel, I would like to push these changes in concert with Launchpad but I would also like to meet the feature freeze deadline on Thursday. What can we do to make it happen?

On 24.08.2009 23:12, Marc Tardif wrote:
> I have linked a branch to this bug which replaces the hal dependency
> with udev instead. Here are some of the considerations taken:
>
> - The nodes <udev>, <dmi> and <scsi-devices> now appear instead of <hal> with the output requested above (note that the dmi information is currently empty because of an unrelated bug which will be fixed before beta).
> - The generated submission.xml file passes validation given the schema provided earlier in this bug report (I have attached the file generated on my system for completeness purposes).
> - The system_id which used to be generated from hal information still results in the same value.
>
> I believe this meets all the requirements mentionned so far. Abel, I
> would like to push these changes in concert with Launchpad but I would
> also like to meet the feature freeze deadline on Thursday. What can we
> do to make it happen?

JFDI? ;) The data looks good, except for the empty <dmi> node. (But you
already noticed that ;)

Martin made a good point that we should collect attribute data for all
udev nodes which don't have an ID_VENDOR property. So we might consider
to rename <scsi-devices> to <sysfs-attributes> or similar and store
there the content of all publicly readable sysfs attributes for those
udev devices which don't have an ID_VENDOR attribute.

On 25.08.2009 10:44, Abel Deuring wrote:
> JFDI? ;) The data looks good, except for the empty <dmi> node. (But you
> already noticed that ;)

Forget one detail: Can you add the kernel version (from "uname -r") like so:

<summary><kernel-release>2.6.28-14-generic</kernel-release></summary> ?

>
> Martin made a good point that we should collect attribute data for all
> udev nodes which don't have an ID_VENDOR property. So we might consider
> to rename <scsi-devices> to <sysfs-attributes> or similar and store
> there the content of all publicly readable sysfs attributes for those
> udev devices which don't have an ID_VENDOR attribute.
>
>
>

Abel Deuring [2009-08-25 10:27 -0000]:
> Forget one detail: Can you add the kernel version (from "uname -r") like
> so:
>
> <summary><kernel-release>2.6.28-14-generic</kernel-release></summary> ?

Note that /proc/version_signature provides more fine-grained
information (not just the ABI version, also the package version, such
as 2.6.31-6.26-generic)

Done, I have pushed the following changes into the branch linked to this bug report:

- <scsi-devices> has been renamed to <sysfs-attributes> and actually contains the sysfs attributes for each of the devices reported by udevadm. I have used a similar format which looks like this where the path (P) could be used to map devices to attributes:

P: /devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
A: modalias=input:b0019v0000p0002e0000-e0,1,k74,ramlsfw
A: uniq=
A: phys=LNXPWRBN/button/input0
A: name=Power Button (FF)

- <kernel-release> has been added to the <summary> section (by the way, it seems that some attributes in the summary section use underscores and others such as this one use a dash).

For completeness purposes, I have attached the newly generated submission.xml file from my system.

Abel, I will be pushing these changes tomorrow before end of day. Even though I can JFDI, please let me know if you might like any other changes relating to udev which could potentially help Launchpad.

On 25.08.2009 21:47, Marc Tardif wrote:
> Done, I have pushed the following changes into the branch linked to this
> bug report:
>
> - <scsi-devices> has been renamed to <sysfs-attributes> and actually
> contains the sysfs attributes for each of the devices reported by
> udevadm. I have used a similar format which looks like this where the
> path (P) could be used to map devices to attributes:
>
> P: /devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
> A: modalias=input:b0019v0000p0002e0000-e0,1,k74,ramlsfw
> A: uniq=
> A: phys=LNXPWRBN/button/input0
> A: name=Power Button (FF)

Looks good!

>
> - <kernel-release> has been added to the <summary> section (by the way,
> it seems that some attributes in the summary section use underscores and
> others such as this one use a dash).
>
> For completeness purposes, I have attached the newly generated
> submission.xml file from my system.
>
> Abel, I will be pushing these changes tomorrow before end of day. Even
> though I can JFDI, please let me know if you might like any other
> changes relating to udev which could potentially help Launchpad.

I think the current data is fine -- once you actually include data for
the <dmi> tag ;)

On 25.08.2009 21:47, Marc Tardif wrote:
> Done, I have pushed the following changes into the branch linked to this
> bug report:
>
> - <scsi-devices> has been renamed to <sysfs-attributes> and actually
> contains the sysfs attributes for each of the devices reported by
> udevadm. I have used a similar format which looks like this where the
> path (P) could be used to map devices to attributes:
>
> P: /devices/LNXSYSTM:00/LNXPWRBN:00/input/input0
> A: modalias=input:b0019v0000p0002e0000-e0,1,k74,ramlsfw
> A: uniq=
> A: phys=LNXPWRBN/button/input0
> A: name=Power Button (FF)
>
> - <kernel-release> has been added to the <summary> section (by the way,
> it seems that some attributes in the summary section use underscores and
> others such as this one use a dash).
>
> For completeness purposes, I have attached the newly generated
> submission.xml file from my system.
>
> Abel, I will be pushing these changes tomorrow before end of day. Even
> though I can JFDI, please let me know if you might like any other
> changes relating to udev which could potentially help Launchpad.

...this reminds me of bug 379393: "add checkbox submission reference".
It would be great if the Karmic version of checkbox stores the
submission IDs somewhere so that apport can use them to link bug reports
with submissions.

That should be done and actually in Karmic already. I'll double check in a moment and update the bug status accordingly, thanks for bringing this to my attention.

On 26.08.2009 02:26, Marc Tardif wrote:
> ** Changed in: checkbox
> Status: Triaged => Fix Committed
>
> ** Changed in: checkbox
> Assignee: (unassigned) => Marc Tardif (cr3)
>

Sorry for being a PITA, but the DMI data (from grep -r .
/sys/class/dmi/id/ or similar) is still missing. So I think setting the
status to "fix committed" is a bit premature ;)

The problem is unrelated to this bug, it will be addressed separately. There is not necessarily a direct dependency between this bug and the problem with reporting dmi data. In fact, this problem could've occured after this bug had been fixed, committed and released. It's just an unfortunate coincidence that it happened before. I would rather not postpone this important feature and request a feature freeze exception just because there's a bug which could later be addressed without such an exception.

On 26.08.2009 17:09, Marc Tardif wrote:
> The problem is unrelated to this bug, it will be addressed separately.
> There is not necessarily a direct dependency between this bug and the
> problem with reporting dmi data. In fact, this problem could've occured
> after this bug had been fixed, committed and released. It's just an
> unfortunate coincidence that it happened before. I would rather not
> postpone this important feature and request a feature freeze exception
> just because there's a bug which could later be addressed without such
> an exception.
>

well, my point is that a submission without the DMI data is useless --
that's the source for the system's vendor and product name. And this bug
is about the transistion away from HAL to, well, let's say, "udev plus
helpers".

triaging high because its assigned to Abel

has been fixed some longer time ago