Re: [libvirt PATCH] nodedev: fix race in API usage vs initial device enumeration

During startup the udev node device driver impl uses a background thread to populate the list of devices to avoid blocking the daemon startup entirely. There is no synchronization to the public APIs, so it is possible for an application to start calling APIs before the device initialization is complete. This was not a problem in the old approach where libvirtd was started on boot, as initialization would easily complete before any APIs were called. With the use of socket activation, however, APIs are invoked from the very moment the daemon starts. This is easily seen by doing a 'virsh -c nodedev:///system list' the first time it runs it will only show one or two devices. The second time it runs it will show all devices. The solution is to introduce a flag and condition variable for APIs to synchronize against before returning any data. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/conf/virnodedeviceobj.h | 2 ++ src/node_device/node_device_driver.c | 44 ++++++++++++++++++++++++++++ src/node_device/node_device_hal.c | 15 ++++++++++ src/node_device/node_device_udev.c | 13 ++++++++ 4 files changed, 74 insertions(+)

@@ -1744,6 +1745,11 @@ nodeStateInitializeEnumerate(void *opaque) if (udevEnumerateDevices(udev) != 0) goto error; + nodeDeviceLock(); + driver->initialized = true; + nodeDeviceUnlock(); + virCondBroadcast(&driver->initCond);

+ return; error: @@ -1806,6 +1812,13 @@ nodeStateInitialize(bool privileged, VIR_FREE(driver); return VIR_DRV_STATE_INIT_ERROR; } + if (virCondInit(&driver->initCond) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Unable to initialize condition variable"));

+ virMutexDestroy(&driver->lock); + VIR_FREE(driver); + return VIR_DRV_STATE_INIT_ERROR; + } driver->privileged = privileged;