[PATCH xorg-gtest 6/6] device: use inotify to wait for the device to appear

[Chase Douglas]

On 08/29/2012 03:25 PM, Peter Hutterer wrote:
> On Wed, Aug 29, 2012 at 01:09:32PM -0700, Chase Douglas wrote:
>> On 08/28/2012 11:14 PM, Peter Hutterer wrote:
>>> uinput can be too slow, leaving us with GuessDeviceNode() failing and an
>>> empty device string. Use inotify to tell us when a /dev/input/event device
>>> appeared and then continue.  However, even when inotify tells us the device
>>> is there, the EVIOCGNAME may still fail on the device, so we need to keep
>>> the backup GuessDeviceNode() in place.
>>>
>>> This leaves us with a race condition - if a different device pops up while
>>> we're waiting, then we may still not get the device name. Chance of that
>>> happening is tiny.
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
>>> ---
>>>   src/device.cpp                     | 59 +++++++++++++++++++++++++++++++++++++-
>>>   test/.gitignore                    |  1 +
>>>   test/Makefile.am                   | 10 +++++--
>>>   test/PIXART-USB-OPTICAL-MOUSE.desc | 41 ++++++++++++++++++++++++++
>>>   test/device-test.cpp               | 33 +++++++++++++++++++++
>>>   5 files changed, 141 insertions(+), 3 deletions(-)
>>>   create mode 100644 test/PIXART-USB-OPTICAL-MOUSE.desc
>>>   create mode 100644 test/device-test.cpp
>>>
>>> diff --git a/src/device.cpp b/src/device.cpp
>>> index 232d3ae..ea98d17 100644
>>> --- a/src/device.cpp
>>> +++ b/src/device.cpp
>>> @@ -31,6 +31,8 @@
>>>   #include <linux/input.h>
>>>   #include <fcntl.h>
>>>   #include <dirent.h>
>>> +#include <sys/inotify.h>
>>> +#include <poll.h>
>>>
>>>   #include <stdexcept>
>>>
>>> @@ -110,6 +112,48 @@ void xorg::testing::evemu::Device::GuessDeviceNode(time_t ctime) {
>>>     free(event_devices);
>>>   }
>>>
>>> +static std::string wait_for_inotify(int fd)
>>> +{
>>> +  std::string devnode;
>>> +  bool found = false;
>>> +  struct pollfd pfd;
>>> +
>>> +  pfd.fd = fd;
>>> +  pfd.events = POLLIN;
>>> +
>>> +  char buf[1024];
>>> +  size_t bufidx = 0;
>>> +
>>> +  while (!found && poll(&pfd, 1, 2000) > 0) {
>>> +    ssize_t r;
>>> +
>>> +    r = read(fd, buf + bufidx, sizeof(buf) - bufidx);
>>> +    if (r == -1 && errno != EAGAIN) {
>>> +      std::cerr << "inotify read failed with: " << std::string(strerror(errno)) << std::endl;
>>> +      break;
>>> +    }
>>> +
>>> +    bufidx += r;
>>> +
>>> +    struct inotify_event *e = reinterpret_cast<struct inotify_event*>(buf);
>>> +
>>> +    while (bufidx > sizeof(*e) && bufidx >= sizeof(*e) + e->len) {
>>> +      if (strncmp(e->name, "event", 5) == 0) {
>>> +        devnode = DEV_INPUT_DIR + std::string(e->name);
>>> +        found = true;
>>> +        break;
>>> +      }
>>> +
>>> +      /* this packet didn't contain what we're looking for */
>>> +      int len = sizeof(*e) + e->len;
>>> +      memmove(buf, buf + len, bufidx - len);
>>> +      bufidx -= len;
>>> +    }
>>> +  }
>>> +
>>> +  return devnode;
>>> +}
>>> +
>>>   xorg::testing::evemu::Device::Device(const std::string& path)
>>>       : d_(new Private) {
>>>     static const char UINPUT_NODE[] = "/dev/uinput";
>>> @@ -132,6 +176,14 @@ xorg::testing::evemu::Device::Device(const std::string& path)
>>>
>>>     fclose(fp);
>>>
>>> +  int ifd = inotify_init1(IN_NONBLOCK);
>>> +  if (ifd == -1 || inotify_add_watch(ifd, DEV_INPUT_DIR, IN_CREATE) == -1) {
>>> +    std::cerr << "Failed to create inotify watch" << std::endl;
>>> +    if (ifd != -1)
>>> +      close(ifd);
>>> +    ifd = -1;
>>> +  }
>>> +
>>>     d_->fd = open(UINPUT_NODE, O_WRONLY);
>>>     if (d_->fd < 0) {
>>>       evemu_delete(d_->device);
>>> @@ -145,7 +197,12 @@ xorg::testing::evemu::Device::Device(const std::string& path)
>>>       throw std::runtime_error("Failed to create evemu device");
>>>     }
>>>
>>> -  GuessDeviceNode(d_->ctime);
>>> +  if (ifd != -1) {
>>> +    std::string devnode = wait_for_inotify(ifd);
>>> +    if (event_is_device(devnode, evemu_get_name(d_->device), d_->ctime))
>>> +        d_->device_node = devnode;
>>> +    close(ifd);
>>
>> We could instead loop around here waiting until the correct device
>> is seen, potentially with a timeout? I'm guessing you've already
>> thought of that, so I'm interested to hear your thoughts.
>
> the problem here is the failed ioctl(EVIOCGNAME). inotify gives us the right
> device, but because the ioctl fails we cannot be sure it really is our
> device.
>
> the alternative here is to loop until the ioctl succeeds but I don't know if
> that can hang the test, and putting a loop limit on it just means we'll need
> GuessDeviceNode() again.
>
> you can try it, remove the GuessDeviceNode() call and run the new test in a
> loop
>
>      while [ $? -eq 0 ]; do sudo ./device-test; done
>
> it'll run anywhere from a minute to several minutes before it fails.

Hmm... strange. Oh well. This shouldn't be a huge concern when running 
the tests.

Reviewed-by: Chase Douglas <chase.douglas at canonical.com>