[libvirt] [PATCH 0/9] Introducing node device lifecycle event APIs

Introducing implementation of node device event APIs. Code changes follow network/storage pool event APIs. Implemented functions: virNodeDeviceEventRegisterAny(), virNodeDeviceEventDeregisterAny(), virNodeDeviceLifeCycleEventNew(), introduced CREATED and DELETED. There are also test as well as unittests for the new functions and signals. This is part of a GSOC project: Asynchronous lifecycle events for storage objects Jovanka Gulicoska (9): Introduce node device lifecycle event APIs conf: add node_device_event handling conf: events: don't crash on NULL uuid test: implement node device lifecycle event APIs remote: implement node device lifecycle event APIs node_device: implement node device lifecycle event APIs node_device: Implement event queue in udev event-test: support node device lifecycle event APIs virsh: Introduce nodedev-event command daemon/libvirtd.h | 2 + daemon/remote.c | 206 ++++++++++++++++++++++++++++++ examples/object-events/event-test.c | 68 ++++++++++ include/libvirt/libvirt-nodedev.h | 90 ++++++++++++++ po/POTFILES.in | 1 + src/Makefile.am | 5 + src/conf/node_device_conf.h | 4 + src/conf/node_device_event.c | 234 +++++++++++++++++++++++++++++++++++ src/conf/node_device_event.h | 59 +++++++++ src/conf/object_event.c | 3 +- src/datatypes.h | 13 ++ src/driver-nodedev.h | 14 +++ src/libvirt-nodedev.c | 127 +++++++++++++++++++ src/libvirt_private.syms | 5 + src/libvirt_public.syms | 6 + src/node_device/node_device_driver.c | 42 +++++++ src/node_device/node_device_driver.h | 10 ++ src/node_device/node_device_udev.c | 24 ++++ src/remote/remote_driver.c | 139 +++++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++- src/remote_protocol-structs | 19 +++ src/test/test_driver.c | 49 ++++++++ tests/objecteventtest.c | 72 +++++++++++ tools/virsh-nodedev.c | 211 +++++++++++++++++++++++++++++++ tools/virsh-nodedev.h | 10 ++ tools/virsh.pod | 18 +++ 26 files changed, 1472 insertions(+), 2 deletions(-) create mode 100644 src/conf/node_device_event.c create mode 100644 src/conf/node_device_event.h -- 2.7.4

Node device lifecycle event API entry points for registering and deregistering node deivce events, as well as types of events associated with node devices. These entry points will be used for implementing asynchronous lifecycle events. Node device API: virConnectNodeDeviceEventRegisterAny virConnectNodeDeviceEventDeregisterAny virNodeDeviceEventLifecycleType which has events CREATED and DELETED --- include/libvirt/libvirt-nodedev.h | 90 +++++++++++++++++++++++++++ po/POTFILES.in | 1 + src/datatypes.h | 13 ++++ src/driver-nodedev.h | 14 +++++ src/libvirt-nodedev.c | 127 ++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 6 files changed, 251 insertions(+) diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index cd0b548..8a35470 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -121,5 +121,95 @@ virNodeDevicePtr virNodeDeviceCreateXML (virConnectPtr conn, int virNodeDeviceDestroy (virNodeDevicePtr dev); +/** + * VIR_NODE_DEVICE_EVENT_CALLBACK: + * + * Used to cast the event specific callback into the generic one + * for use for virConnectNodeDeviceEventRegisterAny() + */ +# define VIR_NODE_DEVICE_EVENT_CALLBACK(cb)((virConnectNodeDeviceEventGenericCallback)(cb)) + +/** + * virNodeDeviceEventID: + * + * An enumeration of supported eventId parameters for + * virConnectNodeDeviceEventRegisterAny(). Each event id determines which + * signature of callback function will be used. + */ +typedef enum { + VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE = 0, /* virConnectNodeDeviceEventLifecycleCallback */ + +# ifdef VIR_ENUM_SENTINELS + VIR_NODE_DEVICE_EVENT_ID_LAST + /* + * NB: this enum value will increase over time as new events are + * added to the libvirt API. It reflects the last event ID supported + * by this version of the libvirt API. + */ +# endif +} virNodeDeviceEventID; + +/** + * virConnectNodeDeviceEventGenericCallback: + * @conn: the connection pointer + * @dev: the node device pointer + * @opaque: application specified data + * + * A generic node device event callback handler, for use with + * virConnectNodeDeviceEventRegisterAny(). Specific events usually + * have a customization with extra parameters, often with @opaque being + * passed in a different parameter position; use + * VIR_NODE_DEVICE_EVENT_CALLBACK() when registering an appropriate handler. + */ +typedef void (*virConnectNodeDeviceEventGenericCallback)(virConnectPtr conn, + virNodeDevicePtr dev, + void *opaque); + +/* Use VIR_NODE_DEVICE_EVENT_CALLBACK() to cast the 'cb' parameter */ +int virConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, /* optional, to filter */ + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb); + +int virConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID); + +/** + * virNodeDeviceEventLifecycleType: + * + * a virNodeDeviceEventLifecycleType is emitted during node device + * lifecycle events + */ +typedef enum { + VIR_NODE_DEVICE_EVENT_CREATED = 0, + VIR_NODE_DEVICE_EVENT_DELETED = 1, + +# ifdef VIR_ENUM_SENTINELS + VIR_NODE_DEVICE_EVENT_LAST +# endif +} virNodeDeviceEventLifecycleType; + +/** + * virConnectNodeDeviceEventLifecycleCallback: + * @conn: connection object + * @dev: node device on which the event occurred + * @event: The specific virNodeDeviceEventLifeCycleType which occurred + * @detail: contains some details on the reason of the event. + * @opaque: application specified data + * + * This callback is called when a node device lifecycle action is performed, + * like added, removed or changed. + * + * The callback signature to use when registering for an event of type + * VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE with + * virConnectNodeDeviceEventRegisterAny() + */ +typedef void (*virConnectNodeDeviceEventLifecycleCallback)(virConnectPtr conn, + virNodeDevicePtr dev, + int event, + int detail, + void *opaque); #endif /* __VIR_LIBVIRT_NODEDEV_H__ */ diff --git a/po/POTFILES.in b/po/POTFILES.in index a6b6c9c..25dbc84 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -70,6 +70,7 @@ src/libvirt-domain.c src/libvirt-host.c src/libvirt-lxc.c src/libvirt-network.c +src/libvirt-nodedev.c src/libvirt-nwfilter.c src/libvirt-qemu.c src/libvirt-secret.c diff --git a/src/datatypes.h b/src/datatypes.h index 996506b..2b6adb4 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -196,6 +196,19 @@ extern virClassPtr virAdmClientClass; } \ } while (0) +# define virCheckNodeDeviceGoto(obj, label) \ + do { \ + virNodeDevicePtr _dev= (obj); \ + if (!virObjectIsClass(_dev, virNodeDeviceClass) || \ + !virObjectIsClass(_dev->conn, virConnectClass)) { \ + virReportErrorHelper(VIR_FROM_NODEDEV, \ + VIR_ERR_INVALID_NODE_DEVICE, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + goto label; \ + } \ + } while (0) + # define virCheckSecretReturn(obj, retval) \ do { \ virSecretPtr _secret = (obj); \ diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h index e846612..5eae239 100644 --- a/src/driver-nodedev.h +++ b/src/driver-nodedev.h @@ -75,6 +75,18 @@ typedef virNodeDevicePtr typedef int (*virDrvNodeDeviceDestroy)(virNodeDevicePtr dev); +typedef int +(*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb); + +typedef int +(*virDrvConnectNodeDeviceEventDeregisterAny)(virConnectPtr conn, + int callbackID); + typedef struct _virNodeDeviceDriver virNodeDeviceDriver; @@ -92,6 +104,8 @@ struct _virNodeDeviceDriver { virDrvNodeNumOfDevices nodeNumOfDevices; virDrvNodeListDevices nodeListDevices; virDrvConnectListAllNodeDevices connectListAllNodeDevices; + virDrvConnectNodeDeviceEventRegisterAny connectNodeDeviceEventRegisterAny; + virDrvConnectNodeDeviceEventDeregisterAny connectNodeDeviceEventDeregisterAny; virDrvNodeDeviceLookupByName nodeDeviceLookupByName; virDrvNodeDeviceLookupSCSIHostByWWN nodeDeviceLookupSCSIHostByWWN; virDrvNodeDeviceGetXMLDesc nodeDeviceGetXMLDesc; diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index c1ca575..8658b71 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -753,3 +753,130 @@ virNodeDeviceDestroy(virNodeDevicePtr dev) virDispatchError(dev->conn); return -1; } + + +/** + * virConnectNodeDeviceEventRegisterAny: + * @conn: pointer to the connection + * @dev: pointer to the node device + * @eventID: the event type to receive + * @cb: callback to the function handling node device events + * @opaque: opaque data to pass on to the callback + * @freecb: optional function to deallocate opaque when not used anymore + * + * Adds a callback to receive notifications of arbitrary node device events + * occurring on a node device. This function requires that an event loop + * has been previously registered with virEventRegisterImpl() or + * virEventRegisterDefaultImpl(). + * + * If @dev is NULL, then events will be monitored for any node device. + * If @dev is non-NULL, then only the specific node device will be monitored. + * + * Most types of events have a callback providing a custom set of parameters + * for the event. When registering an event, it is thus necessary to use + * the VIR_NODE_DEVICE_EVENT_CALLBACK() macro to cast the + * supplied function pointer to match the signature of this method. + * + * The virNodeDevicePtr object handle passed into the callback upon delivery + * of an event is only valid for the duration of execution of the callback. + * If the callback wishes to keep the node device object after the callback + * returns, it shall take a reference to it, by calling virNodeDeviceRef(). + * The reference can be released once the object is no longer required + * by calling virNodeDeviceFree(). + * + * The return value from this method is a positive integer identifier + * for the callback. To unregister a callback, this callback ID should + * be passed to the virConnectNodeDeviceEventDeregisterAny() method. + * + * Returns a callback identifier on success, -1 on failure. + */ +int +virConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb) +{ + VIR_DEBUG("conn=%p, nodeDevice=%p, eventID=%d, cb=%p, opaque=%p, freecb=%p", + conn, dev, eventID, cb, opaque, freecb); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + if (dev) { + virCheckNodeDeviceGoto(dev, error); + if (dev->conn != conn) { + virReportInvalidArg(dev, + _("node device '%s' in %s must match connection"), + dev->name, __FUNCTION__); + goto error; + } + } + virCheckNonNullArgGoto(cb, error); + virCheckNonNegativeArgGoto(eventID, error); + + if (eventID >= VIR_NODE_DEVICE_EVENT_ID_LAST) { + virReportInvalidArg(eventID, + _("eventID in %s must be less than %d"), + __FUNCTION__, VIR_STORAGE_POOL_EVENT_ID_LAST); + goto error; + } + + if (conn->nodeDeviceDriver && + conn->nodeDeviceDriver->connectNodeDeviceEventRegisterAny) { + int ret; + ret = conn->nodeDeviceDriver->connectNodeDeviceEventRegisterAny(conn, + dev, + eventID, + cb, + opaque, + freecb); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} + + +/** + * virConnectNodeDeviceEventDeregisterAny: + * @conn: pointer to the connection + * @callbackID: the callback identifier + * + * Removes an event callback. The callbackID parameter should be the + * value obtained from a previous virConnectNodeDeviceEventRegisterAny() method. + * + * Returns 0 on success, -1 on failure. + */ +int +virConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + VIR_DEBUG("conn=%p, callbackID=%d", conn, callbackID); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckNonNegativeArgGoto(callbackID, error); + + if (conn->nodeDeviceDriver && + conn->nodeDeviceDriver->connectNodeDeviceEventDeregisterAny) { + int ret; + ret = conn->nodeDeviceDriver->connectNodeDeviceEventDeregisterAny(conn, + callbackID); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index b6d2dfd..9722b93 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -740,4 +740,10 @@ LIBVIRT_2.0.0 { virDomainSetGuestVcpus; } LIBVIRT_1.3.3; +LIBVIRT_2.1.0 { + global: + virConnectNodeDeviceEventRegisterAny; + virConnectNodeDeviceEventDeregisterAny; +} LIBVIRT_2.0.0; + # .... define new API here using predicted next version number .... -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
Node device lifecycle event API entry points for registering and deregistering node deivce events, as well as types of events associated with node devices.
* device
These entry points will be used for implementing asynchronous lifecycle events.
These three lines are a bit long. Maybe split them to be 70-75 chars: http://stackoverflow.com/questions/2290016/git-commit-messages-50-72-formatt...
Node device API: virConnectNodeDeviceEventRegisterAny virConnectNodeDeviceEventDeregisterAny virNodeDeviceEventLifecycleType which has events CREATED and DELETED
So one major question here is that we are only added CREATED and DELETED events, but devices can also be 'updated', as in their config can change. For example when a cdrom device has media ejected or inserted, udev fires an event, and we update the cached device config which is reflected in the device XML. Adding an UPDATED event or similar isn't hard, but it's not technically a lifecycle. So if we add it, it should be a separate callback? Similar to what was eventually done for the pool refresh event. CCing danpb for his thoughts
--- include/libvirt/libvirt-nodedev.h | 90 +++++++++++++++++++++++++++ po/POTFILES.in | 1 + src/datatypes.h | 13 ++++ src/driver-nodedev.h | 14 +++++ src/libvirt-nodedev.c | 127 ++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++ 6 files changed, 251 insertions(+)
diff --git a/include/libvirt/libvirt-nodedev.h b/include/libvirt/libvirt-nodedev.h index cd0b548..8a35470 100644 --- a/include/libvirt/libvirt-nodedev.h +++ b/include/libvirt/libvirt-nodedev.h @@ -121,5 +121,95 @@ virNodeDevicePtr virNodeDeviceCreateXML (virConnectPtr conn,
int virNodeDeviceDestroy (virNodeDevicePtr dev);
+/** + * VIR_NODE_DEVICE_EVENT_CALLBACK: + * + * Used to cast the event specific callback into the generic one + * for use for virConnectNodeDeviceEventRegisterAny() + */ +# define VIR_NODE_DEVICE_EVENT_CALLBACK(cb)((virConnectNodeDeviceEventGenericCallback)(cb)) + +/** + * virNodeDeviceEventID: + * + * An enumeration of supported eventId parameters for + * virConnectNodeDeviceEventRegisterAny(). Each event id determines which + * signature of callback function will be used. + */ +typedef enum { + VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE = 0, /* virConnectNodeDeviceEventLifecycleCallback */ + +# ifdef VIR_ENUM_SENTINELS + VIR_NODE_DEVICE_EVENT_ID_LAST + /* + * NB: this enum value will increase over time as new events are + * added to the libvirt API. It reflects the last event ID supported + * by this version of the libvirt API. + */ +# endif +} virNodeDeviceEventID; + +/** + * virConnectNodeDeviceEventGenericCallback: + * @conn: the connection pointer + * @dev: the node device pointer + * @opaque: application specified data + * + * A generic node device event callback handler, for use with + * virConnectNodeDeviceEventRegisterAny(). Specific events usually + * have a customization with extra parameters, often with @opaque being + * passed in a different parameter position; use + * VIR_NODE_DEVICE_EVENT_CALLBACK() when registering an appropriate handler. + */ +typedef void (*virConnectNodeDeviceEventGenericCallback)(virConnectPtr conn, + virNodeDevicePtr dev, + void *opaque); + +/* Use VIR_NODE_DEVICE_EVENT_CALLBACK() to cast the 'cb' parameter */ +int virConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, /* optional, to filter */ + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb); + +int virConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID); + +/** + * virNodeDeviceEventLifecycleType: + * + * a virNodeDeviceEventLifecycleType is emitted during node device + * lifecycle events + */ +typedef enum { + VIR_NODE_DEVICE_EVENT_CREATED = 0, + VIR_NODE_DEVICE_EVENT_DELETED = 1, + +# ifdef VIR_ENUM_SENTINELS + VIR_NODE_DEVICE_EVENT_LAST +# endif +} virNodeDeviceEventLifecycleType; + +/** + * virConnectNodeDeviceEventLifecycleCallback: + * @conn: connection object + * @dev: node device on which the event occurred + * @event: The specific virNodeDeviceEventLifeCycleType which occurred + * @detail: contains some details on the reason of the event. + * @opaque: application specified data + * + * This callback is called when a node device lifecycle action is performed, + * like added, removed or changed. + *
This mentions 'changed' but we don't have an explicit event for that yet.
+ * The callback signature to use when registering for an event of type + * VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE with + * virConnectNodeDeviceEventRegisterAny() + */ +typedef void (*virConnectNodeDeviceEventLifecycleCallback)(virConnectPtr conn, + virNodeDevicePtr dev, + int event, + int detail, + void *opaque);
#endif /* __VIR_LIBVIRT_NODEDEV_H__ */ diff --git a/po/POTFILES.in b/po/POTFILES.in index a6b6c9c..25dbc84 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -70,6 +70,7 @@ src/libvirt-domain.c src/libvirt-host.c src/libvirt-lxc.c src/libvirt-network.c +src/libvirt-nodedev.c src/libvirt-nwfilter.c src/libvirt-qemu.c src/libvirt-secret.c diff --git a/src/datatypes.h b/src/datatypes.h index 996506b..2b6adb4 100644 --- a/src/datatypes.h +++ b/src/datatypes.h @@ -196,6 +196,19 @@ extern virClassPtr virAdmClientClass; } \ } while (0)
+# define virCheckNodeDeviceGoto(obj, label) \ + do { \ + virNodeDevicePtr _dev= (obj); \ + if (!virObjectIsClass(_dev, virNodeDeviceClass) || \ + !virObjectIsClass(_dev->conn, virConnectClass)) { \ + virReportErrorHelper(VIR_FROM_NODEDEV, \ + VIR_ERR_INVALID_NODE_DEVICE, \ + __FILE__, __FUNCTION__, __LINE__, \ + __FUNCTION__); \ + goto label; \ + } \ + } while (0) + # define virCheckSecretReturn(obj, retval) \ do { \ virSecretPtr _secret = (obj); \ diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h index e846612..5eae239 100644 --- a/src/driver-nodedev.h +++ b/src/driver-nodedev.h @@ -75,6 +75,18 @@ typedef virNodeDevicePtr typedef int (*virDrvNodeDeviceDestroy)(virNodeDevicePtr dev);
+typedef int +(*virDrvConnectNodeDeviceEventRegisterAny)(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb); + +typedef int +(*virDrvConnectNodeDeviceEventDeregisterAny)(virConnectPtr conn, + int callbackID); +
typedef struct _virNodeDeviceDriver virNodeDeviceDriver; @@ -92,6 +104,8 @@ struct _virNodeDeviceDriver { virDrvNodeNumOfDevices nodeNumOfDevices; virDrvNodeListDevices nodeListDevices; virDrvConnectListAllNodeDevices connectListAllNodeDevices; + virDrvConnectNodeDeviceEventRegisterAny connectNodeDeviceEventRegisterAny; + virDrvConnectNodeDeviceEventDeregisterAny connectNodeDeviceEventDeregisterAny; virDrvNodeDeviceLookupByName nodeDeviceLookupByName; virDrvNodeDeviceLookupSCSIHostByWWN nodeDeviceLookupSCSIHostByWWN; virDrvNodeDeviceGetXMLDesc nodeDeviceGetXMLDesc; diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c index c1ca575..8658b71 100644 --- a/src/libvirt-nodedev.c +++ b/src/libvirt-nodedev.c @@ -753,3 +753,130 @@ virNodeDeviceDestroy(virNodeDevicePtr dev) virDispatchError(dev->conn); return -1; } + + +/** + * virConnectNodeDeviceEventRegisterAny: + * @conn: pointer to the connection + * @dev: pointer to the node device + * @eventID: the event type to receive + * @cb: callback to the function handling node device events + * @opaque: opaque data to pass on to the callback + * @freecb: optional function to deallocate opaque when not used anymore + * + * Adds a callback to receive notifications of arbitrary node device events + * occurring on a node device. This function requires that an event loop + * has been previously registered with virEventRegisterImpl() or + * virEventRegisterDefaultImpl(). + * + * If @dev is NULL, then events will be monitored for any node device. + * If @dev is non-NULL, then only the specific node device will be monitored. + * + * Most types of events have a callback providing a custom set of parameters + * for the event. When registering an event, it is thus necessary to use + * the VIR_NODE_DEVICE_EVENT_CALLBACK() macro to cast the + * supplied function pointer to match the signature of this method. + * + * The virNodeDevicePtr object handle passed into the callback upon delivery + * of an event is only valid for the duration of execution of the callback. + * If the callback wishes to keep the node device object after the callback + * returns, it shall take a reference to it, by calling virNodeDeviceRef(). + * The reference can be released once the object is no longer required + * by calling virNodeDeviceFree(). + * + * The return value from this method is a positive integer identifier + * for the callback. To unregister a callback, this callback ID should + * be passed to the virConnectNodeDeviceEventDeregisterAny() method. + * + * Returns a callback identifier on success, -1 on failure. + */ +int +virConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb) +{ + VIR_DEBUG("conn=%p, nodeDevice=%p, eventID=%d, cb=%p, opaque=%p, freecb=%p", + conn, dev, eventID, cb, opaque, freecb); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + if (dev) { + virCheckNodeDeviceGoto(dev, error); + if (dev->conn != conn) { + virReportInvalidArg(dev, + _("node device '%s' in %s must match connection"), + dev->name, __FUNCTION__); + goto error; + } + } + virCheckNonNullArgGoto(cb, error); + virCheckNonNegativeArgGoto(eventID, error); + + if (eventID >= VIR_NODE_DEVICE_EVENT_ID_LAST) { + virReportInvalidArg(eventID, + _("eventID in %s must be less than %d"), + __FUNCTION__, VIR_STORAGE_POOL_EVENT_ID_LAST);
Should be VIR_NODE_DEVICE_EVENT_ID_LAST
+ goto error; + } + + if (conn->nodeDeviceDriver && + conn->nodeDeviceDriver->connectNodeDeviceEventRegisterAny) { + int ret; + ret = conn->nodeDeviceDriver->connectNodeDeviceEventRegisterAny(conn, + dev, + eventID, + cb, + opaque, + freecb); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} + + +/** + * virConnectNodeDeviceEventDeregisterAny: + * @conn: pointer to the connection + * @callbackID: the callback identifier + * + * Removes an event callback. The callbackID parameter should be the + * value obtained from a previous virConnectNodeDeviceEventRegisterAny() method. + * + * Returns 0 on success, -1 on failure. + */ +int +virConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + VIR_DEBUG("conn=%p, callbackID=%d", conn, callbackID); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + virCheckNonNegativeArgGoto(callbackID, error); + + if (conn->nodeDeviceDriver && + conn->nodeDeviceDriver->connectNodeDeviceEventDeregisterAny) { + int ret; + ret = conn->nodeDeviceDriver->connectNodeDeviceEventDeregisterAny(conn, + callbackID); + if (ret < 0) + goto error; + return ret; + } + + virReportUnsupportedError(); + error: + virDispatchError(conn); + return -1; +} diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index b6d2dfd..9722b93 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -740,4 +740,10 @@ LIBVIRT_2.0.0 { virDomainSetGuestVcpus; } LIBVIRT_1.3.3;
+LIBVIRT_2.1.0 { + global: + virConnectNodeDeviceEventRegisterAny; + virConnectNodeDeviceEventDeregisterAny; +} LIBVIRT_2.0.0; + # .... define new API here using predicted next version number ....
Since I don't think this will make it into 2.1.0 which freezes tomorrow, this version should be adjusted to 2.2.0 Thanks, Cole

On Tue, Jul 26, 2016 at 01:28:34PM -0400, Cole Robinson wrote:
On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
Node device lifecycle event API entry points for registering and deregistering node deivce events, as well as types of events associated with node devices.
* device
These entry points will be used for implementing asynchronous lifecycle events.
These three lines are a bit long. Maybe split them to be 70-75 chars:
http://stackoverflow.com/questions/2290016/git-commit-messages-50-72-formatt...
Node device API: virConnectNodeDeviceEventRegisterAny virConnectNodeDeviceEventDeregisterAny virNodeDeviceEventLifecycleType which has events CREATED and DELETED
So one major question here is that we are only added CREATED and DELETED events, but devices can also be 'updated', as in their config can change. For example when a cdrom device has media ejected or inserted, udev fires an event, and we update the cached device config which is reflected in the device XML.
Adding an UPDATED event or similar isn't hard, but it's not technically a lifecycle. So if we add it, it should be a separate callback? Similar to what was eventually done for the pool refresh event. CCing danpb for his thoughts
Yep, I'd recommend a separate event callback for that. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|

On 07/26/2016 01:32 PM, Daniel P. Berrange wrote:
On Tue, Jul 26, 2016 at 01:28:34PM -0400, Cole Robinson wrote:
On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
Node device lifecycle event API entry points for registering and deregistering node deivce events, as well as types of events associated with node devices.
* device
These entry points will be used for implementing asynchronous lifecycle events.
These three lines are a bit long. Maybe split them to be 70-75 chars:
http://stackoverflow.com/questions/2290016/git-commit-messages-50-72-formatt...
Node device API: virConnectNodeDeviceEventRegisterAny virConnectNodeDeviceEventDeregisterAny virNodeDeviceEventLifecycleType which has events CREATED and DELETED
So one major question here is that we are only added CREATED and DELETED events, but devices can also be 'updated', as in their config can change. For example when a cdrom device has media ejected or inserted, udev fires an event, and we update the cached device config which is reflected in the device XML.
Adding an UPDATED event or similar isn't hard, but it's not technically a lifecycle. So if we add it, it should be a separate callback? Similar to what was eventually done for the pool refresh event. CCing danpb for his thoughts
Yep, I'd recommend a separate event callback for that.
Sounds good. Jovanka, I suggest just sticking a patch at the end of the series which adds all the UPDATED handling in one shot, rather than break it into a bunch of patches. Similar to what Dan did for the REFRESHED event: http://www.redhat.com/archives/libvir-list/2016-June/msg01897.html Note there's not a clear way to unit test this, since the test driver doesn't really have any plumbing for it, so those bits can be skipped. Thanks, Cole

Add node device event handling infrastructure to node_device_event.[ch] --- src/Makefile.am | 5 + src/conf/node_device_event.c | 234 +++++++++++++++++++++++++++++++++++++++++++ src/conf/node_device_event.h | 59 +++++++++++ src/libvirt_private.syms | 5 + 4 files changed, 303 insertions(+) create mode 100644 src/conf/node_device_event.c create mode 100644 src/conf/node_device_event.h diff --git a/src/Makefile.am b/src/Makefile.am index 78c493c..e2a2128 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -348,6 +348,9 @@ NETWORK_EVENT_SOURCES = \ STORAGE_EVENT_SOURCES = \ conf/storage_event.c conf/storage_event.h +NODE_DEVICE_EVENT_SOURCES = \ + conf/node_device_event.c conf/node_device_event.h + # Network driver generic impl APIs NETWORK_CONF_SOURCES = \ conf/network_conf.c conf/network_conf.h \ @@ -399,6 +402,7 @@ CONF_SOURCES = \ $(DOMAIN_EVENT_SOURCES) \ $(NETWORK_EVENT_SOURCES) \ $(STORAGE_EVENT_SOURCES) \ + $(NODE_DEVICE_EVENT_SOURCES) \ $(NETWORK_CONF_SOURCES) \ $(NWFILTER_CONF_SOURCES) \ $(NODE_DEVICE_CONF_SOURCES) \ @@ -2369,6 +2373,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ conf/network_event.c \ conf/object_event.c \ conf/storage_event.c \ + conf/node_device_event.c \ rpc/virnetsocket.c \ rpc/virnetsocket.h \ rpc/virnetmessage.h \ diff --git a/src/conf/node_device_event.c b/src/conf/node_device_event.c new file mode 100644 index 0000000..61bc912 --- /dev/null +++ b/src/conf/node_device_event.c @@ -0,0 +1,234 @@ +/* + * node_device_event.c: node device event queue processing helpers + * + * Copyright (C) 2010-2014 Red Hat, Inc. + * Copyright (C) 2008 VirtualIron + * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "node_device_event.h" +#include "object_event.h" +#include "object_event_private.h" +#include "datatypes.h" +#include "virlog.h" + +VIR_LOG_INIT("conf.node_device_event"); + +struct _virNodeDeviceEvent { + virObjectEvent parent; + + /* Unused attribute to allow for subclass creation */ + bool dummy; +}; +typedef struct _virNodeDeviceEvent virNodeDeviceEvent; +typedef virNodeDeviceEvent *virNodeDeviceEventPtr; + +struct _virNodeDeviceEventLifecycle { + virNodeDeviceEvent parent; + + int type; + int detail; +}; +typedef struct _virNodeDeviceEventLifecycle virNodeDeviceEventLifecycle; +typedef virNodeDeviceEventLifecycle *virNodeDeviceEventLifecyclePtr; + +static virClassPtr virNodeDeviceEventClass; +static virClassPtr virNodeDeviceEventLifecycleClass; +static void virNodeDeviceEventDispose(void *obj); +static void virNodeDeviceEventLifecycleDispose(void *obj); + +static int +virNodeDeviceEventsOnceInit(void) +{ + if (!(virNodeDeviceEventClass = + virClassNew(virClassForObjectEvent(), + "virNodeDeviceEvent", + sizeof(virNodeDeviceEvent), + virNodeDeviceEventDispose))) + return -1; + if (!(virNodeDeviceEventLifecycleClass = + virClassNew(virNodeDeviceEventClass, + "virNodeDeviceEventLifecycle", + sizeof(virNodeDeviceEventLifecycle), + virNodeDeviceEventLifecycleDispose))) + return -1; + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virNodeDeviceEvents) + +static void +virNodeDeviceEventDispose(void *obj) +{ + virNodeDeviceEventPtr event = obj; + VIR_DEBUG("obj=%p", event); +} + + +static void +virNodeDeviceEventLifecycleDispose(void *obj) +{ + virNodeDeviceEventLifecyclePtr event = obj; + VIR_DEBUG("obj=%p", event); +} + + +static void +virNodeDeviceEventDispatchDefaultFunc(virConnectPtr conn, + virObjectEventPtr event, + virConnectObjectEventGenericCallback cb, + void *cbopaque) +{ + virNodeDevicePtr dev = virGetNodeDevice(conn, + event->meta.name); + + if (!dev) + return; + + switch ((virNodeDeviceEventID)event->eventID) { + case VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE: + { + virNodeDeviceEventLifecyclePtr nodeDeviceLifecycleEvent; + + nodeDeviceLifecycleEvent = (virNodeDeviceEventLifecyclePtr)event; + ((virConnectNodeDeviceEventLifecycleCallback)cb)(conn, dev, + nodeDeviceLifecycleEvent->type, + nodeDeviceLifecycleEvent->detail, + cbopaque); + goto cleanup; + } + + case VIR_NODE_DEVICE_EVENT_ID_LAST: + break; + } + VIR_WARN("Unexpected event ID %d", event->eventID); + + cleanup: + virObjectUnref(dev); +} + + +/** + * virNodeDeviceEventStateRegisterID: + * @conn: connection to associate with callback + * @state: object event state + * @dev: node device to filter on or NULL for all node devices + * @eventID: ID of the event type to register for + * @cb: function to invoke when event occurs + * @opaque: data blob to pass to @callback + * @freecb: callback to free @opaque + * @callbackID: filled with callback ID + * + * Register the function @cb with connection @conn, from @state, for + * events of type @eventID, and return the registration handle in + * @callbackID. + * + * Returns: the number of callbacks now registered, or -1 on error + */ +int +virNodeDeviceEventStateRegisterID(virConnectPtr conn, + virObjectEventStatePtr state, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) +{ + if (virNodeDeviceEventsInitialize() < 0) + return -1; + + return virObjectEventStateRegisterID(conn, state, dev ? dev->name : NULL, + NULL, NULL, + virNodeDeviceEventClass, eventID, + VIR_OBJECT_EVENT_CALLBACK(cb), + opaque, freecb, + false, callbackID, false); +} + + +/** + * virNodeDeviceEventStateRegisterClient: + * @conn: connection to associate with callback + * @state: object event state + * @dev: node device to filter on or NULL for all node devices + * @eventID: ID of the event type to register for + * @cb: function to invoke when event occurs + * @opaque: data blob to pass to @callback + * @freecb: callback to free @opaque + * @callbackID: filled with callback ID + * + * Register the function @cb with connection @conn, from @state, for + * events of type @eventID, and return the registration handle in + * @callbackID. This version is intended for use on the client side + * of RPC. + * + * Returns: the number of callbacks now registered, or -1 on error + */ +int +virNodeDeviceEventStateRegisterClient(virConnectPtr conn, + virObjectEventStatePtr state, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) +{ + if (virNodeDeviceEventsInitialize() < 0) + return -1; + + return virObjectEventStateRegisterID(conn, state, dev ? dev->name : NULL, + NULL, NULL, + virNodeDeviceEventClass, eventID, + VIR_OBJECT_EVENT_CALLBACK(cb), + opaque, freecb, + false, callbackID, true); +} + + +/** + * virNodeDeviceEventLifecycleNew: + * @name: name of the node device object the event describes + * @type: type of lifecycle event + * @detail: more details about @type + * + * Create a new node device lifecycle event. + */ +virObjectEventPtr +virNodeDeviceEventLifecycleNew(const char *name, + int type, + int detail) +{ + virNodeDeviceEventLifecyclePtr event; + + if (virNodeDeviceEventsInitialize() < 0) + return NULL; + + if (!(event = virObjectEventNew(virNodeDeviceEventLifecycleClass, + virNodeDeviceEventDispatchDefaultFunc, + VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE, + 0, name, NULL, name))) + return NULL; + + event->type = type; + event->detail = detail; + + return (virObjectEventPtr)event; +} diff --git a/src/conf/node_device_event.h b/src/conf/node_device_event.h new file mode 100644 index 0000000..9c99633 --- /dev/null +++ b/src/conf/node_device_event.h @@ -0,0 +1,59 @@ +/* + * node_device_event.h: node device event queue processing helpers + * + * Copyright (C) 2010-2014 Red Hat, Inc. + * Copyright (C) 2008 VirtualIron + * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include "internal.h" +#include "object_event.h" +#include "object_event_private.h" + +#ifndef __NODE_DEVICE_EVENT_H__ +# define __NODE_DEVICE_EVENT_H__ + +int +virNodeDeviceEventStateRegisterID(virConnectPtr conn, + virObjectEventStatePtr state, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) + ATTRIBUTE_NONNULL(8); + +int +virNodeDeviceEventStateRegisterClient(virConnectPtr conn, + virObjectEventStatePtr state, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) + ATTRIBUTE_NONNULL(8); + +virObjectEventPtr +virNodeDeviceEventLifecycleNew(const char *name, + int type, + int detail); + +#endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cd3cc64..0bb1d0b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -677,6 +677,11 @@ virNodeDeviceObjRemove; virNodeDeviceObjUnlock; +# conf/node_device_event.h +virNodeDeviceEventLifecycleNew; +virNodeDeviceEventStateRegisterID; + + # conf/numa_conf.h virDomainNumaCheckABIStability; virDomainNumaEquals; -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
Add node device event handling infrastructure to node_device_event.[ch] --- src/Makefile.am | 5 + src/conf/node_device_event.c | 234 +++++++++++++++++++++++++++++++++++++++++++ src/conf/node_device_event.h | 59 +++++++++++ src/libvirt_private.syms | 5 + 4 files changed, 303 insertions(+) create mode 100644 src/conf/node_device_event.c create mode 100644 src/conf/node_device_event.h
Looks good to me - Cole

nodedev events don't have a uuid value and will pass in NULL --- src/conf/object_event.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/object_event.c b/src/conf/object_event.c index cb984ff..bbef7f6 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -659,7 +659,8 @@ virObjectEventNew(virClassPtr klass, return NULL; } event->meta.id = id; - memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN); + if (uuid) + memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN); VIR_DEBUG("obj=%p", event); return event; -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
nodedev events don't have a uuid value and will pass in NULL --- src/conf/object_event.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/object_event.c b/src/conf/object_event.c index cb984ff..bbef7f6 100644 --- a/src/conf/object_event.c +++ b/src/conf/object_event.c @@ -659,7 +659,8 @@ virObjectEventNew(virClassPtr klass, return NULL; } event->meta.id = id; - memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN); + if (uuid) + memcpy(event->meta.uuid, uuid, VIR_UUID_BUFLEN);
VIR_DEBUG("obj=%p", event); return event;
I'll just push this now, since it's self contained and trivial. I reworked the commit message a bit to make it clear that this is for future nodedev work Thanks, Cole

Also includes unittests for node device lifecycle events API --- src/test/test_driver.c | 49 +++++++++++++++++++++++++++++++++ tests/objecteventtest.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 672c163..8233d74 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -51,6 +51,7 @@ #include "storage_conf.h" #include "storage_event.h" #include "node_device_conf.h" +#include "node_device_event.h" #include "virxml.h" #include "virthread.h" #include "virlog.h" @@ -504,6 +505,7 @@ static const char *defaultPoolSourcesNetFSXML = " </source>\n" "</sources>\n"; + static const unsigned long long defaultPoolCap = (100 * 1024 * 1024 * 1024ull); static const unsigned long long defaultPoolAlloc; @@ -5427,6 +5429,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, int parent_host = -1; virNodeDevicePtr dev = NULL; virNodeDevCapsDefPtr caps; + virObjectEventPtr event = NULL; virCheckFlags(0, NULL); @@ -5470,11 +5473,16 @@ testNodeDeviceCreateXML(virConnectPtr conn, goto cleanup; virNodeDeviceObjUnlock(obj); + event = virNodeDeviceEventLifecycleNew(def->name, + VIR_NODE_DEVICE_EVENT_CREATED, + 0); + dev = virGetNodeDevice(conn, def->name); def = NULL; cleanup: testDriverUnlock(driver); virNodeDeviceDefFree(def); + testObjectEventQueue(driver, event); VIR_FREE(wwnn); VIR_FREE(wwpn); return dev; @@ -5488,6 +5496,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) virNodeDeviceObjPtr obj = NULL; char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; int parent_host = -1; + virObjectEventPtr event = NULL; testDriverLock(driver); obj = virNodeDeviceFindByName(&driver->devs, dev->name); @@ -5521,12 +5530,17 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) goto out; } + event = virNodeDeviceEventLifecycleNew(dev->name, + VIR_NODE_DEVICE_EVENT_DELETED, + 0); + virNodeDeviceObjLock(obj); virNodeDeviceObjRemove(&driver->devs, obj); out: if (obj) virNodeDeviceObjUnlock(obj); + testObjectEventQueue(driver, event); VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn); @@ -5667,6 +5681,39 @@ testConnectStoragePoolEventDeregisterAny(virConnectPtr conn, return ret; } +static int +testConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + testDriverPtr driver = conn->privateData; + int ret; + + if (virNodeDeviceEventStateRegisterID(conn, driver->eventState, + dev, eventID, callback, + opaque, freecb, &ret) < 0) + ret = -1; + + return ret; +} + +static int +testConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + testDriverPtr driver = conn->privateData; + int ret = 0; + + if (virObjectEventStateDeregisterID(conn, driver->eventState, + callbackID) < 0) + ret = -1; + + return ret; +} + static int testConnectListAllDomains(virConnectPtr conn, virDomainPtr **domains, unsigned int flags) @@ -6809,6 +6856,8 @@ static virStorageDriver testStorageDriver = { }; static virNodeDeviceDriver testNodeDeviceDriver = { + .connectNodeDeviceEventRegisterAny = testConnectNodeDeviceEventRegisterAny, /* 2.1.0 */ + .connectNodeDeviceEventDeregisterAny = testConnectNodeDeviceEventDeregisterAny, /* 2.1.0 */ .nodeNumOfDevices = testNodeNumOfDevices, /* 0.7.2 */ .nodeListDevices = testNodeListDevices, /* 0.7.2 */ .nodeDeviceLookupByName = testNodeDeviceLookupByName, /* 0.7.2 */ diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c index 5e8087b..d25a9e2 100644 --- a/tests/objecteventtest.c +++ b/tests/objecteventtest.c @@ -61,12 +61,25 @@ static const char storagePoolDef[] = " </target>\n" "</pool>\n"; +static const char nodeDeviceDef[] = +"<device>\n" +" <parent>test-scsi-host-vport</parent>\n" +" <capability type='scsi_host'>\n" +" <capability type='fc_host'>\n" +" <wwpn>1111222233334444</wwpn>\n" +" <wwnn>5555666677778888</wwnn>\n" +" </capability>\n" +" </capability>\n" +"</device>\n"; + typedef struct { int startEvents; int stopEvents; int defineEvents; int undefineEvents; int unexpectedEvents; + int createdEvents; + int deletedEvents; } lifecycleEventCounter; static void @@ -77,12 +90,15 @@ lifecycleEventCounter_reset(lifecycleEventCounter *counter) counter->defineEvents = 0; counter->undefineEvents = 0; counter->unexpectedEvents = 0; + counter->createdEvents = 0; + counter->deletedEvents = 0; } typedef struct { virConnectPtr conn; virNetworkPtr net; virStoragePoolPtr pool; + virNodeDevicePtr dev; } objecteventTest; @@ -163,6 +179,21 @@ storagePoolRefreshCb(virConnectPtr conn ATTRIBUTE_UNUSED, (*counter)++; } +static void +nodeDeviceLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeDevicePtr dev ATTRIBUTE_UNUSED, + int event, + int detail ATTRIBUTE_UNUSED, + void* opaque) +{ + lifecycleEventCounter *counter = opaque; + + if (event == VIR_NODE_DEVICE_EVENT_CREATED) + counter->createdEvents++; + else if (event == VIR_NODE_DEVICE_EVENT_DELETED) + counter->deletedEvents++; +} + static int testDomainCreateXMLOld(const void *data) { @@ -691,6 +722,42 @@ testStoragePoolStartStopEvent(const void *data) return ret; } +static int +testNodeDeviceCreateXML(const void *data) +{ + const objecteventTest *test = data; + lifecycleEventCounter counter; + virNodeDevicePtr dev; + int id; + int ret = 0; + + lifecycleEventCounter_reset(&counter); + + id = virConnectNodeDeviceEventRegisterAny(test->conn, NULL, + VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE, + VIR_NODE_DEVICE_EVENT_CALLBACK(&nodeDeviceLifecycleCb), + &counter, NULL); + dev = virNodeDeviceCreateXML(test->conn, nodeDeviceDef, 0); + virNodeDeviceDestroy(dev); + + if (!dev || virEventRunDefaultImpl() < 0) { + ret = -1; + goto cleanup; + } + + if (counter.createdEvents != 1 || counter.deletedEvents != 1 || + counter.unexpectedEvents > 0) { + ret = -1; + goto cleanup; + } + + cleanup: + virConnectNodeDeviceEventDeregisterAny(test->conn, id); + if (dev) + virNodeDeviceFree(dev); + return ret; +} + static void timeout(int id ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -765,6 +832,11 @@ mymain(void) testStoragePoolStartStopEvent, &test) < 0) ret = EXIT_FAILURE; + /* Node device event tests */ + if (virTestRun("Node device createXML add event ", + testNodeDeviceCreateXML, &test) < 0) + ret = EXIT_FAILURE; + /* Cleanup */ if (test.pool) { virStoragePoolUndefine(test.pool); -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
Also includes unittests for node device lifecycle events API --- src/test/test_driver.c | 49 +++++++++++++++++++++++++++++++++ tests/objecteventtest.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+)
diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 672c163..8233d74 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -51,6 +51,7 @@ #include "storage_conf.h" #include "storage_event.h" #include "node_device_conf.h" +#include "node_device_event.h" #include "virxml.h" #include "virthread.h" #include "virlog.h" @@ -504,6 +505,7 @@ static const char *defaultPoolSourcesNetFSXML = " </source>\n" "</sources>\n";
+
Stray whitespace change, please drop this
static const unsigned long long defaultPoolCap = (100 * 1024 * 1024 * 1024ull); static const unsigned long long defaultPoolAlloc;
@@ -5427,6 +5429,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, int parent_host = -1; virNodeDevicePtr dev = NULL; virNodeDevCapsDefPtr caps; + virObjectEventPtr event = NULL;
virCheckFlags(0, NULL);
@@ -5470,11 +5473,16 @@ testNodeDeviceCreateXML(virConnectPtr conn, goto cleanup; virNodeDeviceObjUnlock(obj);
+ event = virNodeDeviceEventLifecycleNew(def->name, + VIR_NODE_DEVICE_EVENT_CREATED, + 0); + dev = virGetNodeDevice(conn, def->name); def = NULL; cleanup: testDriverUnlock(driver); virNodeDeviceDefFree(def); + testObjectEventQueue(driver, event); VIR_FREE(wwnn); VIR_FREE(wwpn); return dev; @@ -5488,6 +5496,7 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) virNodeDeviceObjPtr obj = NULL; char *parent_name = NULL, *wwnn = NULL, *wwpn = NULL; int parent_host = -1; + virObjectEventPtr event = NULL;
testDriverLock(driver); obj = virNodeDeviceFindByName(&driver->devs, dev->name); @@ -5521,12 +5530,17 @@ testNodeDeviceDestroy(virNodeDevicePtr dev) goto out; }
+ event = virNodeDeviceEventLifecycleNew(dev->name, + VIR_NODE_DEVICE_EVENT_DELETED, + 0); + virNodeDeviceObjLock(obj); virNodeDeviceObjRemove(&driver->devs, obj);
out: if (obj) virNodeDeviceObjUnlock(obj); + testObjectEventQueue(driver, event); VIR_FREE(parent_name); VIR_FREE(wwnn); VIR_FREE(wwpn); @@ -5667,6 +5681,39 @@ testConnectStoragePoolEventDeregisterAny(virConnectPtr conn, return ret; }
+static int +testConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + testDriverPtr driver = conn->privateData; + int ret; + + if (virNodeDeviceEventStateRegisterID(conn, driver->eventState, + dev, eventID, callback, + opaque, freecb, &ret) < 0) + ret = -1; + + return ret; +} + +static int +testConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + testDriverPtr driver = conn->privateData; + int ret = 0; + + if (virObjectEventStateDeregisterID(conn, driver->eventState, + callbackID) < 0) + ret = -1; + + return ret; +} + static int testConnectListAllDomains(virConnectPtr conn, virDomainPtr **domains, unsigned int flags) @@ -6809,6 +6856,8 @@ static virStorageDriver testStorageDriver = { };
static virNodeDeviceDriver testNodeDeviceDriver = { + .connectNodeDeviceEventRegisterAny = testConnectNodeDeviceEventRegisterAny, /* 2.1.0 */ + .connectNodeDeviceEventDeregisterAny = testConnectNodeDeviceEventDeregisterAny, /* 2.1.0 */ .nodeNumOfDevices = testNodeNumOfDevices, /* 0.7.2 */ .nodeListDevices = testNodeListDevices, /* 0.7.2 */ .nodeDeviceLookupByName = testNodeDeviceLookupByName, /* 0.7.2 */
These versions will need to be updated to 2.2.0
diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c index 5e8087b..d25a9e2 100644 --- a/tests/objecteventtest.c +++ b/tests/objecteventtest.c @@ -61,12 +61,25 @@ static const char storagePoolDef[] = " </target>\n" "</pool>\n";
+static const char nodeDeviceDef[] = +"<device>\n" +" <parent>test-scsi-host-vport</parent>\n" +" <capability type='scsi_host'>\n" +" <capability type='fc_host'>\n" +" <wwpn>1111222233334444</wwpn>\n" +" <wwnn>5555666677778888</wwnn>\n" +" </capability>\n" +" </capability>\n" +"</device>\n"; + typedef struct { int startEvents; int stopEvents; int defineEvents; int undefineEvents; int unexpectedEvents; + int createdEvents; + int deletedEvents; } lifecycleEventCounter;
static void @@ -77,12 +90,15 @@ lifecycleEventCounter_reset(lifecycleEventCounter *counter) counter->defineEvents = 0; counter->undefineEvents = 0; counter->unexpectedEvents = 0; + counter->createdEvents = 0; + counter->deletedEvents = 0; }
typedef struct { virConnectPtr conn; virNetworkPtr net; virStoragePoolPtr pool; + virNodeDevicePtr dev; } objecteventTest;
@@ -163,6 +179,21 @@ storagePoolRefreshCb(virConnectPtr conn ATTRIBUTE_UNUSED, (*counter)++; }
+static void +nodeDeviceLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeDevicePtr dev ATTRIBUTE_UNUSED, + int event, + int detail ATTRIBUTE_UNUSED, + void* opaque) +{ + lifecycleEventCounter *counter = opaque; + + if (event == VIR_NODE_DEVICE_EVENT_CREATED) + counter->createdEvents++; + else if (event == VIR_NODE_DEVICE_EVENT_DELETED) + counter->deletedEvents++; +} + static int testDomainCreateXMLOld(const void *data) { @@ -691,6 +722,42 @@ testStoragePoolStartStopEvent(const void *data) return ret; }
+static int +testNodeDeviceCreateXML(const void *data) +{ + const objecteventTest *test = data; + lifecycleEventCounter counter; + virNodeDevicePtr dev; + int id; + int ret = 0; + + lifecycleEventCounter_reset(&counter); + + id = virConnectNodeDeviceEventRegisterAny(test->conn, NULL, + VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE, + VIR_NODE_DEVICE_EVENT_CALLBACK(&nodeDeviceLifecycleCb), + &counter, NULL); + dev = virNodeDeviceCreateXML(test->conn, nodeDeviceDef, 0); + virNodeDeviceDestroy(dev); + + if (!dev || virEventRunDefaultImpl() < 0) { + ret = -1; + goto cleanup; + } + + if (counter.createdEvents != 1 || counter.deletedEvents != 1 || + counter.unexpectedEvents > 0) { + ret = -1; + goto cleanup; + } + + cleanup: + virConnectNodeDeviceEventDeregisterAny(test->conn, id); + if (dev) + virNodeDeviceFree(dev); + return ret; +} + static void timeout(int id ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -765,6 +832,11 @@ mymain(void) testStoragePoolStartStopEvent, &test) < 0) ret = EXIT_FAILURE;
+ /* Node device event tests */ + if (virTestRun("Node device createXML add event ", + testNodeDeviceCreateXML, &test) < 0) + ret = EXIT_FAILURE; + /* Cleanup */ if (test.pool) { virStoragePoolUndefine(test.pool);
Rest looks good to me Thanks, Cole

--- daemon/libvirtd.h | 2 + daemon/remote.c | 206 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 139 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++++- src/remote_protocol-structs | 19 ++++ 5 files changed, 408 insertions(+), 1 deletion(-) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index cc91266..09d505d 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -62,6 +62,8 @@ struct daemonClientPrivate { size_t nqemuEventCallbacks; daemonClientEventCallbackPtr *storageEventCallbacks; size_t nstorageEventCallbacks; + daemonClientEventCallbackPtr *nodeDeviceEventCallbacks; + size_t nnodeDeviceEventCallbacks; bool closeRegistered; # if WITH_SASL diff --git a/daemon/remote.c b/daemon/remote.c index 4aa43c2..d07cacd 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -91,6 +91,7 @@ static virStorageVolPtr get_nonnull_storage_vol(virConnectPtr conn, remote_nonnu static virSecretPtr get_nonnull_secret(virConnectPtr conn, remote_nonnull_secret secret); static virNWFilterPtr get_nonnull_nwfilter(virConnectPtr conn, remote_nonnull_nwfilter nwfilter); static virDomainSnapshotPtr get_nonnull_domain_snapshot(virDomainPtr dom, remote_nonnull_domain_snapshot snapshot); +static virNodeDevicePtr get_nonnull_node_device(virConnectPtr conn, remote_nonnull_node_device dev); static void make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src); static void make_nonnull_network(remote_nonnull_network *net_dst, virNetworkPtr net_src); static void make_nonnull_interface(remote_nonnull_interface *interface_dst, virInterfacePtr interface_src); @@ -209,6 +210,32 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr client, } static bool +remoteRelayNodeDeviceEventCheckACL(virNetServerClientPtr client, + virConnectPtr conn, + virNodeDevicePtr dev) +{ + virNodeDeviceDef def; + virIdentityPtr identity = NULL; + bool ret = false; + + /* For now, we just create a virNodeDeviceDef with enough contents to + * satisfy what viraccessdriverpolkit.c references. This is a bit + * fragile, but I don't know of anything better. */ + def.name = dev->name; + + if (!(identity = virNetServerClientGetIdentity(client))) + goto cleanup; + if (virIdentitySetCurrent(identity) < 0) + goto cleanup; + ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, &def); + + cleanup: + ignore_value(virIdentitySetCurrent(NULL)); + virObjectUnref(identity); + return ret; +} + +static bool remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, virConnectPtr conn, virDomainPtr dom) { @@ -1329,6 +1356,44 @@ static virConnectStoragePoolEventGenericCallback storageEventCallbacks[] = { verify(ARRAY_CARDINALITY(storageEventCallbacks) == VIR_STORAGE_POOL_EVENT_ID_LAST); +static int +remoteRelayNodeDeviceEventLifecycle(virConnectPtr conn, + virNodeDevicePtr dev, + int event, + int detail, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_node_device_event_lifecycle_msg data; + + if (callback->callbackID < 0 || + !remoteRelayNodeDeviceEventCheckACL(callback->client, conn, dev)) + return -1; + + VIR_DEBUG("Relaying node device lifecycle event %d, detail %d, callback %d", + event, detail, callback->callbackID); + + /* build return data */ + memset(&data, 0, sizeof(data)); + make_nonnull_node_device(&data.dev, dev); + data.callbackID = callback->callbackID; + data.event = event; + data.detail = detail; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_NODE_DEVICE_EVENT_LIFECYCLE, + (xdrproc_t)xdr_remote_node_device_event_lifecycle_msg, + &data); + + return 0; +} + +static virConnectNodeDeviceEventGenericCallback nodeDeviceEventCallbacks[] = { + VIR_NODE_DEVICE_EVENT_CALLBACK(remoteRelayNodeDeviceEventLifecycle), +}; + +verify(ARRAY_CARDINALITY(nodeDeviceEventCallbacks) == VIR_NODE_DEVICE_EVENT_ID_LAST); + static void remoteRelayDomainQemuMonitorEvent(virConnectPtr conn, virDomainPtr dom, @@ -1451,6 +1516,21 @@ void remoteClientFreeFunc(void *data) } VIR_FREE(priv->storageEventCallbacks); + for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) { + int callbackID = priv->nodeDeviceEventCallbacks[i]->callbackID; + if (callbackID < 0) { + VIR_WARN("unexpected incomplete node device callback %zu", i); + continue; + } + VIR_DEBUG("Deregistering remote node device event relay %d", + callbackID); + priv->nodeDeviceEventCallbacks[i]->callbackID = -1; + if (virConnectNodeDeviceEventDeregisterAny(priv->conn, + callbackID) < 0) + VIR_WARN("unexpected node device event deregister failure"); + } + VIR_FREE(priv->nodeDeviceEventCallbacks); + for (i = 0; i < priv->nqemuEventCallbacks; i++) { int callbackID = priv->qemuEventCallbacks[i]->callbackID; if (callbackID < 0) { @@ -5656,6 +5736,126 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIB return rv; } +static int +remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_connect_node_device_event_register_any_args *args, + remote_connect_node_device_event_register_any_ret *ret) +{ + int callbackID; + int rv = -1; + daemonClientEventCallbackPtr callback = NULL; + daemonClientEventCallbackPtr ref; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + virNodeDevicePtr dev = NULL; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + if (args->dev && + !(dev = get_nonnull_node_device(priv->conn, *args->dev))) + goto cleanup; + + if (args->eventID >= VIR_NODE_DEVICE_EVENT_ID_LAST || args->eventID < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported storage pool event ID %d"), args->eventID); + goto cleanup; + } + + /* If we call register first, we could append a complete callback + * to our array, but on OOM append failure, we'd have to then hope + * deregister works to undo our register. So instead we append an + * incomplete callback to our array, then register, then fix up + * our callback; but since VIR_APPEND_ELEMENT clears 'callback' on + * success, we use 'ref' to save a copy of the pointer. */ + if (VIR_ALLOC(callback) < 0) + goto cleanup; + callback->client = client; + callback->eventID = args->eventID; + callback->callbackID = -1; + ref = callback; + if (VIR_APPEND_ELEMENT(priv->nodeDeviceEventCallbacks, + priv->nnodeDeviceEventCallbacks, + callback) < 0) + goto cleanup; + + if ((callbackID = virConnectNodeDeviceEventRegisterAny(priv->conn, + dev, + args->eventID, + nodeDeviceEventCallbacks[args->eventID], + ref, + remoteEventCallbackFree)) < 0) { + VIR_SHRINK_N(priv->nodeDeviceEventCallbacks, + priv->nnodeDeviceEventCallbacks, 1); + callback = ref; + goto cleanup; + } + + ref->callbackID = callbackID; + ret->callbackID = callbackID; + + rv = 0; + + cleanup: + VIR_FREE(callback); + if (rv < 0) + virNetMessageSaveError(rerr); + virObjectUnref(dev); + virMutexUnlock(&priv->lock); + return rv; +} + +static int +remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_connect_node_device_event_deregister_any_args *args) +{ + int rv = -1; + size_t i; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) { + if (priv->nodeDeviceEventCallbacks[i]->callbackID == args->callbackID) + break; + } + if (i == priv->nnodeDeviceEventCallbacks) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("node device event callback %d not registered"), + args->callbackID); + goto cleanup; + } + + if (virConnectNodeDeviceEventDeregisterAny(priv->conn, args->callbackID) < 0) + goto cleanup; + + VIR_DELETE_ELEMENT(priv->nodeDeviceEventCallbacks, i, + priv->nnodeDeviceEventCallbacks); + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} static int qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED, @@ -6425,6 +6625,12 @@ get_nonnull_domain_snapshot(virDomainPtr dom, remote_nonnull_domain_snapshot sna return virGetDomainSnapshot(dom, snapshot.name); } +static virNodeDevicePtr +get_nonnull_node_device(virConnectPtr conn, remote_nonnull_node_device dev) +{ + return virGetNodeDevice(conn, dev.name); +} + /* Make remote_nonnull_domain and remote_nonnull_network. */ static void make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index 29552aa..fbaec80 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -35,6 +35,7 @@ #include "domain_event.h" #include "network_event.h" #include "storage_event.h" +#include "node_device_event.h" #include "driver.h" #include "virbuffer.h" #include "remote_driver.h" @@ -151,6 +152,8 @@ static void make_nonnull_network(remote_nonnull_network *net_dst, virNetworkPtr static void make_nonnull_interface(remote_nonnull_interface *interface_dst, virInterfacePtr interface_src); static void make_nonnull_storage_pool(remote_nonnull_storage_pool *pool_dst, virStoragePoolPtr vol_src); static void make_nonnull_storage_vol(remote_nonnull_storage_vol *vol_dst, virStorageVolPtr vol_src); +static void +make_nonnull_node_device(remote_nonnull_node_device *dev_dst, virNodeDevicePtr dev_src); static void make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr secret_src); static void make_nonnull_nwfilter(remote_nonnull_nwfilter *nwfilter_dst, virNWFilterPtr nwfilter_src); static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src); @@ -367,6 +370,11 @@ remoteStoragePoolBuildEventRefresh(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, void *evdata, void *opaque); static void +remoteNodeDeviceBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque); + +static void remoteConnectNotifyEventConnectionClosed(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque); @@ -553,6 +561,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteStoragePoolBuildEventRefresh, sizeof(remote_storage_pool_event_refresh_msg), (xdrproc_t)xdr_remote_storage_pool_event_refresh_msg }, + { REMOTE_PROC_NODE_DEVICE_EVENT_LIFECYCLE, + remoteNodeDeviceBuildEventLifecycle, + sizeof(remote_node_device_event_lifecycle_msg), + (xdrproc_t)xdr_remote_node_device_event_lifecycle_msg }, }; static void @@ -3147,6 +3159,103 @@ remoteConnectStoragePoolEventDeregisterAny(virConnectPtr conn, static int +remoteConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + int rv = -1; + struct private_data *priv = conn->privateData; + remote_connect_node_device_event_register_any_args args; + remote_connect_node_device_event_register_any_ret ret; + int callbackID; + int count; + remote_nonnull_node_device node_device; + + remoteDriverLock(priv); + + if ((count = virNodeDeviceEventStateRegisterClient(conn, priv->eventState, + dev, eventID, callback, + opaque, freecb, + &callbackID)) < 0) + goto done; + + /* If this is the first callback for this eventID, we need to enable + * events on the server */ + if (count == 1) { + args.eventID = eventID; + if (dev) { + make_nonnull_node_device(&node_device, dev); + args.dev = &node_device; + } else { + args.dev = NULL; + } + + memset(&ret, 0, sizeof(ret)); + if (call(conn, priv, 0, REMOTE_PROC_CONNECT_NODE_DEVICE_EVENT_REGISTER_ANY, + (xdrproc_t) xdr_remote_connect_node_device_event_register_any_args, (char *) &args, + (xdrproc_t) xdr_remote_connect_node_device_event_register_any_ret, (char *) &ret) == -1) { + virObjectEventStateDeregisterID(conn, priv->eventState, + callbackID); + goto done; + } + + virObjectEventStateSetRemote(conn, priv->eventState, callbackID, + ret.callbackID); + } + + rv = callbackID; + + done: + remoteDriverUnlock(priv); + return rv; +} + + +static int +remoteConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + struct private_data *priv = conn->privateData; + int rv = -1; + remote_connect_node_device_event_deregister_any_args args; + int eventID; + int remoteID; + int count; + + remoteDriverLock(priv); + + if ((eventID = virObjectEventStateEventID(conn, priv->eventState, + callbackID, &remoteID)) < 0) + goto done; + + if ((count = virObjectEventStateDeregisterID(conn, priv->eventState, + callbackID)) < 0) + goto done; + + /* If that was the last callback for this eventID, we need to disable + * events on the server */ + if (count == 0) { + args.callbackID = remoteID; + + if (call(conn, priv, 0, REMOTE_PROC_CONNECT_NODE_DEVICE_EVENT_DEREGISTER_ANY, + (xdrproc_t) xdr_remote_connect_node_device_event_deregister_any_args, (char *) &args, + (xdrproc_t) xdr_void, (char *) NULL) == -1) + goto done; + + } + + rv = 0; + + done: + remoteDriverUnlock(priv); + return rv; +} + + +static int remoteConnectDomainQemuMonitorEventRegister(virConnectPtr conn, virDomainPtr dom, const char *event, @@ -5155,6 +5264,28 @@ remoteStoragePoolBuildEventRefresh(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, } static void +remoteNodeDeviceBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + struct private_data *priv = conn->privateData; + remote_node_device_event_lifecycle_msg *msg = evdata; + virNodeDevicePtr dev; + virObjectEventPtr event = NULL; + + dev = get_nonnull_node_device(conn, msg->dev); + if (!dev) + return; + + event = virNodeDeviceEventLifecycleNew(dev->name, msg->event, + msg->detail); + virObjectUnref(dev); + + remoteEventQueue(priv, event, msg->callbackID); +} + +static void remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, virNetClientPtr client ATTRIBUTE_UNUSED, void *evdata, void *opaque) @@ -7753,6 +7884,12 @@ make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr secret_src) } static void +make_nonnull_node_device(remote_nonnull_node_device *dev_dst, virNodeDevicePtr dev_src) +{ + dev_dst->name = dev_src->name; +} + +static void make_nonnull_nwfilter(remote_nonnull_nwfilter *nwfilter_dst, virNWFilterPtr nwfilter_src) { nwfilter_dst->name = nwfilter_src->name; @@ -8103,6 +8240,8 @@ static virSecretDriver secret_driver = { }; static virNodeDeviceDriver node_device_driver = { + .connectNodeDeviceEventDeregisterAny = remoteConnectNodeDeviceEventDeregisterAny, /* 2.1.0 */ + .connectNodeDeviceEventRegisterAny = remoteConnectNodeDeviceEventRegisterAny, /* 2.1.0 */ .nodeNumOfDevices = remoteNodeNumOfDevices, /* 0.5.0 */ .nodeListDevices = remoteNodeListDevices, /* 0.5.0 */ .connectListAllNodeDevices = remoteConnectListAllNodeDevices, /* 0.10.2 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 4403714..b4fd057 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3131,6 +3131,26 @@ struct remote_storage_pool_event_refresh_msg { remote_nonnull_storage_pool pool; }; +struct remote_connect_node_device_event_register_any_args { + int eventID; + remote_node_device dev; +}; + +struct remote_connect_node_device_event_register_any_ret { + int callbackID; +}; + +struct remote_connect_node_device_event_deregister_any_args { + int callbackID; +}; + +struct remote_node_device_event_lifecycle_msg { + int callbackID; + remote_nonnull_node_device dev; + int event; + int detail; +}; + struct remote_domain_fsfreeze_args { remote_nonnull_domain dom; remote_nonnull_string mountpoints<REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX>; /* (const char **) */ @@ -5882,5 +5902,26 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_STORAGE_POOL_EVENT_REFRESH = 373 + REMOTE_PROC_STORAGE_POOL_EVENT_REFRESH = 373, + + /** + * @generate: none + * @priority: high + * @acl: connect:search_node_devices + * @aclfilter: node_device:getattr + */ + REMOTE_PROC_CONNECT_NODE_DEVICE_EVENT_REGISTER_ANY = 374, + + /** + * @generate: none + * @priority: high + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_NODE_DEVICE_EVENT_DEREGISTER_ANY = 375, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_NODE_DEVICE_EVENT_LIFECYCLE = 376 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 46e798b..0e66fc5 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2571,6 +2571,22 @@ struct remote_storage_pool_event_refresh_msg { int callbackID; remote_nonnull_storage_pool pool; }; +struct remote_connect_node_device_event_register_any_args { + int eventID; + remote_node_device dev; +}; +struct remote_connect_node_device_event_register_any_ret { + int callbackID; +}; +struct remote_connect_node_device_event_deregister_any_args { + int callbackID; +}; +struct remote_node_device_event_lifecycle_msg { + int callbackID; + remote_nonnull_node_device dev; + int event; + int detail; +}; struct remote_domain_fsfreeze_args { remote_nonnull_domain dom; struct { @@ -3145,4 +3161,7 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_GET_GUEST_VCPUS = 371, REMOTE_PROC_DOMAIN_SET_GUEST_VCPUS = 372, REMOTE_PROC_STORAGE_POOL_EVENT_REFRESH = 373, + REMOTE_PROC_CONNECT_NODE_DEVICE_EVENT_REGISTER_ANY = 374, + REMOTE_PROC_CONNECT_NODE_DEVICE_EVENT_DEREGISTER_ANY = 375, + REMOTE_PROC_NODE_DEVICE_EVENT_LIFECYCLE = 376, }; -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
--- daemon/libvirtd.h | 2 + daemon/remote.c | 206 +++++++++++++++++++++++++++++++++++++++++++ src/remote/remote_driver.c | 139 +++++++++++++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++++- src/remote_protocol-structs | 19 ++++ 5 files changed, 408 insertions(+), 1 deletion(-)
diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index cc91266..09d505d 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -62,6 +62,8 @@ struct daemonClientPrivate { size_t nqemuEventCallbacks; daemonClientEventCallbackPtr *storageEventCallbacks; size_t nstorageEventCallbacks; + daemonClientEventCallbackPtr *nodeDeviceEventCallbacks; + size_t nnodeDeviceEventCallbacks; bool closeRegistered;
# if WITH_SASL diff --git a/daemon/remote.c b/daemon/remote.c index 4aa43c2..d07cacd 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -91,6 +91,7 @@ static virStorageVolPtr get_nonnull_storage_vol(virConnectPtr conn, remote_nonnu static virSecretPtr get_nonnull_secret(virConnectPtr conn, remote_nonnull_secret secret); static virNWFilterPtr get_nonnull_nwfilter(virConnectPtr conn, remote_nonnull_nwfilter nwfilter); static virDomainSnapshotPtr get_nonnull_domain_snapshot(virDomainPtr dom, remote_nonnull_domain_snapshot snapshot); +static virNodeDevicePtr get_nonnull_node_device(virConnectPtr conn, remote_nonnull_node_device dev); static void make_nonnull_domain(remote_nonnull_domain *dom_dst, virDomainPtr dom_src); static void make_nonnull_network(remote_nonnull_network *net_dst, virNetworkPtr net_src); static void make_nonnull_interface(remote_nonnull_interface *interface_dst, virInterfacePtr interface_src); @@ -209,6 +210,32 @@ remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr client, }
static bool +remoteRelayNodeDeviceEventCheckACL(virNetServerClientPtr client, + virConnectPtr conn, + virNodeDevicePtr dev) +{ + virNodeDeviceDef def; + virIdentityPtr identity = NULL; + bool ret = false; + + /* For now, we just create a virNodeDeviceDef with enough contents to + * satisfy what viraccessdriverpolkit.c references. This is a bit + * fragile, but I don't know of anything better. */ + def.name = dev->name; + + if (!(identity = virNetServerClientGetIdentity(client))) + goto cleanup; + if (virIdentitySetCurrent(identity) < 0) + goto cleanup; + ret = virConnectNodeDeviceEventRegisterAnyCheckACL(conn, &def); + + cleanup: + ignore_value(virIdentitySetCurrent(NULL)); + virObjectUnref(identity); + return ret; +} + +static bool remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, virConnectPtr conn, virDomainPtr dom) { @@ -1329,6 +1356,44 @@ static virConnectStoragePoolEventGenericCallback storageEventCallbacks[] = {
verify(ARRAY_CARDINALITY(storageEventCallbacks) == VIR_STORAGE_POOL_EVENT_ID_LAST);
+static int +remoteRelayNodeDeviceEventLifecycle(virConnectPtr conn, + virNodeDevicePtr dev, + int event, + int detail, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_node_device_event_lifecycle_msg data; + + if (callback->callbackID < 0 || + !remoteRelayNodeDeviceEventCheckACL(callback->client, conn, dev)) + return -1; + + VIR_DEBUG("Relaying node device lifecycle event %d, detail %d, callback %d", + event, detail, callback->callbackID); + + /* build return data */ + memset(&data, 0, sizeof(data)); + make_nonnull_node_device(&data.dev, dev); + data.callbackID = callback->callbackID; + data.event = event; + data.detail = detail; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_NODE_DEVICE_EVENT_LIFECYCLE, + (xdrproc_t)xdr_remote_node_device_event_lifecycle_msg, + &data); + + return 0; +} + +static virConnectNodeDeviceEventGenericCallback nodeDeviceEventCallbacks[] = { + VIR_NODE_DEVICE_EVENT_CALLBACK(remoteRelayNodeDeviceEventLifecycle), +}; + +verify(ARRAY_CARDINALITY(nodeDeviceEventCallbacks) == VIR_NODE_DEVICE_EVENT_ID_LAST); + static void remoteRelayDomainQemuMonitorEvent(virConnectPtr conn, virDomainPtr dom, @@ -1451,6 +1516,21 @@ void remoteClientFreeFunc(void *data) } VIR_FREE(priv->storageEventCallbacks);
+ for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) { + int callbackID = priv->nodeDeviceEventCallbacks[i]->callbackID; + if (callbackID < 0) { + VIR_WARN("unexpected incomplete node device callback %zu", i); + continue; + } + VIR_DEBUG("Deregistering remote node device event relay %d", + callbackID); + priv->nodeDeviceEventCallbacks[i]->callbackID = -1; + if (virConnectNodeDeviceEventDeregisterAny(priv->conn, + callbackID) < 0) + VIR_WARN("unexpected node device event deregister failure"); + } + VIR_FREE(priv->nodeDeviceEventCallbacks); + for (i = 0; i < priv->nqemuEventCallbacks; i++) { int callbackID = priv->qemuEventCallbacks[i]->callbackID; if (callbackID < 0) { @@ -5656,6 +5736,126 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIB return rv; }
+static int +remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_connect_node_device_event_register_any_args *args, + remote_connect_node_device_event_register_any_ret *ret) +{ + int callbackID; + int rv = -1; + daemonClientEventCallbackPtr callback = NULL; + daemonClientEventCallbackPtr ref; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + virNodeDevicePtr dev = NULL; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + if (args->dev && + !(dev = get_nonnull_node_device(priv->conn, *args->dev))) + goto cleanup; + + if (args->eventID >= VIR_NODE_DEVICE_EVENT_ID_LAST || args->eventID < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unsupported storage pool event ID %d"), args->eventID); + goto cleanup; + }
Error message mentions storage Otherwise the rest looks fine to me - Cole

--- src/conf/node_device_conf.h | 4 ++++ src/node_device/node_device_driver.c | 42 ++++++++++++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 10 +++++++++ 3 files changed, 56 insertions(+) diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 9e3e6fe..8f23a98 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -31,6 +31,7 @@ # include "virthread.h" # include "virpci.h" # include "device_conf.h" +# include "object_event.h" # include <libxml/tree.h> @@ -229,6 +230,9 @@ struct _virNodeDeviceDriverState { virNodeDeviceObjList devs; /* currently-known devices */ void *privateData; /* driver-specific private data */ + + /* Immutable pointer, self-locking APIs */ + virObjectEventStatePtr nodeDeviceEventState; }; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 500caeb..91bb142 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -35,6 +35,7 @@ #include "virfile.h" #include "virstring.h" #include "node_device_conf.h" +#include "node_device_event.h" #include "node_device_driver.h" #include "node_device_hal.h" #include "node_device_linux_sysfs.h" @@ -677,6 +678,47 @@ nodeDeviceDestroy(virNodeDevicePtr dev) return ret; } +int +nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + int callbackID = -1; + + if (virConnectNodeDeviceEventRegisterAnyEnsureACL(conn) < 0) + goto cleanup; + + if (virNodeDeviceEventStateRegisterID(conn, driver->nodeDeviceEventState, + dev, eventID, callback, + opaque, freecb, &callbackID) < 0) + callbackID = -1; + cleanup: + return callbackID; +} + +int +nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + int ret = -1; + + if (virConnectNodeDeviceEventDeregisterAnyEnsureACL(conn) < 0) + goto cleanup; + + if (virObjectEventStateDeregisterID(conn, + driver->nodeDeviceEventState, + callbackID) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} + int nodedevRegister(void) { #ifdef WITH_UDEV diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 0f4ea57..56f89ab 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -63,4 +63,14 @@ virNodeDevicePtr nodeDeviceCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags); int nodeDeviceDestroy(virNodeDevicePtr dev); +int +nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback callback, + void *opaque, + virFreeCallback freecb); +int +nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID); #endif /* __VIR_NODE_DEVICE_H__ */ -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
--- src/conf/node_device_conf.h | 4 ++++ src/node_device/node_device_driver.c | 42 ++++++++++++++++++++++++++++++++++++ src/node_device/node_device_driver.h | 10 +++++++++ 3 files changed, 56 insertions(+)
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 9e3e6fe..8f23a98 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -31,6 +31,7 @@ # include "virthread.h" # include "virpci.h" # include "device_conf.h" +# include "object_event.h"
# include <libxml/tree.h>
@@ -229,6 +230,9 @@ struct _virNodeDeviceDriverState {
virNodeDeviceObjList devs; /* currently-known devices */ void *privateData; /* driver-specific private data */ + + /* Immutable pointer, self-locking APIs */ + virObjectEventStatePtr nodeDeviceEventState; };
diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index 500caeb..91bb142 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -35,6 +35,7 @@ #include "virfile.h" #include "virstring.h" #include "node_device_conf.h" +#include "node_device_event.h" #include "node_device_driver.h" #include "node_device_hal.h" #include "node_device_linux_sysfs.h" @@ -677,6 +678,47 @@ nodeDeviceDestroy(virNodeDevicePtr dev) return ret; }
+int +nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + int callbackID = -1; + + if (virConnectNodeDeviceEventRegisterAnyEnsureACL(conn) < 0) + goto cleanup; + + if (virNodeDeviceEventStateRegisterID(conn, driver->nodeDeviceEventState, + dev, eventID, callback, + opaque, freecb, &callbackID) < 0) + callbackID = -1; + cleanup: + return callbackID; +} + +int +nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + int ret = -1; + + if (virConnectNodeDeviceEventDeregisterAnyEnsureACL(conn) < 0) + goto cleanup; + + if (virObjectEventStateDeregisterID(conn, + driver->nodeDeviceEventState, + callbackID) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} +
So this is just adding the generic API entrypoints. It looks a little strange adding functions that can access driver->nodeDeviceEventState without any associated code to initialize nodeDeviceEventState, but since we leave that up to hal/udev backends I think this makes sense. Plus these functions aren't callable via the public API unless the backend exports the function via virNodeDeviceDriver, so the backends need to explicitly opt in anyways ACK - Cole
int nodedevRegister(void) { #ifdef WITH_UDEV diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 0f4ea57..56f89ab 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -63,4 +63,14 @@ virNodeDevicePtr nodeDeviceCreateXML(virConnectPtr conn, const char *xmlDesc, unsigned int flags); int nodeDeviceDestroy(virNodeDevicePtr dev);
+int +nodeConnectNodeDeviceEventRegisterAny(virConnectPtr conn, + virNodeDevicePtr dev, + int eventID, + virConnectNodeDeviceEventGenericCallback callback, + void *opaque, + virFreeCallback freecb); +int +nodeConnectNodeDeviceEventDeregisterAny(virConnectPtr conn, + int callbackID); #endif /* __VIR_NODE_DEVICE_H__ */

--- 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; @@ -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; } + 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); 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 */ .nodeDeviceLookupByName = nodeDeviceLookupByName, /* 0.7.3 */ .nodeDeviceLookupSCSIHostByWWN = nodeDeviceLookupSCSIHostByWWN, /* 1.0.2 */ .nodeDeviceGetXMLDesc = nodeDeviceGetXMLDesc, /* 0.7.3 */ -- 2.7.4

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 */

--- examples/object-events/event-test.c | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index f2f71cc..2f9756a 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -355,6 +355,20 @@ storagePoolEventToString(int event) return "unknown"; } +static const char * +nodeDeviceEventToString(int event) +{ + switch ((virNodeDeviceEventLifecycleType) event) { + case VIR_NODE_DEVICE_EVENT_CREATED: + return "Created"; + case VIR_NODE_DEVICE_EVENT_DELETED: + return "Deleted"; + case VIR_NODE_DEVICE_EVENT_LAST: + break; + } + return "unknown"; +} + static int myDomainEventCallback1(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -688,6 +702,21 @@ myStoragePoolEventRefreshCallback(virConnectPtr conn ATTRIBUTE_UNUSED, } +static int +myNodeDeviceEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeDevicePtr dev, + int event, + int detail, + void *opaque ATTRIBUTE_UNUSED) +{ + printf("%s EVENT: Node device %s %s %d\n", __func__, + virNodeDeviceGetName(dev), + nodeDeviceEventToString(event), + detail); + return 0; +} + + static void eventTypedParamsPrint(virTypedParameterPtr params, int nparams) @@ -948,9 +977,24 @@ struct storagePoolEventData storagePoolEvents[] = { STORAGE_POOL_EVENT(VIR_STORAGE_POOL_EVENT_ID_REFRESH, myStoragePoolEventRefreshCallback), }; +struct nodeDeviceEventData { + int event; + int id; + virConnectNodeDeviceEventGenericCallback cb; + const char *name; +}; + +#define NODE_DEVICE_EVENT(event, callback) \ + {event, -1, VIR_NODE_DEVICE_EVENT_CALLBACK(callback), #event} + +struct nodeDeviceEventData nodeDeviceEvents[] = { + NODE_DEVICE_EVENT(VIR_NODE_DEVICE_EVENT_ID_LIFECYCLE, myNodeDeviceEventCallback), +}; + /* make sure that the events are kept in sync */ verify(ARRAY_CARDINALITY(domainEvents) == VIR_DOMAIN_EVENT_ID_LAST); verify(ARRAY_CARDINALITY(storagePoolEvents) == VIR_STORAGE_POOL_EVENT_ID_LAST); +verify(ARRAY_CARDINALITY(nodeDeviceEvents) == VIR_NODE_DEVICE_EVENT_ID_LAST); int main(int argc, char **argv) @@ -1042,6 +1086,22 @@ main(int argc, char **argv) } } + /* register common node device callbacks */ + for (i = 0; i < ARRAY_CARDINALITY(nodeDeviceEvents); i++) { + struct nodeDeviceEventData *event = nodeDeviceEvents + i; + + event->id = virConnectNodeDeviceEventRegisterAny(dconn, NULL, + event->event, + event->cb, + strdup(event->name), + myFreeFunc); + + if (event->id < 0) { + fprintf(stderr, "Failed to register event '%s'\n", event->name); + goto cleanup; + } + } + if ((callback1ret == -1) || (callback16ret == -1)) goto cleanup; @@ -1077,6 +1137,14 @@ main(int argc, char **argv) virConnectStoragePoolEventDeregisterAny(dconn, storagePoolEvents[i].id); } + + printf("Deregistering node device event callbacks\n"); + for (i = 0; i < ARRAY_CARDINALITY(nodeDeviceEvents); i++) { + if (nodeDeviceEvents[i].id > 0) + virConnectNodeDeviceEventDeregisterAny(dconn, nodeDeviceEvents[i].id); + } + + virConnectUnregisterCloseCallback(dconn, connectClose); ret = EXIT_SUCCESS; -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
--- examples/object-events/event-test.c | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+)
ACK, looks fine Thanks, Cole

Add nodedev-event support for node device lifecycle events --- tools/virsh-nodedev.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-nodedev.h | 10 +++ tools/virsh.pod | 18 +++++ 3 files changed, 239 insertions(+) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index a715b61..a48f0bd 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -31,6 +31,7 @@ #include "viralloc.h" #include "virfile.h" #include "virstring.h" +#include "virtime.h" #include "conf/node_device_conf.h" /* @@ -739,6 +740,210 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd) return ret; } +/* + * "nodedev-event" command + */ +VIR_ENUM_DECL(virshNodeDeviceEvent) +VIR_ENUM_IMPL(virshNodeDeviceEvent, + VIR_NODE_DEVICE_EVENT_LAST, + N_("Created"), + N_("Deleted")) + +static const char * +virshNodeDeviceEventToString(int event) +{ + const char *str = virshNodeDeviceEventTypeToString(event); + return str ? _(str) : _("unknown"); +} + +struct virshNodeDeviceEventData { + vshControl *ctl; + bool loop; + bool timestamp; + int count; +}; +typedef struct virshNodeDeviceEventData virshNodeDeviceEventData; + +VIR_ENUM_DECL(virshNodeDeviceEventId) +VIR_ENUM_IMPL(virshNodeDeviceEventId, + VIR_NODE_DEVICE_EVENT_ID_LAST, + "lifecycle") + +static void +vshEventLifecyclePrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeDevicePtr dev, + int event, + int detail ATTRIBUTE_UNUSED, + void *opaque) +{ + virshNodeDeviceEventData *data = opaque; + + if (!data->loop && data->count) + return; + + if (data->timestamp) { + char timestamp[VIR_TIME_STRING_BUFLEN]; + + if (virTimeStringNowRaw(timestamp) < 0) + timestamp[0] = '\0'; + + vshPrint(data->ctl, _("%s: event 'lifecycle' for node device %s: %s\n"), + timestamp, + virNodeDeviceGetName(dev), virshNodeDeviceEventToString(event)); + } else { + vshPrint(data->ctl, _("event 'lifecycle' for node device %s: %s\n"), + virNodeDeviceGetName(dev), virshNodeDeviceEventToString(event)); + } + + data->count++; + if (!data->loop) + vshEventDone(data->ctl); +} + +static const vshCmdInfo info_node_device_event[] = { + {.name = "help", + .data = N_("Node Device Events") + }, + {.name = "desc", + .data = N_("List event types, or wait for node device events to occur") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_node_device_event[] = { + {.name = "node device", + .type = VSH_OT_STRING, + .help = N_("filter by node device name") + }, + {.name = "event", + .type = VSH_OT_STRING, + .help = N_("which event type to wait for") + }, + {.name = "loop", + .type = VSH_OT_BOOL, + .help = N_("loop until timeout or interrupt, rather than one-shot") + }, + {.name = "timeout", + .type = VSH_OT_INT, + .help = N_("timeout seconds") + }, + {.name = "list", + .type = VSH_OT_BOOL, + .help = N_("list valid event types") + }, + {.name = "timestamp", + .type = VSH_OT_BOOL, + .help = N_("show timestamp for each printed event") + }, + {.name = NULL} +}; + +virNodeDevicePtr +virshCommandOptNodeDeviceBy(vshControl *ctl, const vshCmd *cmd, + const char **name, unsigned int flags) +{ + virNodeDevicePtr dev = NULL; + const char *n = NULL; + const char *optname = "node device"; + virCheckFlags(VIRSH_BYNAME, NULL); + virshControlPtr priv = ctl->privData; + + if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + return NULL; + + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + cmd->def->name, optname, n); + + if (name) + *name = n; + + /* try it by NAME */ + if (!dev && (flags & VIRSH_BYNAME)) { + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as node device NAME\n", + cmd->def->name, optname); + dev = virNodeDeviceLookupByName(priv->conn, n); + } + + if (!dev) + vshError(ctl, _("failed to get node device '%s'"), n); + + return dev; +} + +static bool +cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd) +{ + virNodeDevicePtr dev = NULL; + bool ret = false; + int eventId = -1; + int timeout = 0; + virshNodeDeviceEventData data; + const char *eventName = NULL; + int event; + virshControlPtr priv = ctl->privData; + + if (vshCommandOptBool(cmd, "list")) { + size_t i; + + for (i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++) + vshPrint(ctl, "%s\n", virshNodeDeviceEventIdTypeToString(i)); + return true; + } + + if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0) + return false; + if (!eventName) { + vshError(ctl, "%s", _("either --list or event type is required")); + return false; + } + if ((event = virshNodeDeviceEventIdTypeFromString(eventName)) < 0) { + vshError(ctl, _("unknown event type %s"), eventName); + return false; + } + + data.ctl = ctl; + data.loop = vshCommandOptBool(cmd, "loop"); + data.timestamp = vshCommandOptBool(cmd, "timestamp"); + data.count = 0; + if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) + return false; + + if (vshCommandOptBool(cmd, "node device")) + dev = virshCommandOptNodeDevice(ctl, cmd, NULL); + if (vshEventStart(ctl, timeout) < 0) + goto cleanup; + + if ((eventId = virConnectNodeDeviceEventRegisterAny(priv->conn, dev, event, + VIR_NODE_DEVICE_EVENT_CALLBACK(vshEventLifecyclePrint), + &data, NULL)) < 0) + goto cleanup; + switch (vshEventWait(ctl)) { + case VSH_EVENT_INTERRUPT: + vshPrint(ctl, "%s", _("event loop interrupted\n")); + break; + case VSH_EVENT_TIMEOUT: + vshPrint(ctl, "%s", _("event loop timed out\n")); + break; + case VSH_EVENT_DONE: + break; + default: + goto cleanup; + } + vshPrint(ctl, _("events received: %d\n"), data.count); + if (data.count) + ret = true; + + cleanup: + vshEventCleanup(ctl); + if (eventId >= 0 && + virConnectNodeDeviceEventDeregisterAny(priv->conn, eventId) < 0) + ret = false; + if (dev) + virNodeDeviceFree(dev); + return ret; +} + + const vshCmdDef nodedevCmds[] = { {.name = "nodedev-create", .handler = cmdNodeDeviceCreate, @@ -788,5 +993,11 @@ const vshCmdDef nodedevCmds[] = { .info = info_node_device_reset, .flags = 0 }, + {.name = "nodedev-event", + .handler = cmdNodeDeviceEvent, + .opts = opts_node_device_event, + .info = info_node_device_event, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virsh-nodedev.h b/tools/virsh-nodedev.h index c64f7df..b6b2e02 100644 --- a/tools/virsh-nodedev.h +++ b/tools/virsh-nodedev.h @@ -28,6 +28,16 @@ # include "virsh.h" +virNodeDevicePtr +virshCommandOptNodeDeviceBy(vshControl *ctl, const vshCmd *cmd, + const char **name, unsigned int flags); + +/* default is lookup by Name */ +# define virshCommandOptNodeDevice(_ctl, _cmd, _name) \ + virshCommandOptNodeDeviceBy(_ctl, _cmd, _name, \ + VIRSH_BYNAME) + + extern const vshCmdDef nodedevCmds[]; #endif /* VIRSH_NODEDEV_H */ diff --git a/tools/virsh.pod b/tools/virsh.pod index d7cd10e..e3530a1 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -2932,6 +2932,24 @@ a node device between guest passthrough or the host. Libvirt will often do this action implicitly when required, but this command allows an explicit reset when needed. +=item B<nodedev-event> {[I<nodedev>] I<event> [I<--loop>] [I<--timeout> +I<seconds>] [I<--timestamp>] | I<--list>} + +Wait for a class of node device events to occur, and print appropriate +details of events as they happen. The events can optionally be filtered +by I<nodedev>. Using I<--list> as the only argument will provide a list +of possible I<event> values known by this client, although the connection +might not allow registering for all these events. + +By default, this command is one-shot, and returns success once an event +occurs; you can send SIGINT (usually via C<Ctrl-C>) to quit immediately. +If I<--timeout> is specified, the command gives up waiting for events +after I<seconds> have elapsed. With I<--loop>, the command prints all +events until a timeout or interrupt key. + +When I<--timestamp> is used, a human-readable timestamp will be printed +before the event. + =back =head1 VIRTUAL NETWORK COMMANDS -- 2.7.4

On 07/20/2016 09:50 AM, Jovanka Gulicoska wrote:
Add nodedev-event support for node device lifecycle events --- tools/virsh-nodedev.c | 211 ++++++++++++++++++++++++++++++++++++++++++++++++++ tools/virsh-nodedev.h | 10 +++ tools/virsh.pod | 18 +++++ 3 files changed, 239 insertions(+)
diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index a715b61..a48f0bd 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -31,6 +31,7 @@ #include "viralloc.h" #include "virfile.h" #include "virstring.h" +#include "virtime.h" #include "conf/node_device_conf.h"
/* @@ -739,6 +740,210 @@ cmdNodeDeviceReset(vshControl *ctl, const vshCmd *cmd) return ret; }
+/* + * "nodedev-event" command + */ +VIR_ENUM_DECL(virshNodeDeviceEvent) +VIR_ENUM_IMPL(virshNodeDeviceEvent, + VIR_NODE_DEVICE_EVENT_LAST, + N_("Created"), + N_("Deleted")) + +static const char * +virshNodeDeviceEventToString(int event) +{ + const char *str = virshNodeDeviceEventTypeToString(event); + return str ? _(str) : _("unknown"); +} + +struct virshNodeDeviceEventData { + vshControl *ctl; + bool loop; + bool timestamp; + int count; +}; +typedef struct virshNodeDeviceEventData virshNodeDeviceEventData; + +VIR_ENUM_DECL(virshNodeDeviceEventId) +VIR_ENUM_IMPL(virshNodeDeviceEventId, + VIR_NODE_DEVICE_EVENT_ID_LAST, + "lifecycle") + +static void +vshEventLifecyclePrint(virConnectPtr conn ATTRIBUTE_UNUSED, + virNodeDevicePtr dev, + int event, + int detail ATTRIBUTE_UNUSED, + void *opaque) +{ + virshNodeDeviceEventData *data = opaque; + + if (!data->loop && data->count) + return; + + if (data->timestamp) { + char timestamp[VIR_TIME_STRING_BUFLEN]; + + if (virTimeStringNowRaw(timestamp) < 0) + timestamp[0] = '\0'; + + vshPrint(data->ctl, _("%s: event 'lifecycle' for node device %s: %s\n"), + timestamp, + virNodeDeviceGetName(dev), virshNodeDeviceEventToString(event)); + } else { + vshPrint(data->ctl, _("event 'lifecycle' for node device %s: %s\n"), + virNodeDeviceGetName(dev), virshNodeDeviceEventToString(event)); + } + + data->count++; + if (!data->loop) + vshEventDone(data->ctl); +} + +static const vshCmdInfo info_node_device_event[] = { + {.name = "help", + .data = N_("Node Device Events") + }, + {.name = "desc", + .data = N_("List event types, or wait for node device events to occur") + }, + {.name = NULL} +}; + +static const vshCmdOptDef opts_node_device_event[] = { + {.name = "node device", + .type = VSH_OT_STRING, + .help = N_("filter by node device name") + }, + {.name = "event", + .type = VSH_OT_STRING, + .help = N_("which event type to wait for") + }, + {.name = "loop", + .type = VSH_OT_BOOL, + .help = N_("loop until timeout or interrupt, rather than one-shot") + }, + {.name = "timeout", + .type = VSH_OT_INT, + .help = N_("timeout seconds") + }, + {.name = "list", + .type = VSH_OT_BOOL, + .help = N_("list valid event types") + }, + {.name = "timestamp", + .type = VSH_OT_BOOL, + .help = N_("show timestamp for each printed event") + }, + {.name = NULL} +}; + +virNodeDevicePtr +virshCommandOptNodeDeviceBy(vshControl *ctl, const vshCmd *cmd, + const char **name, unsigned int flags) +{ + virNodeDevicePtr dev = NULL; + const char *n = NULL; + const char *optname = "node device"; + virCheckFlags(VIRSH_BYNAME, NULL); + virshControlPtr priv = ctl->privData; + + if (vshCommandOptStringReq(ctl, cmd, optname, &n) < 0) + return NULL; + + vshDebug(ctl, VSH_ERR_INFO, "%s: found option <%s>: %s\n", + cmd->def->name, optname, n); + + if (name) + *name = n; + + /* try it by NAME */ + if (!dev && (flags & VIRSH_BYNAME)) { + vshDebug(ctl, VSH_ERR_DEBUG, "%s: <%s> trying as node device NAME\n", + cmd->def->name, optname); + dev = virNodeDeviceLookupByName(priv->conn, n); + } + + if (!dev) + vshError(ctl, _("failed to get node device '%s'"), n); + + return dev; +} + +static bool +cmdNodeDeviceEvent(vshControl *ctl, const vshCmd *cmd) +{ + virNodeDevicePtr dev = NULL; + bool ret = false; + int eventId = -1; + int timeout = 0; + virshNodeDeviceEventData data; + const char *eventName = NULL; + int event; + virshControlPtr priv = ctl->privData; + + if (vshCommandOptBool(cmd, "list")) { + size_t i; + + for (i = 0; i < VIR_NODE_DEVICE_EVENT_ID_LAST; i++) + vshPrint(ctl, "%s\n", virshNodeDeviceEventIdTypeToString(i)); + return true; + } + + if (vshCommandOptStringReq(ctl, cmd, "event", &eventName) < 0) + return false; + if (!eventName) { + vshError(ctl, "%s", _("either --list or event type is required")); + return false; + } + if ((event = virshNodeDeviceEventIdTypeFromString(eventName)) < 0) { + vshError(ctl, _("unknown event type %s"), eventName); + return false; + } + + data.ctl = ctl; + data.loop = vshCommandOptBool(cmd, "loop"); + data.timestamp = vshCommandOptBool(cmd, "timestamp"); + data.count = 0; + if (vshCommandOptTimeoutToMs(ctl, cmd, &timeout) < 0) + return false; + + if (vshCommandOptBool(cmd, "node device")) + dev = virshCommandOptNodeDevice(ctl, cmd, NULL); + if (vshEventStart(ctl, timeout) < 0) + goto cleanup; + + if ((eventId = virConnectNodeDeviceEventRegisterAny(priv->conn, dev, event, + VIR_NODE_DEVICE_EVENT_CALLBACK(vshEventLifecyclePrint), + &data, NULL)) < 0) + goto cleanup; + switch (vshEventWait(ctl)) { + case VSH_EVENT_INTERRUPT: + vshPrint(ctl, "%s", _("event loop interrupted\n")); + break; + case VSH_EVENT_TIMEOUT: + vshPrint(ctl, "%s", _("event loop timed out\n")); + break; + case VSH_EVENT_DONE: + break; + default: + goto cleanup; + } + vshPrint(ctl, _("events received: %d\n"), data.count); + if (data.count) + ret = true; + + cleanup: + vshEventCleanup(ctl); + if (eventId >= 0 && + virConnectNodeDeviceEventDeregisterAny(priv->conn, eventId) < 0) + ret = false; + if (dev) + virNodeDeviceFree(dev); + return ret; +} + + const vshCmdDef nodedevCmds[] = { {.name = "nodedev-create", .handler = cmdNodeDeviceCreate, @@ -788,5 +993,11 @@ const vshCmdDef nodedevCmds[] = { .info = info_node_device_reset, .flags = 0 }, + {.name = "nodedev-event", + .handler = cmdNodeDeviceEvent, + .opts = opts_node_device_event, + .info = info_node_device_event, + .flags = 0 + }, {.name = NULL} }; diff --git a/tools/virsh-nodedev.h b/tools/virsh-nodedev.h index c64f7df..b6b2e02 100644 --- a/tools/virsh-nodedev.h +++ b/tools/virsh-nodedev.h @@ -28,6 +28,16 @@
# include "virsh.h"
+virNodeDevicePtr +virshCommandOptNodeDeviceBy(vshControl *ctl, const vshCmd *cmd, + const char **name, unsigned int flags); + +/* default is lookup by Name */ +# define virshCommandOptNodeDevice(_ctl, _cmd, _name) \ + virshCommandOptNodeDeviceBy(_ctl, _cmd, _name, \ + VIRSH_BYNAME) + +
All this OptNodeDevice stuff isn't required... we use that pattern for doms/pools since there are multiple ways you can specify an object (like name, or uuid, etc). For nodedev commands, we only accept the nodedev name. So you can drop all this and just use virNodeDeviceLookupByName like is used for other commands. Looks fine otherwise Thanks, Cole
participants (3)
-
Cole Robinson
-
Daniel P. Berrange
-
Jovanka Gulicoska