I'd title this: node_device: udev: implement lifecycle events On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
--- src/node_device/node_device_udev.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 76c60ea..a4748ef 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,6 +1025,7 @@ 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;
Make this 'ret = -1'. So 'goto cleanup'; can be used for error conditions.
@@ -1031,15 +1033,24 @@ static int udevRemoveOneDevice(struct udev_device *device) dev = virNodeDeviceFindBySysfsPath(&driver->devs, name);
if (dev != NULL) { + 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); + goto cleanup; } else { VIR_DEBUG("Failed to find device to remove that has udev name '%s'", name); ret = -1; + goto cleanup; }
Take this else block and raise it up to be if (!dev) ... goto cleanup; Then handle the lifecycle event creation outside of an if block. And put ret = 0; right before the 'cleanup:'. This is typically how the pattern works
+ cleanup: + if (event) + virObjectEventStateQueue(driver->nodeDeviceEventState, event); return ret; }
@@ -1096,6 +1107,7 @@ static int udevAddOneDevice(struct udev_device *device) { virNodeDeviceDefPtr def = NULL; virNodeDeviceObjPtr dev = NULL; + virObjectEventPtr event = NULL; int ret = -1;
if (VIR_ALLOC(def) != 0) @@ -1125,11 +1137,18 @@ static int udevAddOneDevice(struct udev_device *device) if (dev == NULL) goto cleanup;
+ event = virNodeDeviceEventLifecycleNew(dev->def->name, + VIR_NODE_DEVICE_EVENT_CREATED, + 0); + virNodeDeviceObjUnlock(dev);
This section is problematic if we want to add a separate UPDATED event, since this same function is used for both an updated and added device. I think before calling NodeDeviceAssignDef, you'll want to call virNodeDeviceFindByName to check if the device already exists or not. Only emit the CREATED event if it's a brand new device.
ret = 0;
cleanup: + if (event) + virObjectEventStateQueue(driver->nodeDeviceEventState, event); + if (ret != 0) { VIR_DEBUG("Discarding device %d %p %s", ret, def, def ? NULLSTR(def->sysfs_path) : ""); @@ -1241,6 +1260,8 @@ static int nodeStateCleanup(void)
nodeDeviceLock();
+ virObjectEventStateFree(driver->nodeDeviceEventState); + priv = driver->privateData;
if (priv) { @@ -1456,6 +1477,7 @@ static int nodeStateInitialize(bool privileged,
driver->privateData = priv; nodeDeviceLock(); + driver->nodeDeviceEventState = virObjectEventStateNew();
if (udevPCITranslateInit(privileged) < 0) goto cleanup; @@ -1526,6 +1548,8 @@ static virNodeDeviceDriver udevNodeDeviceDriver = { .nodeNumOfDevices = nodeNumOfDevices, /* 0.7.3 */ .nodeListDevices = nodeListDevices, /* 0.7.3 */ .connectListAllNodeDevices = nodeConnectListAllNodeDevices, /* 0.10.2 */ + .connectNodeDeviceEventRegisterAny = nodeConnectNodeDeviceEventRegisterAny, /* 2.1.0 */ + .connectNodeDeviceEventDeregisterAny = nodeConnectNodeDeviceEventDeregisterAny, /* 2.1.0 */
Update these comments to 2.2.0 Thanks, Cole
.nodeDeviceLookupByName = nodeDeviceLookupByName, /* 0.7.3 */ .nodeDeviceLookupSCSIHostByWWN = nodeDeviceLookupSCSIHostByWWN, /* 1.0.2 */ .nodeDeviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.7.3 */