[libvirt] PATCH [0/1] Fix locking in udev node device backend
The initial udev node device backend implementation fails to take the lock on the node device driverState struct when adding and removing devices. The following patch adds the appropriate locking. Dave
* The original udev node device backend neglected to lock the driverState struct containing the device list when adding and removing devices. --- src/node_device/node_device_udev.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2bc2d32..be765c4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1190,7 +1190,9 @@ static int udevRemoveOneDevice(struct udev_device *device) int ret = 0; name = udev_device_get_syspath(device); + nodeDeviceLock(driverState); dev = virNodeDeviceFindBySysfsPath(&driverState->devs, name); + if (dev != NULL) { VIR_DEBUG("Removing device '%s' with sysfs path '%s'", dev->def->name, name); @@ -1200,6 +1202,7 @@ static int udevRemoveOneDevice(struct udev_device *device) name); ret = -1; } + nodeDeviceUnlock(driverState); return ret; } @@ -1288,7 +1291,10 @@ static int udevAddOneDevice(struct udev_device *device) goto out; } + nodeDeviceLock(driverState); dev = virNodeDeviceAssignDef(NULL, &driverState->devs, def); + nodeDeviceUnlock(driverState); + if (dev == NULL) { VIR_ERROR("Failed to create device for '%s'", def->name); virNodeDeviceDefFree(def); -- 1.6.5.5
On Tue, Feb 02, 2010 at 02:54:12PM -0500, David Allan wrote:
* The original udev node device backend neglected to lock the driverState struct containing the device list when adding and removing devices. --- src/node_device/node_device_udev.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2bc2d32..be765c4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1190,7 +1190,9 @@ static int udevRemoveOneDevice(struct udev_device *device) int ret = 0;
name = udev_device_get_syspath(device); + nodeDeviceLock(driverState); dev = virNodeDeviceFindBySysfsPath(&driverState->devs, name); + if (dev != NULL) { VIR_DEBUG("Removing device '%s' with sysfs path '%s'", dev->def->name, name); @@ -1200,6 +1202,7 @@ static int udevRemoveOneDevice(struct udev_device *device) name); ret = -1; } + nodeDeviceUnlock(driverState);
return ret; } @@ -1288,7 +1291,10 @@ static int udevAddOneDevice(struct udev_device *device) goto out; }
+ nodeDeviceLock(driverState); dev = virNodeDeviceAssignDef(NULL, &driverState->devs, def); + nodeDeviceUnlock(driverState); + if (dev == NULL) { VIR_ERROR("Failed to create device for '%s'", def->name); virNodeDeviceDefFree(def);
ACK, looks fine to me, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
On Tue, Feb 02, 2010 at 09:27:35PM +0100, Daniel Veillard wrote:
On Tue, Feb 02, 2010 at 02:54:12PM -0500, David Allan wrote:
* The original udev node device backend neglected to lock the driverState struct containing the device list when adding and removing devices. --- src/node_device/node_device_udev.c | 6 ++++++ 1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2bc2d32..be765c4 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -1190,7 +1190,9 @@ static int udevRemoveOneDevice(struct udev_device *device) int ret = 0;
name = udev_device_get_syspath(device); + nodeDeviceLock(driverState); dev = virNodeDeviceFindBySysfsPath(&driverState->devs, name); + if (dev != NULL) { VIR_DEBUG("Removing device '%s' with sysfs path '%s'", dev->def->name, name); @@ -1200,6 +1202,7 @@ static int udevRemoveOneDevice(struct udev_device *device) name); ret = -1; } + nodeDeviceUnlock(driverState);
return ret; } @@ -1288,7 +1291,10 @@ static int udevAddOneDevice(struct udev_device *device) goto out; }
+ nodeDeviceLock(driverState); dev = virNodeDeviceAssignDef(NULL, &driverState->devs, def); + nodeDeviceUnlock(driverState); + if (dev == NULL) { VIR_ERROR("Failed to create device for '%s'", def->name); virNodeDeviceDefFree(def);
ACK, looks fine to me,
Okay, applied ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard -
David Allan