[RFC PATCH] Initial libudev input-hotplug support

Rémi Cardona remi at gentoo.org
Tue Sep 29 23:47:27 PDT 2009


Hi Julien,

Le mardi 29 septembre 2009 à 23:54 +0200, Julien Cristau a écrit :
> If libudev is found, we use that for hotplug and disable the hal and
> dbus backends.

Why disable dbus? I see udev being in conflict with HAL, but not with
the dbus code. Not that it matters much, I don't think any tool/utility
uses the dbus interfaces anyway. Maybe it's time to ditch it?

> 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
> - 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."

<harmless bikeshedding>
Why not merge the 2 into a single :

UDEV_PROP_KEY "xorg."
</bikeshedding>

I think the "xkb" HAL keys were deprecated in favor of
"x11_options.Xkb*", let's at least drop the deprecated one.

> +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);
> +}
> +
> +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;
> +
> +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"

Maybe we could have --enable-config={dbus,hal,udev,none} instead?

Anyhow, I don't have much else to say about the real libudev code except
that it's well indented ;)

Rémi



More information about the xorg-devel mailing list