When removing a node device object from the internal list the udevRemoveOneDevice() function does plain unref over the object. This is not sufficient. If there is another thread that's waiting for the object lock it will wait forever. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/node_device/node_device_udev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 3149de8321..0166a13b0f 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1244,7 +1244,7 @@ udevRemoveOneDevice(struct udev_device *device) VIR_DEBUG("Removing device '%s' with sysfs path '%s'", def->name, name); virNodeDeviceObjListRemove(driver->devs, obj); - virObjectUnref(obj); + virNodeDeviceObjEndAPI(&obj); virObjectEventStateQueue(driver->nodeDeviceEventState, event); return 0; -- 2.25.3

Move internals of udevRemoveOneDevice() into a separate function which accepts sysfs path as an argument and actually removes the device from the internal list. It will be reused later. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/node_device/node_device_udev.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 0166a13b0f..e9a76a7b01 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1222,17 +1222,15 @@ udevGetDeviceDetails(struct udev_device *device, static int -udevRemoveOneDevice(struct udev_device *device) +udevRemoveOneDeviceSysPath(const char *path) { virNodeDeviceObjPtr obj = NULL; virNodeDeviceDefPtr def; virObjectEventPtr event = NULL; - const char *name = NULL; - name = udev_device_get_syspath(device); - if (!(obj = virNodeDeviceObjListFindBySysfsPath(driver->devs, name))) { - VIR_DEBUG("Failed to find device to remove that has udev name '%s'", - name); + if (!(obj = virNodeDeviceObjListFindBySysfsPath(driver->devs, path))) { + VIR_DEBUG("Failed to find device to remove that has udev path '%s'", + path); return -1; } def = virNodeDeviceObjGetDef(obj); @@ -1242,7 +1240,7 @@ udevRemoveOneDevice(struct udev_device *device) 0); VIR_DEBUG("Removing device '%s' with sysfs path '%s'", - def->name, name); + def->name, path); virNodeDeviceObjListRemove(driver->devs, obj); virNodeDeviceObjEndAPI(&obj); @@ -1251,6 +1249,15 @@ udevRemoveOneDevice(struct udev_device *device) } +static int +udevRemoveOneDevice(struct udev_device *device) +{ + const char *path = udev_device_get_syspath(device); + + return udevRemoveOneDeviceSysPath(path); +} + + static int udevSetParent(struct udev_device *device, virNodeDeviceDefPtr def) -- 2.25.3

When a device is "move"-d (this basically means it was renamed), we add the new device onto our list but keep the old there too. Fortunately, udev sets this DEVPATH_OLD property which points to the old device path. We can use it to remove the old instance. To test this try renaming an interface, for instance: # ip link set tunl0 name tunl1 # ip link set tunl1 name tunl0 One problem with udev is that it sends old ifname in INTERFACE property, which creates a problem for us, the property is where we get the ifname from and use it then to query all kind of info about the interface. Well, if it is non-existent then we can't query anything. This happens if ifname rename is suppressed (net.ifnames=0 on kernel cmd line for instance). Fortunately, we can use "kernel" source for udev events which has always the fresh info. Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; --- src/node_device/node_device_udev.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index e9a76a7b01..386f23ef3a 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1507,7 +1507,14 @@ udevHandleOneDevice(struct udev_device *device) return udevRemoveOneDevice(device); if (STREQ(action, "move")) { - /* TODO: implement a way of finding and removing the old device */ + const char *devpath_old = udevGetDeviceProperty(device, "DEVPATH_OLD"); + + if (devpath_old) { + g_autofree char *devpath_old_fixed = g_strdup_printf("/sys%s", devpath_old); + + udevRemoveOneDeviceSysPath(devpath_old_fixed); + } + return udevAddOneDevice(device); } @@ -1872,7 +1879,7 @@ nodeStateInitialize(bool privileged, virObjectLock(priv); - priv->udev_monitor = udev_monitor_new_from_netlink(udev, "udev"); + priv->udev_monitor = udev_monitor_new_from_netlink(udev, "kernel"); if (!priv->udev_monitor) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("udev_monitor_new_from_netlink returned NULL")); -- 2.25.3