[PATCH] Back off if we detect a vmmouse kernel driver v3
Thomas Hellstrom
thellstrom at vmware.com
Wed Oct 8 11:06:06 PDT 2014
On 10/07/2014 05:01 PM, Sinclair Yeh wrote:
> Reviewed-by: Sinclair Yeh <syeh at vmware.com>
Thanks for the review. Pushed.
/Thomas
>
> On Mon, Oct 06, 2014 at 10:30:12PM +0200, Thomas Hellstrom wrote:
>> On 10/06/2014 04:59 PM, Sinclair Yeh wrote:
>>> On Mon, Oct 06, 2014 at 01:17:13PM +0200, Thomas Hellstrom wrote:
>>>> If a vmmouse kernel driver is active, vmmouse input is handled by the Xorg
>>>> evdev driver and not by the vmmouse driver, so make sure the vmmouse_detect
>>>> utility doesn't detect a vmmouse if a kernel driver is active.
>>>>
>>>> v2: Change the vmmouse kernel device name, fix comment.
>>>> v3: Fix up libudev error handling.
>>>>
>>>> Signed-off-by: Thomas Hellstrom <thellstrom at vmware.com>
>>>> ---
>>>> configure.ac | 14 +++++++
>>>> tools/Makefile.am | 7 +++-
>>>> tools/vmmouse_detect.c | 5 +++
>>>> tools/vmmouse_udev.c | 105 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>> 4 files changed, 129 insertions(+), 2 deletions(-)
>>>> create mode 100644 tools/vmmouse_udev.c
>>>>
>>>> diff --git a/configure.ac b/configure.ac
>>>> index ad05504..1197555 100644
>>>> --- a/configure.ac
>>>> +++ b/configure.ac
>>>> @@ -105,6 +105,20 @@ XORG_DRIVER_CHECK_EXT(RANDR, randrproto)
>>>> XORG_DRIVER_CHECK_EXT(XINPUT, inputproto)
>>>>
>>>> # Checks for pkg-config packages
>>>> +libudev_check=yes
>>>> +AC_ARG_WITH([libudev],
>>>> + [AS_HELP_STRING([--without-libudev],
>>>> + [Use to build without libudev on linux])],
>>>> + [if test x$withval = xno; then libudev_check=no; fi]
>>>> + [])
>>>> +
>>>> +if test x`uname` = xLinux -a $libudev_check = yes; then
>>>> + PKG_CHECK_MODULES(LIBUDEV, [libudev],
>>>> + [AC_DEFINE([HAVE_LIBUDEV], 1,
>>>> + [Has libudev installed])],
>>>> + []);
>>>> +fi
>>>> +
>>>> PKG_CHECK_MODULES(XORG, [xorg-server >= 1.0.1] xproto $REQUIRED_MODULES)
>>>>
>>>> PKG_CHECK_EXISTS([xorg-server >= 1.1.0],
>>>> diff --git a/tools/Makefile.am b/tools/Makefile.am
>>>> index 8ae5516..a1396ba 100644
>>>> --- a/tools/Makefile.am
>>>> +++ b/tools/Makefile.am
>>>> @@ -22,8 +22,11 @@ bin_PROGRAMS = @DRIVER_NAME at _detect
>>>>
>>>> AM_CPPFLAGS = -I$(top_srcdir)/shared $(XORG_CFLAGS)
>>>>
>>>> - at DRIVER_NAME@_detect_SOURCES = vmmouse_detect.c
>>>> - at DRIVER_NAME@_detect_LDADD = $(top_builddir)/shared/lib at DRIVER_NAME@.la
>>>> + at DRIVER_NAME@_detect_SOURCES = vmmouse_detect.c vmmouse_udev.c
>>>> + at DRIVER_NAME@_detect_LDADD = $(top_builddir)/shared/lib at DRIVER_NAME@.la \
>>>> + @LIBUDEV_LIBS@
>>>> + at DRIVER_NAME@_detect_CFLAGS = @LIBUDEV_CFLAGS@
>>>> +
>>>>
>>>> calloutsdir=$(HAL_CALLOUTS_DIR)
>>>> callouts_SCRIPTS = hal-probe-vmmouse
>>>> diff --git a/tools/vmmouse_detect.c b/tools/vmmouse_detect.c
>>>> index 7939ff8..b743b2d 100644
>>>> --- a/tools/vmmouse_detect.c
>>>> +++ b/tools/vmmouse_detect.c
>>>> @@ -34,6 +34,8 @@
>>>> #include <signal.h>
>>>> #include "vmmouse_client.h"
>>>>
>>>> +extern int vmmouse_uses_kernel_driver(void);
>>>> +
>>>> void
>>>> segvCB(int sig)
>>>> {
>>>> @@ -46,6 +48,9 @@ segvCB(int sig)
>>>> int
>>>> main(void)
>>>> {
>>>> + if (vmmouse_uses_kernel_driver())
>>>> + return 1;
>>>> +
>>>> /*
>>>> * If the vmmouse test is not run in a VMware virtual machine, it
>>>> * will segfault instead of successfully accessing the port.
>>>> diff --git a/tools/vmmouse_udev.c b/tools/vmmouse_udev.c
>>>> new file mode 100644
>>>> index 0000000..4e51dd6
>>>> --- /dev/null
>>>> +++ b/tools/vmmouse_udev.c
>>>> @@ -0,0 +1,105 @@
>>>> +/*
>>>> + * Copyright 2014 by VMware, Inc.
>>>> + *
>>>> + * 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 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 COPYRIGHT HOLDER(S) OR AUTHOR(S) 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.
>>>> + *
>>>> + * Except as contained in this notice, the name of the copyright holder(s)
>>>> + * and author(s) shall not be used in advertising or otherwise to promote
>>>> + * the sale, use or other dealings in this Software without prior written
>>>> + * authorization from the copyright holder(s) and author(s).
>>>> + */
>>>> +
>>>> +#ifdef HAVE_CONFIG_H
>>>> +#include "config.h"
>>>> +#endif
>>>> +
>>>> +#ifdef HAVE_LIBUDEV
>>>> +#include <libudev.h>
>>>> +#include <stdlib.h>
>>>> +#include <string.h>
>>>> +
>>>> +#define KERNEL_DEVNAME "VirtualPS/2 VMware VMMouse"
>>>> +
>>>> +/**
>>>> + * vmmouse_uses_kernel_driver - Check whether there's an active
>>>> + * vmmouse driver in the kernel.
>>>> + *
>>>> + * Returns 0 if there was no kernel driver found.
>>>> + * Returns non-zero on error or if there was an active driver found.
>>>> + *
>>>> + * Scans the input subsystem for devices matching KERNEL_DEVNAME. These
>>>> + * devices are assumed to be active vmmouse drivers.
>>>> + */
>>>> +int vmmouse_uses_kernel_driver(void)
>>>> +{
>>>> + struct udev *udev;
>>>> + struct udev_enumerate *enumerate;
>>>> + struct udev_list_entry *devices, *dev_list_entry;
>>>> + struct udev_device *dev;
>>>> +
>>>> + udev = udev_new();
>>>> + if (!udev)
>>>> + return 1;
>>>> +
>>>> + /*
>>>> + * Udev error return codes that are not caught immediately are
>>>> + * typically caught in the input argument check in the udev
>>>> + * function calls following the failing call!
>>>> + */
>>>> + enumerate = udev_enumerate_new(udev);
>>>> + if (udev_enumerate_add_match_subsystem(enumerate, "input"))
>>>> + goto out_err;
>>>> + if (udev_enumerate_scan_devices(enumerate))
>>>> + goto out_err;
>>>> +
>>>> + devices = udev_enumerate_get_list_entry(enumerate);
>>>> + udev_list_entry_foreach(dev_list_entry, devices) {
>>>> + const char *path, *name;
>>>> +
>>>> + path = udev_list_entry_get_name(dev_list_entry);
>>>> + dev = udev_device_new_from_syspath(udev, path);
>>>> + if (!dev)
>>>> + goto out_err;
>>>> + name = udev_device_get_sysattr_value(dev, "name");
>>>> + if (name && !strcasecmp(name, KERNEL_DEVNAME))
>>>> + goto out_found;
>>>> +
>>>> + udev_device_unref(dev);
>>>> + }
>>>> +
>>>> + out_not_found:
>>>> + udev_enumerate_unref(enumerate);
>>>> + udev_unref(udev);
>>>> +
>>>> + return 0;
>>>> +
>>>> + out_found:
>>>> + udev_device_unref(dev);
>>>> + out_err:
>>>> + udev_enumerate_unref(enumerate);
>>>> + udev_unref(udev);
>>> If there's an error, would it make sense to let the vmmouse driver try
>>> to drive it, e.g. return 0?
>> If udev errors, and we enable the user-space vmmouse driver, it might
>> clash with a kernel driver which is a bad situation. If we instead back
>> off, the mouse will be treated as a normal ps/2 mouse. We'll lose the
>> VMMouse features, but the mouse will still work.
>>
>>> It seems to me that the failure case should return the same value as
>>> the case when udev is not enabled.
>> The --without-libudev option is intended for the situation where the
>> distro maintainer knows for sure that there will be no kernel vmmouse
>> driver enabled and where he doesn't want to or can't introduce a libudev
>> dependency. In that situation, we always enable the vmmouse.
>>
>>> I'm not sure how udev is used in vmmouse driver, so maybe it'll run
>>> into the same failure on its side. In that case the code looks okay
>>> to me.
>> The vmmouse driver itself doesn't use libudev, so the errors are treated
>> the way they are only because of the above considerations.
>>
>>> Sinclair
>>>
>>>
>> Thanks,
>> Thomas
>>
>>
>>
>>>> +
>>>> + return 1;
>>>> +}
>>>> +#else
>>>> +int vmmouse_uses_kernel_driver(void)
>>>> +{
>>>> + return 0;
>>>> +}
>>>> +#endif
>>>> --
>>>> 1.8.3.2
>>>>
More information about the xorg-devel
mailing list