On 07/28/2016 08:02 AM, Jovanka Gulicoska wrote:
---
src/node_device/node_device_udev.c | 46 ++++++++++++++++++++++++++++++--------
1 file changed, 37 insertions(+), 9 deletions(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 76c60ea..e7a407f 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -28,6 +28,7 @@
#include "dirname.h"
#include "node_device_conf.h"
+#include "node_device_event.h"
#include "node_device_driver.h"
#include "node_device_linux_sysfs.h"
#include "node_device_udev.h"
@@ -1024,22 +1025,31 @@ static int udevGetDeviceDetails(struct udev_device *device,
static int udevRemoveOneDevice(struct udev_device *device)
{
virNodeDeviceObjPtr dev = NULL;
+ virObjectEventPtr event = NULL;
const char *name = NULL;
- int ret = 0;
+ int ret = -1;
name = udev_device_get_syspath(device);
dev = virNodeDeviceFindBySysfsPath(&driver->devs, name);
- if (dev != NULL) {
- VIR_DEBUG("Removing device '%s' with sysfs path
'%s'",
- dev->def->name, name);
- virNodeDeviceObjRemove(&driver->devs, dev);
- } else {
+ if (!dev) {
VIR_DEBUG("Failed to find device to remove that has udev name
'%s'",
name);
- ret = -1;
+ goto cleanup;
}
+ event = virNodeDeviceEventLifecycleNew(dev->def->name,
+ VIR_NODE_DEVICE_EVENT_DELETED,
+ 0);
+
+ VIR_DEBUG("Removing device '%s' with sysfs path '%s'",
+ dev->def->name, name);
+ virNodeDeviceObjRemove(&driver->devs, dev);
+
+ ret = 0;
+ cleanup:
+ if (event)
+ virObjectEventStateQueue(driver->nodeDeviceEventState, event);
return ret;
}
@@ -1096,6 +1106,8 @@ static int udevAddOneDevice(struct udev_device *device)
{
virNodeDeviceDefPtr def = NULL;
virNodeDeviceObjPtr dev = NULL;
+ virObjectEventPtr event = NULL;
+ bool new_device;
int ret = -1;
if (VIR_ALLOC(def) != 0)
@@ -1119,17 +1131,28 @@ static int udevAddOneDevice(struct udev_device *device)
if (udevSetParent(device, def) != 0)
goto cleanup;
+ dev = virNodeDeviceFindByName(&driver->devs, def->name);
+ new_device = !!dev;
I know I recommended this little tidbit offline, but the logic is actually
wrong :) new_device should be !dev
+ if (new_device)
+ nodeDeviceUnlock();
+
Wrong unlock call, we need to unlock 'dev', not the whole driver.
/* If this is a device change, the old definition will be freed
* and the current definition will take its place. */
dev = virNodeDeviceAssignDef(&driver->devs, def);
- if (dev == NULL)
- goto cleanup;
+
We shouldn't drop this NULL check either.
These were the only issues in the patch series, so applied locally with this
stuff fixed:
diff --git a/src/node_device/node_device_udev.c
b/src/node_device/node_device_udev.c
index e7a407f..4182d5b 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1107,7 +1107,7 @@ static int udevAddOneDevice(struct udev_device *device)
virNodeDeviceDefPtr def = NULL;
virNodeDeviceObjPtr dev = NULL;
virObjectEventPtr event = NULL;
- bool new_device;
+ bool new_device = true;
int ret = -1;
if (VIR_ALLOC(def) != 0)
@@ -1132,13 +1132,16 @@ static int udevAddOneDevice(struct udev_device *device)
goto cleanup;
dev = virNodeDeviceFindByName(&driver->devs, def->name);
- new_device = !!dev;
- if (new_device)
- nodeDeviceUnlock();
+ if (dev) {
+ virNodeDeviceObjUnlock(dev);
+ new_device = false;
+ }
/* If this is a device change, the old definition will be freed
* and the current definition will take its place. */
dev = virNodeDeviceAssignDef(&driver->devs, def);
+ if (dev == NULL)
+ goto cleanup;
if (new_device)
event = virNodeDeviceEventLifecycleNew(dev->def->name,
I'll push after the 2.1.0 release is out!
Thanks,
Cole