[libvirt] [PATCH 1/6] Introduce storage lifecycle event APIs
Cole Robinson
crobinso at redhat.com
Thu Jun 9 21:43:05 UTC 2016
On 06/09/2016 02:25 PM, Jovanka Gulicoska wrote:
> virConnectStoragePoolEventRegisterAny:
> Adds a callback to receive notifications of storage pool events
> occurring on a storage pool. Returns a positive integer identifier for
> the callback
>
> virConnectStoragePoolEventDeregisterAny:
> Removes an event callback
>
> virConnectStoragePoolEventGenericCallback:
> A generic storage pool event callback handler, for use with
> virStoragePoolEventRegisterAny().
>
> VIR_STORAGE_POOL_EVENT_CALLBACK:
> Used to cast the event specific callback into the generic one
> for use for virConnectionStoragePoolEventRegisterAny()
>
> virStoragePoolEventID:
> An enumeration of supported eventIDs for virStoragePoolEventRegisterAny()
>
> virStoragePoolEventLifecycleType (VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE):
> emitting during storage pool lifecycle events
>
> virConnectStoragePoolEventLifecycleCallback:
> This callback occurs when pool is started, stopped or refreshed. Used
> when registering an event of type VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE with
> virStoragePoolEvenTrgisterAny()
> ---
> include/libvirt/libvirt-storage.h | 94 +++++++++++++++++++++++++++++
> src/datatypes.h | 13 ++++
> src/driver-storage.h | 14 +++++
> src/libvirt-storage.c | 122 ++++++++++++++++++++++++++++++++++++++
> src/libvirt_public.syms | 7 +++
> 5 files changed, 250 insertions(+)
>
> diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h
> index db6f2b4..cfbdedf 100644
> --- a/include/libvirt/libvirt-storage.h
> +++ b/include/libvirt/libvirt-storage.h
> @@ -377,5 +377,99 @@ int virStorageVolResize (virStorageVolPtr vol,
> int virStoragePoolIsActive(virStoragePoolPtr pool);
> int virStoragePoolIsPersistent(virStoragePoolPtr pool);
>
> +/**
> + * VIR_STORAGE_POOL_EVENT_CALLBACK:
> + *
> + * Used to cast the event specific callback into the generic one
> + * for use for virConnectStoragePoolEventRegisterAny()
> + */
> +# define VIR_STORAGE_POOL_EVENT_CALLBACK(cb) ((virConnectStoragePoolEventGenericCallback)(cb))
> +
There's an extra space here ') ('
> +/**
> + * virStoragePoolEventID:
> + *
> + * An enumeration of supported eventId parameters for
> + * virConnectStoragePoolEventRegisterAny(). Each event id determines which
> + * signature of callback function will be used.
> + */
> +typedef enum {
> + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE = 0, /* virConnectStoragePoolEventLifecycleCallback */
> +
> +# ifdef VIR_ENUM_SENTINELS
> + VIR_STORAGE_POOL_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
> +} virStoragePoolEventID;
> +
> +/**
> + * virConnectStoragePoolEventGenericCallback:
> + * @conn: the connection pointer
> + * @pool: the pool pointer
> + * @opaque: application specified data
> + *
> + * A generic storage pool event callback handler, for use with
> + * virConnectStoragePoolEventRegisterAny().
> + * Spcific events usually have a customization with extra paramenters,
> + * often with @opaque being passed in a different parameter possition;
*Specific *parameters *position
> + * use VIR_STORAGE_POOL_EVENT_CALLBACK() when registering an
> + * appropriate handler.
> + */
> +typedef void (*virConnectStoragePoolEventGenericCallback)(virConnectPtr conn,
> + virStoragePool pool,
This should be virStoragePoolPtr pool
> + void *opaque);
> +
> +/* Use VIR_STORAGE_POOL_EVENT_CALLBACK() to cast the 'cb' parameter */
> +int virConnectStoragePoolEventRegisterAny(virConnectPtr conn,
> + virStoragePoolPtr pool, /* optional, to filter */
> + int eventID,
> + virConnectStoragePoolEventGenericCallback cb,
> + void *opaque,
> + virFreeCallback freecb);
Indentation is off here, please align these lines with the open parentheses
> +
> +int virConnectStoragePoolEventDeregisterAny(virConnectPtr conn,
> + int callbackID);
> +
> +/**
> + * virStoragePoolEventLifecycleType:
> + *
> + * a virStoragePoolEventLifecycleType is emitted during storage pool
> + * lifecycle events
> + */
> +typedef enum {
> + VIR_STORAGE_POOL_EVENT_DEFINED = 0,
> + VIR_STORAGE_POOL_EVENT_UNDEFINED = 1,
> + VIR_STORAGE_POOL_EVENT_STARTED = 2,
> + VIR_STORAGE_POOL_EVENT_STOPPED = 3,
> + VIR_STORAGE_POOL_EVENT_REFRESHED = 4,
> +
> +# ifdef VIR_ENUM_SENTINELS
> + VIR_STORAGE_POOL_EVENT_LAST
> +# endif
> +} virStoragePoolEventLifecycleType;
> +
> +/**
> + * virConnectStoragePoolEventLifecycleCallback:
> + * @conn: connection object
> + * @pool: pool on which the event occurred
> + * @event: The specific virStoragePoolEventLifeCycleType which occurred
> + * @detail: contains some details on the reason of the event.
> + * It will be 0 for the while.
I know this mirrors the network event comment, but maybe just change the
phrasing to
@detail: may contain some details on the reason for the event.
and drop the 'It will be 0...'
> + * @opaque: application specified data
> + *
> + * This callback occurs when the pool is started, stopped or refreshed.
> + *
This isn't wholly accurate. Maybe like:
This callback is called when a pool lifecycle action is performed, like start
or stop.
> + * The callback signature to use when registering for an event of type
> + * VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE with
> + * virConnectStoragePoolEventRegisterAny()
> + */
> +typedef void (*virConnectStoragePoolEventLifecycleCallback)(virConnectPtr conn,
> + virStoragePoolPtr pool,
> + int event,
> + int detail,
> + void *opaque);
>
> #endif /* __VIR_LIBVIRT_STORAGE_H__ */
> diff --git a/src/datatypes.h b/src/datatypes.h
> index 8ccc7b0..638bd23 100644
> --- a/src/datatypes.h
> +++ b/src/datatypes.h
> @@ -143,6 +143,19 @@ extern virClassPtr virAdmClientClass;
> } \
> } while (0)
>
> +# define virCheckStoragePoolGoto(obj, label) \
> + do { \
> + virStoragePoolPtr _pool= (obj); \
> + if (!virObjectIsClass(_pool, virStoragePoolClass) || \
> + !virObjectIsClass(_pool->conn, virConnectClass)) { \
> + virReportErrorHelper(VIR_FROM_STORAGE, \
> + VIR_ERR_INVALID_STORAGE_POOL, \
> + __FILE__, __FUNCTION__, __LINE__, \
> + __FUNCTION__); \
> + goto label; \
> + } \
> + } while (0)
> +
> # define virCheckStorageVolReturn(obj, retval) \
> do { \
> virStorageVolPtr _vol = (obj); \
> diff --git a/src/driver-storage.h b/src/driver-storage.h
> index 0489647..70a1dcc 100644
> --- a/src/driver-storage.h
> +++ b/src/driver-storage.h
> @@ -196,6 +196,18 @@ typedef int
> typedef int
> (*virDrvStoragePoolIsPersistent)(virStoragePoolPtr pool);
>
> +typedef int
> +(*virDrvConnectStoragePoolEventRegisterAny)(virConnectPtr conn,
> + virStoragePoolPtr pool,
> + int eventID,
> + virConnectStoragePoolEventGenericCallback cb,
> + void *opaque,
> + virFreeCallback freecb);
> +
> +typedef int
> +(*virDrvConnectStoragePoolEventDeregisterAny)(virConnectPtr conn,
> + int callbackID);
> +
>
>
> typedef struct _virStorageDriver virStorageDriver;
> @@ -215,6 +227,8 @@ struct _virStorageDriver {
> virDrvConnectListDefinedStoragePools connectListDefinedStoragePools;
> virDrvConnectListAllStoragePools connectListAllStoragePools;
> virDrvConnectFindStoragePoolSources connectFindStoragePoolSources;
> + virDrvConnectStoragePoolEventRegisterAny connectStoragePoolEventRegisterAny;
> + virDrvConnectStoragePoolEventDeregisterAny connectStoragePoolEventDeregisterAny;
> virDrvStoragePoolLookupByName storagePoolLookupByName;
> virDrvStoragePoolLookupByUUID storagePoolLookupByUUID;
> virDrvStoragePoolLookupByVolume storagePoolLookupByVolume;
> diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
> index 1ce6745..33362ea 100644
> --- a/src/libvirt-storage.c
> +++ b/src/libvirt-storage.c
> @@ -2124,3 +2124,125 @@ virStoragePoolIsPersistent(virStoragePoolPtr pool)
> virDispatchError(pool->conn);
> return -1;
> }
> +
> +/**
> + * virConnectStoragePoolEventRegisterAny:
> + * @conn: pointer to the connection
> + * @pool: pointer to the storage pool
> + * @eventID: the event type to receive
> + * @cb: callback to the function handling network 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 storage pool events
> + * occurring on a storage pool. This function requires that an event loop
> + * has been previously registered with virEventRegisterImpl() or
> + * virEventRegisterDefaultImpl().
> + *
> + * If @pool is NULL, then events will be monitored for any storage pool.
> + * If @pool is non-NULL, then only the specific storage pool will be monitored.
> + *
> + * Most types of event have a callback providing a custom set of parameters
types of events
> + * for the event. When registering an event, it is thus necessary to use
> + * the VIR_STORAGE_POOL_EVENT_CALLBACK() macro to cast the supplied function pointer
this line is too long, move 'pointer' to the next line
> + * to match the signature of this method.
> + *
> + * The virStoragePoolPtr 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 storage pool object after the callback
> + * returns, it shall take a reference to it, by calling virStoragePoolRef().
> + * The reference can be released once the object is no longer required
> + * by calling virStoragePoolFree().
> + *
> + * 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 virConnectStoragePoolEventDeregisterAny() method.
> + *
> + * Returns a callback identifier on success, -1 on failure.
> + */
> +int
> +virConnectStoragePoolEventRegisterAny(virConnectPtr conn,
> + virStoragePoolPtr pool,
> + int eventID,
> + virConnectStoragePoolEventGenericCallback cb,
> + void *opaque,
> + virFreeCallback freecb)
> +{
> + VIR_DEBUG("conn=%p, eventID=%d, cb=%p, opaque=%p, freecb=%p",
> + conn, eventID, cb, opaque, freecb);
> +
this should have pool=%p as well
> + virResetLastError();
> +
> + virCheckConnectReturn(conn, -1);
> + if (pool) {
> + virCheckStoragePoolGoto(pool, error);
> + if (pool->conn != conn) {
> + virReportInvalidArg(pool,
> + _("storage pool'%s' in %s must match connection"),
Need a space after 'pool'. This is a long line, but since it's in an error
message I say just leave it for grepability
> + pool->name, __FUNCTION__);
> + goto error;
> + }
> + }
> + virCheckNonNullArgGoto(cb, error);
> + virCheckNonNegativeArgGoto(eventID, error);
> +
> + if (eventID >= VIR_STORAGE_POOL_EVENT_ID_LAST) {
> + virReportInvalidArg(eventID,
> + _("eventID in %s must be less than %d"),
> + __FUNCTION__, VIR_STORAGE_POOL_EVENT_ID_LAST);
> + goto error;
> + }
> +
> + if (conn->storageDriver && conn->storageDriver->connectStoragePoolEventRegisterAny) {
Long line, split it after the &&
> + int ret;
> + ret = conn->storageDriver->connectStoragePoolEventRegisterAny(conn, pool,
> + eventID,
> + cb, opaque,
> + freecb);
If you put pool and opaque on their own lines this breaks some over 80 column
lines, but I don't know if that's better or worse
> + if (ret < 0)
> + goto error;
> + return ret;
> + }
> +
> + virReportUnsupportedError();
> + error:
> + virDispatchError(conn);
> + return -1;
> +}
> +
> +/**
> + * virConnectStoragePoolEventDeregisterAny:
> + * @conn: pointer to the connection
> + * @callbackID: the callback identifier
> + *
> + * Removes an event callback. The callbackID parameter should be the
> + * value obtained from a previous virConnectStoragePoolEventRegisterAny() method.
> + *
> + * Returns 0 on success, -1 on failure
> + */
> +int
> +virConnectStoragePoolEventDeregisterAny(virConnectPtr conn,
> + int callbackID)
> +{
> + VIR_DEBUG("conn=%p, callbackID=%d", conn, callbackID);
> +
> + virResetLastError();
> +
> + virCheckConnectReturn(conn, -1);
> + virCheckNonNegativeArgGoto(callbackID, error);
> +
> + if (conn->storageDriver &&
> + conn->storageDriver->connectStoragePoolEventDeregisterAny) {
> + int ret;
> + ret = conn->storageDriver->connectStoragePoolEventDeregisterAny(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 1e920d6..f5898ee 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -732,4 +732,11 @@ LIBVIRT_1.3.3 {
> virDomainSetPerfEvents;
> } LIBVIRT_1.2.19;
>
> +
> +LIBVIRT_1.3.6 {
> + global:
> + virConnectStoragePoolEventRegisterAny;
> + virConnectStoragePoolEventDeregisterAny;
> +} LIBVIRT_1.3.3;
> +
> # .... define new API here using predicted next version number ....
>
Aside from those issues this looks fine to me
Thanks,
Cole
More information about the libvir-list
mailing list