[libvirt] [PATCH v5 4/6] udev: Convert udevEventHandleThread to an actual thread routine

[Erik Skultety]

On Sun, Oct 15, 2017 at 10:23:56AM -0400, John Ferlan wrote:
>
>
> On 10/11/2017 10:52 AM, Erik Skultety wrote:
> > Adjust udevEventHandleThread to be a proper thread routine running in an
> > infinite loop handling devices. The handler thread pulls all available
> > data from the udev monitor and only then waits until a wakeup signal for
> > new incoming data has been emitted by udevEventHandleCallback.
> >
>
> This doing a bit more by removing the driver locks from initialization
> too.  Perhaps those locks should be kept on Initialization for now and
> then in a followup patch remove them since @privateData (or whatever
> name it becomes) is then totally self locking.
>
> I don't think we'd run into any deadlocks since Initialization and
> Cleanup would still be the only consumers.

Like I said, patch will be added.

[...]
> >  static bool
> > @@ -1625,15 +1639,25 @@ udevPCITranslateDeinit(void)
> >  static int
> >  nodeStateCleanup(void)
> >  {
> > +    udevEventDataPtr priv = NULL;
> > +
> >      if (!driver)
> >          return -1;
> >
> > +    priv = driver->privateData;
> > +
>
> Although perhaps impossible, better safe than sorry

Here it's actually quite possible, when allocation of the private data in
Initialize fails, cleanup is performed, so this would get a SIGSEGV, thanks.

Erik

> Perhaps we should keep it just for this patch and remove in the "next"
> patch leaving the goto unlock intact, but modifying it to be :
>
>  unlock:
>     if (priv)
>         virObjectUnlock(priv);
>     nodeDeviceUnlock();
>
> Then the next patch alters the gots's and unlock: label to what this
> patch has. It also removes nodeDeviceLock from Cleanup

Yeah, I'll handle the locking prior to adding this thread-related stuff.