Re: [PATCH] libvirtd: fix potential deadlock when starting vm

["owen.si@xxxxxxxxx" <owen.si@xxxxxxxxx>]

Yes, this is CentOS 6 only, The monitor socket is by default set to NONBLOCK with CentOS 7.

I compare the udev code on centos6 and centos7,

in udev-147 on centos6:

udev_monitor->sock = socket(PF_NETLINK, SOCK_DGRAM|SOCK_CLOEXEC, NETLINK_KOBJECT_UEVENT);

in systemd-219 on centos7:

udev_monitor->sock = socket(PF_NETLINK, SOCK_RAW|SOCK_CLOEXEC|SOCK_NONBLOCK, NETLINK_KOBJECT_UEVENT);

---

owen.si@xxxxxxxxx

 

From: John Ferlan

Date: 2018-10-17 05:57

To: Bingsong Si; libvir-list

CC: Erik Skultety

Subject: Re: [PATCH] libvirtd: fix potential deadlock when starting vm

 

 

On 10/11/18 4:13 AM, Bingsong Si wrote:

> On CentOS 6, udev_monitor_receive_device will block until the socket becomes

 

Is this really CentOS6 only or just where you've seen it?

 

> readable, udevEventHandleThread will hold the lock all the time and

> udevEventHandleCallback hard to get the lock, will block the event poll.

> To fix this, set dataReady to false after receive an udev event.

>

> Signed-off-by: Bingsong Si <owen.si@xxxxxxxxx>

> ---

>  src/node_device/node_device_udev.c | 5 +----

>  1 file changed, 1 insertion(+), 4 deletions(-)

>

 

I've CC'd Erik since he wrote and perhaps remembers all the "gotchas" he

discovered in the udev callback code.

 

I wonder if this has to do with the EAGAIN and EWOULDBLOCK @errno checks

done in the !device loop that are different in "older" (much older) code.

 

Although I have this very vague recollection that there was some problem

with centos6 that was fixed by some OS patch.  Hopefully Erik remembers

(and maybe we should log it in the code at this point ;-)) - I did do

some searching, but came up empty.

 

John

 

> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c

> index 22897591de..ce1101d7cc 100644

> --- a/src/node_device/node_device_udev.c

> +++ b/src/node_device/node_device_udev.c

> @@ -1616,6 +1616,7 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED)

> 

>          errno = 0;

>          device = udev_monitor_receive_device(priv->udev_monitor);

> +        priv->dataReady = false;

>          virObjectUnlock(priv);

> 

>          if (!device) {

> @@ -1637,10 +1638,6 @@ udevEventHandleThread(void *opaque ATTRIBUTE_UNUSED)

>                  return;

>              }

> 

> -            virObjectLock(priv);

> -            priv->dataReady = false;

> -            virObjectUnlock(priv);

> -

>              continue;

>          }

> 

>

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list