[RFC PATCH] Initial libudev input-hotplug support
Peter Hutterer
peter.hutterer at who-t.net
Wed Sep 30 17:56:17 PDT 2009
On Tue, Sep 29, 2009 at 11:54:44PM +0200, Julien Cristau wrote:
> If libudev is found, we use that for hotplug and disable the hal and
> dbus backends.
> We look for event devices with an "x11_driver" property. XKB
> configuration happens using xkb.{rules,model,layout,variant,options}
> properties, and arbitrary options can be set with "x11_options.foo"
> (this is similar to the hal backend).
>
> udev rules would look something like:
> SUBSYSTEM=="input", KERNEL=="event*", ENV{x11_driver}="evdev"
> SUBSYSTEM=="input", KERNEL=="event*", ENV{ID_CLASS}=="kbd", ENV{xkb.layout}="fr", ENV{xkb.options}="terminate:ctrl_alt_bksp,compose:lwin"
>
> ---
> This seems to work fine on my laptop, with/without an USB mouse. One
> weird thing is the device names in udev contain quotes, so you get stuff
> like:
> (II) XINPUT: Adding extended input device ""AT Translated Set 2 keyboard"" (type: KEYBOARD)
>
> Known issues/questions:
> - add_option() should probably be shared with config/hal.c instead of
> duplicated, and the device_is_duplicate() logic should be used here as
> well
stick it in config.c, same with remove_device too I guess.
> - I'm not quite sure about the Timer logic
> - libudev doesn't give any error if it can't get to /sys or
> /dev/.udev/db, we just don't get any devices (this bit me when trying
> out this code in a chroot)
> - what's the recommended way to avoid races between the 'enumerate' and
> 'monitor' parts?
>
> Any comments?
>
> Cheers,
> Julien
>
> config/Makefile.am | 15 ++-
> config/config-backends.h | 16 ++-
> config/config.c | 9 +-
> config/udev.c | 283 +++++++++++++++++++++++++++++++++++++++
> configure.ac | 22 +++-
> hw/xfree86/common/xf86Config.c | 4 +
> hw/xfree86/common/xf86Globals.c | 2 +-
> include/dix-config.h.in | 3 +
> 8 files changed, 343 insertions(+), 11 deletions(-)
> create mode 100644 config/udev.c
>
> diff --git a/config/Makefile.am b/config/Makefile.am
> index 7fa2df8..80e2bb2 100644
> --- a/config/Makefile.am
> +++ b/config/Makefile.am
> @@ -3,10 +3,18 @@ AM_CFLAGS = @DIX_CFLAGS@
> noinst_LTLIBRARIES = libconfig.la
> libconfig_la_SOURCES = config.c config-backends.h
>
> +if CONFIG_UDEV
> +
> +AM_CFLAGS += @UDEV_CFLAGS@
> +libconfig_la_SOURCES += udev.c
> +libconfig_la_LIBADD = @UDEV_LIBS@
> +
> +else
> +
> if CONFIG_NEED_DBUS
> AM_CFLAGS += @DBUS_CFLAGS@
> libconfig_la_SOURCES += dbus-core.c
> -endif
> +libconfig_la_LIBADD = @DBUS_LIBS@
>
> if CONFIG_DBUS_API
> dbusconfigdir = $(sysconfdir)/dbus-1/system.d
> @@ -17,6 +25,11 @@ endif
>
> if CONFIG_HAL
> libconfig_la_SOURCES += hal.c
> +libconfig_la_LIBADD += @HAL_LIBS@
> endif
>
> +endif # CONFIG_NEED_DBUS
> +
> +endif # !CONFIG_UDEV
> +
> EXTRA_DIST = xorg-server.conf x11-input.fdi
> diff --git a/config/config-backends.h b/config/config-backends.h
> index 907e86b..7cd930f 100644
> --- a/config/config-backends.h
> +++ b/config/config-backends.h
> @@ -27,7 +27,12 @@
> #include <dix-config.h>
> #endif
>
> -#ifdef CONFIG_NEED_DBUS
> +#ifdef CONFIG_UDEV
> +int config_udev_init(void);
> +void config_udev_fini(void);
> +#else
> +
> +# ifdef CONFIG_NEED_DBUS
> #include <dbus/dbus.h>
>
> typedef void (*config_dbus_core_connect_hook)(DBusConnection *connection,
> @@ -46,14 +51,15 @@ int config_dbus_core_init(void);
> void config_dbus_core_fini(void);
> int config_dbus_core_add_hook(struct config_dbus_core_hook *hook);
> void config_dbus_core_remove_hook(struct config_dbus_core_hook *hook);
> -#endif
> +# endif
>
> -#ifdef CONFIG_DBUS_API
> +# ifdef CONFIG_DBUS_API
> int config_dbus_init(void);
> void config_dbus_fini(void);
> -#endif
> +# endif
>
> -#ifdef CONFIG_HAL
> +# ifdef CONFIG_HAL
> int config_hal_init(void);
> void config_hal_fini(void);
> +# endif
> #endif
> diff --git a/config/config.c b/config/config.c
> index b013293..3d7df16 100644
> --- a/config/config.c
> +++ b/config/config.c
> @@ -34,7 +34,10 @@
> void
> config_init(void)
> {
> -#if defined(CONFIG_DBUS_API) || defined(CONFIG_HAL)
> +#ifdef CONFIG_UDEV
> + if (!config_udev_init())
> + ErrorF("[config] failed to initialise udev\n");
> +#elif defined(CONFIG_NEED_DBUS)
> if (config_dbus_core_init()) {
> # ifdef CONFIG_DBUS_API
> if (!config_dbus_init())
> @@ -54,7 +57,9 @@ config_init(void)
> void
> config_fini(void)
> {
> -#if defined(CONFIG_DBUS_API) || defined(CONFIG_HAL)
> +#if defined(CONFIG_UDEV)
> + config_udev_fini();
> +#elif defined(CONFIG_NEED_DBUS)
> # ifdef CONFIG_HAL
> config_hal_fini();
> # endif
> diff --git a/config/udev.c b/config/udev.c
> new file mode 100644
> index 0000000..bc0a8ec
> --- /dev/null
> +++ b/config/udev.c
> @@ -0,0 +1,283 @@
> +/*
> + * Copyright © 2009 Julien Cristau
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * Author: Julien Cristau <jcristau at debian.org>
> + */
> +
> +#ifdef HAVE_DIX_CONFIG_H
> +#include <dix-config.h>
> +#endif
> +
> +#include <libudev.h>
> +
> +#include "input.h"
> +#include "inputstr.h"
> +#include "hotplug.h"
> +#include "config-backends.h"
> +#include "os.h"
> +
> +#define UDEV_PROP_KEY "x11_options."
> +#define UDEV_XKB_PROP_KEY "xkb."
> +
> +static void
> +add_option(InputOption **options, const char *key, const char *value)
> +{
> + if (!value || *value == '\0')
> + return;
> +
> + for (; *options; options = &(*options)->next)
> + ;
> + *options = xcalloc(sizeof(**options), 1);
> + if (!*options) /* Yeesh. */
> + return;
> + (*options)->key = xstrdup(key);
> + (*options)->value = xstrdup(value);
> + (*options)->next = NULL;
> +}
> +
> +static void
> +device_added(struct udev_device *udev_device)
> +{
> + const char *path = NULL, *driver = NULL, *name = NULL;
> + char *config_info = NULL;
> + const char *syspath;
> + const char *key, *value, *tmp;
> + InputOption *options = NULL, *tmpo;
> + DeviceIntPtr dev = NULL;
> + struct udev_list_entry *set, *entry;
> + struct udev_device *parent;
> + int rc;
> +
> + driver = udev_device_get_property_value(udev_device, "x11_driver");
> +
> + path = udev_device_get_devnode(udev_device);
> +
> + syspath = udev_device_get_syspath(udev_device);
> +
> + parent = udev_device_get_parent(udev_device);
> + if (parent)
> + name = udev_device_get_property_value(parent, "NAME");
> +
> + if (!driver)
> + return;
> + if (!path)
> + return;
> + if (!syspath)
> + return;
> + options = xcalloc(sizeof(*options), 1);
> + if (!options)
> + return;
> +
> + options->key = xstrdup("_source");
> + options->value = xstrdup("server/udev");
> + if (!options->key || !options->value)
> + goto unwind;
> +
> + add_option(&options, "path", path);
> + add_option(&options, "device", path);
> + add_option(&options, "driver", driver);
> +
> + config_info = xalloc(strlen(syspath) + 6); /* "udev:" + \0 */
> + if (!config_info)
> + goto unwind;
> + sprintf(config_info, "udev:%s", syspath);
> +
> + set = udev_device_get_properties_list_entry(udev_device);
> + udev_list_entry_foreach(entry, set) {
> + key = udev_list_entry_get_name(entry);
> + if (!key)
> + continue;
> + if (!strncasecmp(key, UDEV_PROP_KEY, sizeof(UDEV_PROP_KEY) - 1)) {
> + value = udev_list_entry_get_value(entry);
> + add_option(&options, key + sizeof(UDEV_PROP_KEY) - 1, value);
> + } else if (!strncasecmp(key, UDEV_XKB_PROP_KEY, sizeof(UDEV_XKB_PROP_KEY) - 1)) {
> + tmp = key + sizeof(UDEV_XKB_PROP_KEY) - 1;
> + value = udev_list_entry_get_value(entry);
> + if (!strcasecmp(tmp, "rules"))
> + add_option(&options, "xkb_rules", value);
> + else if (!strcasecmp(tmp, "layout"))
> + add_option(&options, "xkb_layout", value);
> + else if (!strcasecmp(tmp, "variant"))
> + add_option(&options, "xkb_variant", value);
> + else if (!strcasecmp(tmp, "model"))
> + add_option(&options, "xkb_model", value);
> + else if (!strcasecmp(tmp, "options"))
> + add_option(&options, "xkb_options", value);
> + }
> + }
> + add_option(&options, "name", name ? name : "(unnamed)");
> + LogMessage(X_INFO, "config/udev: Adding input device %s (%s)\n",
> + name, path);
> + rc = NewInputDeviceRequest(options, &dev);
> + if (rc != Success)
> + goto unwind;
> +
> + for (; dev; dev = dev->next) {
> + xfree(dev->config_info);
> + dev->config_info = xstrdup(config_info);
> + }
> +
> + unwind:
> + xfree(config_info);
> + while (!dev && (tmpo = options)) {
> + options = tmpo->next;
> + xfree(tmpo->key);
> + xfree(tmpo->value);
> + xfree(tmpo);
> + }
> +
> + return;
> +}
> +
> +static void
> +remove_device(DeviceIntPtr dev)
> +{
> + LogMessage(X_INFO, "config/udev: Removing input device %s\n", dev->name);
> + OsBlockSignals();
> + ProcessInputEvents();
> + DeleteInputDeviceRequest(dev);
> + OsReleaseSignals();
> +}
> +
> +static void
> +device_removed(struct udev_device *device)
> +{
> + DeviceIntPtr dev, next;
> + char *value;
> + const char *syspath = udev_device_get_syspath(device);
> +
> + value = xalloc(strlen(syspath) + 6); /* "udev:" + \0 */
> + if (!value)
> + return;
> + sprintf(value, "udev:%s", syspath);
> +
> + for (dev = inputInfo.devices; dev; dev = next) {
> + next = dev->next;
> + if (dev->config_info && !strcmp(dev->config_info, value))
> + remove_device(dev);
> + }
> + for (dev = inputInfo.off_devices; dev; dev = next) {
> + next = dev->next;
> + if (dev->config_info && !strcmp(dev->config_info, value))
> + remove_device(dev);
> + }
> +
> + xfree(value);
this can be shared as well.
> +}
> +
> +static void
> +wakeup_handler(pointer data, int err, pointer read_mask)
> +{
> + struct udev_monitor *udev_monitor = data;
> + int udev_fd = udev_monitor_get_fd(udev_monitor);
> + struct udev_device *udev_device;
> + const char *action;
> +
> + if (err < 0)
> + return;
> +
> + if (FD_ISSET(udev_fd, (fd_set *)read_mask)) {
> + udev_device = udev_monitor_receive_device(udev_monitor);
> + if (!udev_device)
> + return;
> + action = udev_device_get_action(udev_device);
> + if (!action)
> + ;
> + else if (!strcmp(action, "add"))
> + device_added(udev_device);
> + else if (!strcmp(action, "remove"))
> + device_removed(udev_device);
> + else
> + ErrorF("config/udev: unhandled action %s\n", action);
> + udev_device_unref(udev_device);
> + }
> +}
> +
> +static void
> +block_handler(pointer data, struct timeval **tv, pointer read_mask)
> +{
> +}
> +
> +static struct udev_monitor *udev_monitor;
> +static OsTimerPtr udev_timer;
please stick those towards the top of the file.
> +
> +static CARD32
> +connect_timer(OsTimerPtr timer, CARD32 time, pointer arg) {
> + int fd;
> + struct udev_enumerate *enumerate;
> + struct udev_list_entry *devices, *device;
> + struct udev *udev = udev_monitor_get_udev(udev_monitor);
> +
> + fd = udev_monitor_get_fd(udev_monitor);
> + enumerate = udev_enumerate_new(udev);
> + udev_enumerate_add_match_subsystem(enumerate, "input");
> + udev_enumerate_scan_devices(enumerate);
> + if (udev_monitor_enable_receiving(udev_monitor)) {
> + ErrorF("config/udev: failed to bind the udev monitor\n");
> + goto unwind;
> + }
> + devices = udev_enumerate_get_list_entry(enumerate);
> + udev_list_entry_foreach(device, devices) {
> + const char *syspath = udev_list_entry_get_name(device);
> + struct udev_device *udev_device = udev_device_new_from_syspath(udev, syspath);
> + device_added(udev_device);
> + udev_device_unref(udev_device);
> + }
> + RegisterBlockAndWakeupHandlers(block_handler, wakeup_handler, udev_monitor);
> + AddGeneralSocket(fd);
> + unwind:
> + udev_enumerate_unref(enumerate);
> + if (udev_timer) {
> + TimerFree(udev_timer);
> + udev_timer = NULL;
> + }
> + return 0;
> +}
> +
> +int
> +config_udev_init(void) {
> + struct udev *udev;
> +
> + udev = udev_new();
> + udev_monitor = udev_monitor_new_from_netlink(udev, "udev");
> + if (!udev_monitor)
> + return 0;
> + udev_timer = TimerSet(NULL, 0, 1, connect_timer, NULL);
> + if (!udev_timer)
> + return 0;
> + return 1;
> +}
> +
> +void
> +config_udev_fini(void)
> +{
> + struct udev *udev = udev_monitor_get_udev(udev_monitor);
> + if (udev_timer) {
> + TimerFree(udev_timer);
> + udev_timer = NULL;
> + }
> + RemoveGeneralSocket(udev_monitor_get_fd(udev_monitor));
> + RemoveBlockAndWakeupHandlers(block_handler, wakeup_handler, udev_monitor);
> + udev_monitor_unref(udev_monitor);
> + udev_monitor = NULL;
> + udev_unref(udev);
> +}
> diff --git a/configure.ac b/configure.ac
> index 7fdcf5a..f14b93c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -586,6 +586,7 @@ AC_ARG_ENABLE(multibuffer, AS_HELP_STRING([--enable-multibuffer], [Build Mult
> AC_ARG_ENABLE(dbe, AS_HELP_STRING([--disable-dbe], [Build DBE extension (default: enabled)]), [DBE=$enableval], [DBE=yes])
> AC_ARG_ENABLE(xf86bigfont, AS_HELP_STRING([--disable-xf86bigfont], [Build XF86 Big Font extension (default: disabled)]), [XF86BIGFONT=$enableval], [XF86BIGFONT=no])
> AC_ARG_ENABLE(dpms, AS_HELP_STRING([--disable-dpms], [Build DPMS extension (default: enabled)]), [DPMSExtension=$enableval], [DPMSExtension=yes])
> +AC_ARG_ENABLE(config-udev, AS_HELP_STRING([--enable-config-udev], [Build udev support (default: auto)]), [CONFIG_UDEV=$enableval], [CONFIG_UDEV=auto])
> AC_ARG_ENABLE(config-dbus, AS_HELP_STRING([--enable-config-dbus], [Build D-BUS API support (default: no)]), [CONFIG_DBUS_API=$enableval], [CONFIG_DBUS_API=no])
> AC_ARG_ENABLE(config-hal, AS_HELP_STRING([--disable-config-hal], [Build HAL support (default: auto)]), [CONFIG_HAL=$enableval], [CONFIG_HAL=auto])
> AC_ARG_ENABLE(xfree86-utils, AS_HELP_STRING([--enable-xfree86-utils], [Build xfree86 DDX utilities (default: enabled)]), [XF86UTILS=$enableval], [XF86UTILS=yes])
> @@ -742,6 +743,25 @@ LIBXI="xi >= 1.2.99.1"
> LIBPCIACCESS="pciaccess >= 0.8.0"
> LIBGLIB="glib-2.0 >= 2.16"
>
> +if test "x$CONFIG_UDEV" = xyes &&
> + { test "x$CONFIG_DBUS_API" = xyes || test "x$CONFIG_HAL" = xyes; }; then
> + AC_MSG_ERROR([Hotplugging through both libudev and dbus/hal not allowed])
> +fi
> +
> +PKG_CHECK_MODULES(UDEV, libudev >= 143, [HAVE_LIBUDEV=yes], [HAVE_LIBUDEV=no])
this should probably go up into a LIBUDEV just below the REQUIRED_MODULES
since it requires a specific version.
see 43a2eb794f19a2ba56d653f465fc5f6b2ff0d3d3
> +if test "x$CONFIG_UDEV" = xauto; then
> + CONFIG_UDEV="$HAVE_LIBUDEV"
> +fi
> +AM_CONDITIONAL(CONFIG_UDEV, [test "x$CONFIG_UDEV" = xyes])
> +if test "x$CONFIG_UDEV" = xyes; then
> + CONFIG_DBUS_API=no
> + CONFIG_HAL=no
> + if ! test "x$HAVE_LIBUDEV" = xyes; then
> + AC_MSG_ERROR([udev configuration API requested, but libudev is not installed])
> + fi
> + AC_DEFINE(CONFIG_UDEV, 1, [Use libudev for input hotplug])
> +fi
> +
> dnl HAVE_DBUS is true if we actually have the D-Bus library, whereas
> dnl CONFIG_DBUS_API is true if we want to enable the D-Bus config
> dnl API.
> @@ -774,13 +794,11 @@ if test "x$CONFIG_HAL" = xyes; then
> fi
>
> AC_DEFINE(CONFIG_HAL, 1, [Use the HAL hotplug API])
> - REQUIRED_LIBS="$REQUIRED_LIBS hal"
> CONFIG_NEED_DBUS="yes"
> fi
> AM_CONDITIONAL(CONFIG_HAL, [test "x$CONFIG_HAL" = xyes])
>
> if test "x$CONFIG_NEED_DBUS" = xyes; then
> - REQUIRED_LIBS="$REQUIRED_LIBS dbus-1"
> AC_DEFINE(CONFIG_NEED_DBUS, 1, [Use D-Bus for input hotplug])
> fi
> AM_CONDITIONAL(CONFIG_NEED_DBUS, [test "x$CONFIG_NEED_DBUS" = xyes])
> diff --git a/hw/xfree86/common/xf86Config.c b/hw/xfree86/common/xf86Config.c
> index 40f65bd..6a5d0f8 100644
> --- a/hw/xfree86/common/xf86Config.c
> +++ b/hw/xfree86/common/xf86Config.c
> @@ -1457,6 +1457,10 @@ checkCoreInputDevices(serverLayoutPtr servlayoutp, Bool implicitLayout)
> xf86Msg(X_INFO, "The server relies on HAL to provide the list of "
> "input devices.\n\tIf no devices become available, "
> "reconfigure HAL or disable AllowEmptyInput.\n");
> +#elif CONFIG_UDEV
> + xf86Msg(X_INFO, "The server relies on udev to provide the list of "
> + "input devices.\n\tIf no devices become available, "
> + "reconfigure udev or disable AllowEmptyInput.\n");
> #else
> xf86Msg(X_INFO, "HAL is disabled and no input devices were configured.\n"
> "\tTry disabling AllowEmptyInput.\n");
char *config_subsystem;
#ifdef udev
config_subsystem = "udev"
#else
config_subsystem = "HAL"
#endif
and then stick the subsystem in with %s into the error message. saves us
having the error message twice.
> diff --git a/hw/xfree86/common/xf86Globals.c b/hw/xfree86/common/xf86Globals.c
> index d8f7f7f..0b310e8 100644
> --- a/hw/xfree86/common/xf86Globals.c
> +++ b/hw/xfree86/common/xf86Globals.c
> @@ -132,7 +132,7 @@ xf86InfoRec xf86Info = {
> .kbdCustomKeycodes = FALSE,
> .disableRandR = FALSE,
> .randRFrom = X_DEFAULT,
> -#ifdef CONFIG_HAL
> +#if defined(CONFIG_HAL) || defined(CONFIG_UDEV)
> .allowEmptyInput = TRUE,
> .autoAddDevices = TRUE,
> .autoEnableDevices = TRUE
> diff --git a/include/dix-config.h.in b/include/dix-config.h.in
> index 798d9e7..423ba3a 100644
> --- a/include/dix-config.h.in
> +++ b/include/dix-config.h.in
> @@ -384,6 +384,9 @@
> /* Support D-Bus */
> #undef HAVE_DBUS
>
> +/* Use libudev for input hotplug */
> +#undef CONFIG_UDEV
> +
> /* Use D-Bus for input hotplug */
> #undef CONFIG_NEED_DBUS
last thing and this may just be my vim rendering it badly - we usually
indent by 4 spaces, not 8. Most of the code is 8 space indented (2 tabs).
I can't comment much on the actual libudev implemention though. Since others
said the same thing here it may be worth sticking it onto a branch to allow
for easier testing.
Cheers,
Peter
More information about the xorg-devel
mailing list