[PATCH] xserver: add udev/systemd multi-seat support

Lennart Poettering lennart at poettering.net
Mon Jul 18 11:56:27 PDT 2011


On Mon, 18.07.11 19:29, Lennart Poettering (lennart at poettering.net) wrote:

> If this is supposed to help with the wacom driver, then this is bogus
> too, as the associated rule in /lib/udev/rules.d/70-wacom.rules actually
> does check for ID_INPUT.

So, I had another closer look on this, and now understand what is going
on. This rules file is actually really really borked, and really needs
to be fixed. And not only this files is borked, the matching code in X11
is borked too. 

If I understand this properly, then the wacom serial tablets come with
their own special ISAPNP serial port?

So here's what needs to be fixed:

1) You cannot overide ENV{SUBSYSTEM}. This env var encodes to which
kernel subsystem a device belongs to. Just by overriding this in
userspace you cannot make it move to a different piece of code! This
will break things if left in this way. In fact, after I told Kay about
this he has now added code to udev to explicitly disallow lines like
this. That means these lines will stop working with newer udev unless
they are fixed to stop doing this.

2) If you introduce your own env vars on devices, you are not allowed
not to prefix them. Unprefixed variables are private property of the
kernel. Userspace may only add prefixed variables to that. A common
prefix used is ID_, but you can choose whatever you want. This needs
fixing too. You need to find a better variable than "NAME" to store
your device name.

3) We try to avoid setting pretty names in udev rules, since they might
need translations later on. Hence having a property like "NAME" here
isn't a good idea at all -- not even if you rename it.

4) If you just check attr{id} like you do in those rules this will cause
*all* devices' id attribute to be read, and if if they happen to include
"FUJ" as prefix, you'll mark them as tablet. Example: an audio driver
also exposes an id property, you can even set it to whatevr you want via
a module argument. If a poor soul has a sound card with an id of "FUJ"
then your rule will mark it as input device for X.

To fix this: since the devices in question show up as ISAPNP devices,
this is what you need to check for. i.e. instead of this:

ATTRS{id}=="FUJ*", ...

use this:

SUBSYSTEMS="pnp", ATTRS{id}=="FUJ*", ...

5) You are now adding your properties to *every* child device of the pnp
device which is very much broken. You should only add it to the serial
port itself. I.e. you also need to check SUBSYSTEM="tty". (Singular, not
plural in this case).

6) Where a device shows up in the device tree is not considered ABI,
kernel devs are free and happy to add new layers to the tree where
necessary. That means in the X11 udev code you cannot use
udev_device_get_parent() to check the parent device of the wacom tablet,
assuming it was a pnp device. This is borked and will break at any
time. Use udev_device_get_parent_with_subsystem_devtype() instead, or
much better: assign the properties you are looking for to the serial
device itself, via the udev rule, by inheriting them as necessary.

Sorry that I cannot really prep a patch for this, as I do not own this
hardware. However I am happy to review any patches which fix this, just
CC me.

And again, Kay has changed udev now so that the wacom rule will fail,
due to 1). If this rule file isn't fixed then wacom devices will just
stop working very soon.

I will modify my own patch set for X11 now to look for both devices in
the "input" and the "tty" subsystem. This should prepare things for
wacom, but as mentioned the wacom udev rule needs to be fixed as well.

Lennart

-- 
Lennart Poettering - Red Hat, Inc.


More information about the xorg-devel mailing list