[PATCH] Support filtering of hotplugged input devices

[Dan Nicholson]

On Sat, Oct 24, 2009 at 9:29 AM, Ilia K. <mail4ilia at gmail.com> wrote:
> Hi all.
> I've just filled X.Org Bug 24712
> <http://bugs.freedesktop.org/show_bug.cgi?id=24712> and attached a
> patch to fix the issue.
> Follows a copy of bug/fix description I added to bug report.
>
> The usual deployment of multiseat environment involves running several
> instances of X server simultaneously. If hotplugging is enabled, all
> keyboards/mice are detected and used by all X servers, which makes multiseat
> environment practically unusable. A traditional workaround for this issue was
> disabling hotplugging with
>  Option  "AutoAddDevices" "false"
> However, this workaround misses all the advantages of hotplugging.
> Particularly, changing a physical keyboard/mouse requires X server restart.
> This is especially annoying when some low-end keyboards/mice/usb hubs "blink"
> (go off, then immediately on) due to EMI or something like that.
>
> A possible solution is to allow hotplugging but "filter out" new input device
> notifications for unnecessary devices.
>
> Attached patch implements the fix.
> A user is able to attach x11_layout option to the device which is matched
> against current server layout id. If x11_layout option is missing, is "*"
> or equals to server layout id, the device is added. Otherwise it's ignored.
>
> Assume that in xorg.conf AutoAddDevices option is set to true (which is a
> default) and two ServerLayout sections are present:
> Section "ServerLayout"
>        Identifier     "Seat0"
>        Screen      0  "Screen0" 0 0
> EndSection
> Section "ServerLayout"
>        Identifier     "Seat1"
>        Screen      0  "Screen1" 0 0
> EndSection
>
> Than the following file can be added as
> /etc/hal/fdi/policy/30-multiseat_input.fdi :
>
> <?xml version="1.0" encoding="UTF-8"?>
> <!-- define which keyboard/mouse should be used by which instance of X server
> -->
>
> <deviceinfo version="0.2">
>  <device>
>    <!-- Seat 0 uses usb hub 1-7 only -->
>    <match key="info.category" string="input">
>      <match key="linux.sysfs_path"
> contains="/sys/devices/pci0000:00/0000:00:02.1/usb1/1-7/">
>        <merge key="input.x11_options.x11_layout" type="string">Seat0</merge>
>      </match>
>    </match>
>
>    <!-- Seat 1 uses usb hub 1-8 only -->
>    <match key="info.category" string="input">
>      <match key="linux.sysfs_path"
> contains="/sys/devices/pci0000:00/0000:00:02.1/usb1/1-8/">
>        <merge key="input.x11_options.x11_layout" type="string">Seat1</merge>
>      </match>
>    </match>
>
>    <!-- input devices which don't match above rules will be added to all X
> instances -->
>  </device>
> </deviceinfo>
>
> I've tested the patch (and actually I'm using it right now) on
> xorg-server-1.6.0 under Ubuntu 9.04.

That seems nice, but how will this work in the future of ConsoleKit
multiseat? I'm not crazy about about adding options that are only
available through an undocumented hal key, but two things particularly
jump out at me:

1. Drop the special case of "*" unless all wild card patterns are
handled. An empty or missing key (the default) does the same thing and
doesn't imply that "Seat1*" will work.

2. Can you split up the conditional or make the str*cmp tests more
explicit? It took me a couple minutes to follow the logic there. Also,
please wrap that line. I think the rule for server code is 78
characters.

3. I don't think printing a warning or returning a BadMatch is what
you want to do unless you want to spam people's logs. I would
personally do something like this:

xf86Msg(X_INFO, "Device \"%s\" configured for ServerLayout \"%s\". Skipping.\n",
             idev->identifier, layout);
rval = Success;
goto unwind;

Then the resources will be cleaned up and the hal layer won't complain
that things failed when they actually worked successfully to filter
the device.

--
Dan