[libvirt] [PATCH v2 0/6] Introducing storage pool lifecycle event APIs

Changes follow comments on v1 patches. Introducing implementation of storage pool event APIs. Code changes follow network event APIs. Implemented functions: virStoragePoolEventRegisterAny(), virStoragePoolEventDeregisterAny(), virStoragePoolLifeCycleEventNew(), introduced STARTED, STOPPED, DEFINE, UNDEFINE and REFRESHED. STARTED signal is emiited in storagePoolCreateXML() and storagePoolCreate() DEFINED signal is emitted in storagePoolDefineXML() UNDEFINED signal is emitted in storagePoolUndefine() STOPPED signal is emitted in storagePoolDestroy() and storagePoolRefresh() REFRESHED signal is emitted in storagePoolRefresh() 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 As part of the project there should also be implementation for storage volume events. For now there's no signal when volumes are created or deleted, they can also be implemented, but probably the easiest way is to have apps watch for REFRESH signal, and later extend storage driver code to refresh a pool after volume APIs are called. Jovanka Gulicoska (6): Introduce storage lifecycle event APIs conf: add storage_event handling test: implement storage lifecycle event APIs remote: implement storage lifecycle event APIs storage: implement storage lifecycle event APIs event-test: support storage lifecycle event APIs daemon/libvirtd.h | 2 + daemon/remote.c | 201 +++++++++++++++++++++++++++++- examples/object-events/event-test.c | 46 ++++++- include/libvirt/libvirt-storage.h | 94 ++++++++++++++ src/Makefile.am | 5 + src/conf/storage_conf.h | 4 + src/conf/storage_event.c | 237 ++++++++++++++++++++++++++++++++++++ src/conf/storage_event.h | 60 +++++++++ src/datatypes.h | 13 ++ src/driver-storage.h | 14 +++ src/libvirt-storage.c | 125 +++++++++++++++++++ src/libvirt_private.syms | 5 + src/libvirt_public.syms | 7 ++ src/remote/remote_driver.c | 128 +++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++- src/remote_protocol-structs | 16 +++ src/storage/storage_driver.c | 110 +++++++++++++++++ src/test/test_driver.c | 71 +++++++++++ tests/objecteventtest.c | 177 +++++++++++++++++++++++++++ 19 files changed, 1355 insertions(+), 3 deletions(-) create mode 100644 src/conf/storage_event.c create mode 100644 src/conf/storage_event.h -- 2.5.5

Storage pool lifecycle event API entry points for registering and deregistering storage pool events, as well as types of events associated with storage pools. These entry points will be used for implementing asynchronous lifecycle events. Storage pool API: virConnectStoragePoolEventRegisterAny virConnectStoragePoolEventDeregisterAny virStoragePoolEventLifecycleType which has events STARTED, STOPPED, DEFINED, UNDEFINED, and REFRESHED --- include/libvirt/libvirt-storage.h | 94 ++++++++++++++++++++++++++++ src/datatypes.h | 13 ++++ src/driver-storage.h | 14 +++++ src/libvirt-storage.c | 125 ++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 7 +++ 5 files changed, 253 insertions(+) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index db6f2b4..a744e95 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)) + +/** + * 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 specific parameter possition; + * use VIR_STORAGE_POOL_EVENT_CALLBACK() when registering an + * appropriate handler. + */ +typedef void (*virConnectStoragePoolEventGenericCallback)(virConnectPtr conn, + 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); + +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. + * @opaque: application specified data + * + * 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..cebe02f 100644 --- a/src/libvirt-storage.c +++ b/src/libvirt-storage.c @@ -2124,3 +2124,128 @@ 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 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_STORAGE_POOL_EVENT_CALLBACK() macro to cast the + * supplied function pointer 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, pool=%p, eventID=%d, cb=%p, opaque=%p, freecb=%p", + conn, pool, eventID, cb, opaque, freecb); + + virResetLastError(); + + virCheckConnectReturn(conn, -1); + if (pool) { + virCheckStoragePoolGoto(pool, error); + if (pool->conn != conn) { + virReportInvalidArg(pool, + _("storage pool '%s' in %s must match connection"), + 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) { + int ret; + ret = conn->storageDriver->connectStoragePoolEventRegisterAny(conn, + pool, + eventID, + cb, + opaque, + freecb); + 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 .... -- 2.5.5

On Mon, Jun 13, 2016 at 18:38:38 +0200, Jovanka Gulicoska wrote:
Storage pool lifecycle event API entry points for registering and deregistering storage pool events, as well as types of events associated with storage pools. These entry points will be used for implementing asynchronous lifecycle events.
Storage pool API: virConnectStoragePoolEventRegisterAny virConnectStoragePoolEventDeregisterAny virStoragePoolEventLifecycleType which has events STARTED, STOPPED, DEFINED, UNDEFINED, and REFRESHED --- include/libvirt/libvirt-storage.h | 94 ++++++++++++++++++++++++++++ src/datatypes.h | 13 ++++ src/driver-storage.h | 14 +++++ src/libvirt-storage.c | 125 ++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 7 +++ 5 files changed, 253 insertions(+)
[...]
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); \
Broken alignment
+ 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); \

Add storage event handling infrastructure to storage_event.[ch], following the network_event.[ch] pattern. --- src/Makefile.am | 5 + src/conf/storage_conf.h | 4 + src/conf/storage_event.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/storage_event.h | 60 ++++++++++++ src/libvirt_private.syms | 5 + 5 files changed, 311 insertions(+) create mode 100644 src/conf/storage_event.c create mode 100644 src/conf/storage_event.h diff --git a/src/Makefile.am b/src/Makefile.am index ee4a7bf..cb9bf9b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -342,6 +342,9 @@ DOMAIN_EVENT_SOURCES = \ NETWORK_EVENT_SOURCES = \ conf/network_event.c conf/network_event.h +STORAGE_POOL_EVENT_SOURCES = \ + conf/storage_event.c conf/storage_event.h + # Network driver generic impl APIs NETWORK_CONF_SOURCES = \ conf/network_conf.c conf/network_conf.h \ @@ -392,6 +395,7 @@ CONF_SOURCES = \ $(OBJECT_EVENT_SOURCES) \ $(DOMAIN_EVENT_SOURCES) \ $(NETWORK_EVENT_SOURCES) \ + $(STORAGE_POOL_EVENT_SOURCES) \ $(NETWORK_CONF_SOURCES) \ $(NWFILTER_CONF_SOURCES) \ $(NODE_DEVICE_CONF_SOURCES) \ @@ -2358,6 +2362,7 @@ libvirt_setuid_rpc_client_la_SOURCES = \ conf/domain_event.c \ conf/network_event.c \ conf/object_event.c \ + conf/storage_event.c \ rpc/virnetsocket.c \ rpc/virnetsocket.h \ rpc/virnetmessage.h \ diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 54116a6..185ae5e 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -31,6 +31,7 @@ # include "virthread.h" # include "device_conf.h" # include "node_device_conf.h" +# include "object_event.h" # include <libxml/tree.h> @@ -296,6 +297,9 @@ struct _virStorageDriverState { char *autostartDir; char *stateDir; bool privileged; + + /* Immutable pointer, self-locking APIs */ + virObjectEventStatePtr storageEventState; }; typedef struct _virStoragePoolSourceList virStoragePoolSourceList; diff --git a/src/conf/storage_event.c b/src/conf/storage_event.c new file mode 100644 index 0000000..c5688be --- /dev/null +++ b/src/conf/storage_event.c @@ -0,0 +1,237 @@ +/* + * storage_event.c: storage 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 "storage_event.h" +#include "object_event.h" +#include "object_event_private.h" +#include "datatypes.h" +#include "virlog.h" + +VIR_LOG_INIT("conf.storage_event"); + +struct _virStoragePoolEvent { + virObjectEvent parent; + + /* Unused attribute to allow for subclass creation */ + bool dummy; +}; +typedef struct _virStoragePoolEvent virStoragePoolEvent; +typedef virStoragePoolEvent *virStoragePoolEventPtr; + +struct _virStoragePoolEventLifecycle { + virStoragePoolEvent parent; + + int type; + int detail; +}; +typedef struct _virStoragePoolEventLifecycle virStoragePoolEventLifecycle; +typedef virStoragePoolEventLifecycle *virStoragePoolEventLifecyclePtr; + +static virClassPtr virStoragePoolEventClass; +static virClassPtr virStoragePoolEventLifecycleClass; +static void virStoragePoolEventDispose(void *obj); +static void virStoragePoolEventLifecycleDispose(void *obj); + +static int +virStoragePoolEventsOnceInit(void) +{ + if (!(virStoragePoolEventClass = + virClassNew(virClassForObjectEvent(), + "virStoragePoolEvent", + sizeof(virStoragePoolEvent), + virStoragePoolEventDispose))) + return -1; + if (!(virStoragePoolEventLifecycleClass = + virClassNew(virStoragePoolEventClass, + "virStoragePoolEventLifecycle", + sizeof(virStoragePoolEventLifecycle), + virStoragePoolEventLifecycleDispose))) + return -1; + return 0; +} + +VIR_ONCE_GLOBAL_INIT(virStoragePoolEvents) + +static void +virStoragePoolEventDispose(void *obj) +{ + virStoragePoolEventPtr event = obj; + VIR_DEBUG("obj=%p", event); +} + + +static void +virStoragePoolEventLifecycleDispose(void *obj) +{ + virStoragePoolEventLifecyclePtr event = obj; + VIR_DEBUG("obj=%p", event); +} + + +static void +virStoragePoolEventDispatchDefaultFunc(virConnectPtr conn, + virObjectEventPtr event, + virConnectObjectEventGenericCallback cb, + void *cbopaque) +{ + virStoragePoolPtr pool = virGetStoragePool(conn, + event->meta.name, + event->meta.uuid, + NULL, NULL); + if (!pool) + return; + + switch ((virStoragePoolEventID)event->eventID) { + case VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE: + { + virStoragePoolEventLifecyclePtr storagePoolLifecycleEvent; + + storagePoolLifecycleEvent = (virStoragePoolEventLifecyclePtr)event; + ((virConnectStoragePoolEventLifecycleCallback)cb)(conn, pool, + storagePoolLifecycleEvent->type, + storagePoolLifecycleEvent->detail, + cbopaque); + goto cleanup; + } + + case VIR_STORAGE_POOL_EVENT_ID_LAST: + break; + } + VIR_WARN("Unexpected event ID %d", event->eventID); + + cleanup: + virObjectUnref(pool); +} + + +/** + * virStoragePoolEventStateRegisterID: + * @conn: connection to associate with callback + * @state: object event state + * @pool: storage pool to filter on or NULL for all storage pools + * @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 +virStoragePoolEventStateRegisterID(virConnectPtr conn, + virObjectEventStatePtr state, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) +{ + if (virStoragePoolEventsInitialize() < 0) + return -1; + + return virObjectEventStateRegisterID(conn, state, pool ? pool->uuid : NULL, + NULL, NULL, + virStoragePoolEventClass, eventID, + VIR_OBJECT_EVENT_CALLBACK(cb), + opaque, freecb, + false, callbackID, false); +} + + +/** + * virStoragePoolEventStateRegisterClient: + * @conn: connection to associate with callback + * @state: object event state + * @pool: storage pool to filter on or NULL for all storage pools + * @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 +virStoragePoolEventStateRegisterClient(virConnectPtr conn, + virObjectEventStatePtr state, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) +{ + if (virStoragePoolEventsInitialize() < 0) + return -1; + + return virObjectEventStateRegisterID(conn, state, pool ? pool->uuid : NULL, + NULL, NULL, + virStoragePoolEventClass, eventID, + VIR_OBJECT_EVENT_CALLBACK(cb), + opaque, freecb, + false, callbackID, true); +} + + +/** + * virStoragePoolEventLifecycleNew: + * @name: name of the storage pool object the event describes + * @uuid: uuid of the storage pool object the event describes + * @type: type of lifecycle event + * @detail: more details about @type + * + * Create a new storage pool lifecycle event. + */ +virObjectEventPtr +virStoragePoolEventLifecycleNew(const char *name, + const unsigned char *uuid, + int type, + int detail) +{ + virStoragePoolEventLifecyclePtr event; + + if (virStoragePoolEventsInitialize() < 0) + return NULL; + + if (!(event = virObjectEventNew(virStoragePoolEventLifecycleClass, + virStoragePoolEventDispatchDefaultFunc, + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE, + 0, name, uuid))) + return NULL; + + event->type = type; + event->detail = detail; + + return (virObjectEventPtr)event; +} diff --git a/src/conf/storage_event.h b/src/conf/storage_event.h new file mode 100644 index 0000000..6f244be --- /dev/null +++ b/src/conf/storage_event.h @@ -0,0 +1,60 @@ +/* + * storage_event.h: storage 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 __STORAGE_POOL_EVENT_H__ +# define __STORAGE_POOL_EVENT_H__ + +int +virStoragePoolEventStateRegisterID(virConnectPtr conn, + virObjectEventStatePtr state, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) + ATTRIBUTE_NONNULL(8); + +int +virStoragePoolEventStateRegisterClient(virConnectPtr conn, + virObjectEventStatePtr state, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback cb, + void *opaque, + virFreeCallback freecb, + int *callbackID) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5) + ATTRIBUTE_NONNULL(8); + +virObjectEventPtr +virStoragePoolEventLifecycleNew(const char *name, + const unsigned char *uuid, + int type, + int detail); + +#endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b42e2cd..83f6834 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -876,6 +876,11 @@ virStorageVolTypeFromString; virStorageVolTypeToString; +# conf/storage_event.h +virStoragePoolEventLifecycleNew; +virStoragePoolEventStateRegisterID; + + # conf/virchrdev.h virChrdevAlloc; virChrdevFree; -- 2.5.5

On Mon, Jun 13, 2016 at 18:38:39 +0200, Jovanka Gulicoska wrote:
Add storage event handling infrastructure to storage_event.[ch], following the network_event.[ch] pattern. --- src/Makefile.am | 5 + src/conf/storage_conf.h | 4 + src/conf/storage_event.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/storage_event.h | 60 ++++++++++++ src/libvirt_private.syms | 5 + 5 files changed, 311 insertions(+) create mode 100644 src/conf/storage_event.c create mode 100644 src/conf/storage_event.h
diff --git a/src/Makefile.am b/src/Makefile.am index ee4a7bf..cb9bf9b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -342,6 +342,9 @@ DOMAIN_EVENT_SOURCES = \ NETWORK_EVENT_SOURCES = \ conf/network_event.c conf/network_event.h
+STORAGE_POOL_EVENT_SOURCES = \
Variable name contains "POOL"
+ conf/storage_event.c conf/storage_event.h
but files don't
+ # Network driver generic impl APIs NETWORK_CONF_SOURCES = \ conf/network_conf.c conf/network_conf.h \
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 54116a6..185ae5e 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -31,6 +31,7 @@ # include "virthread.h" # include "device_conf.h" # include "node_device_conf.h" +# include "object_event.h"
# include <libxml/tree.h>
@@ -296,6 +297,9 @@ struct _virStorageDriverState { char *autostartDir; char *stateDir; bool privileged; + + /* Immutable pointer, self-locking APIs */ + virObjectEventStatePtr storageEventState;
This isn't used anywhere as of this patch.
};
typedef struct _virStoragePoolSourceList virStoragePoolSourceList; diff --git a/src/conf/storage_event.c b/src/conf/storage_event.c new file mode 100644 index 0000000..c5688be --- /dev/null +++ b/src/conf/storage_event.c @@ -0,0 +1,237 @@ +/* + * storage_event.c: storage 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 copyright was copied somewhere but not updated/cleaned up.
+ * 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/>. +*/
[...]
diff --git a/src/conf/storage_event.h b/src/conf/storage_event.h new file mode 100644 index 0000000..6f244be --- /dev/null +++ b/src/conf/storage_event.h @@ -0,0 +1,60 @@ +/* + * storage_event.h: storage event queue processing helpers + * + * Copyright (C) 2010-2014 Red Hat, Inc. + * Copyright (C) 2008 VirtualIron + * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
Same as above regarding copyright.
+ * + * 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 __STORAGE_POOL_EVENT_H__
Macro name contains POOL but file name doesn't
+# define __STORAGE_POOL_EVENT_H__ +

Hi, first thanks for your comments. I have one question regarding the copyright in files: storage_event.[ch], since they are copy of network_event.[ch], with changes needed for storage pool lifecycle events. Which way will be the best to handle the copyright in these files? Thanks, Jovanka On Tue, Jun 14, 2016 at 6:49 AM, Peter Krempa <pkrempa@redhat.com> wrote:
On Mon, Jun 13, 2016 at 18:38:39 +0200, Jovanka Gulicoska wrote:
Add storage event handling infrastructure to storage_event.[ch], following the network_event.[ch] pattern. --- src/Makefile.am | 5 + src/conf/storage_conf.h | 4 + src/conf/storage_event.c | 237 +++++++++++++++++++++++++++++++++++++++++++++++ src/conf/storage_event.h | 60 ++++++++++++ src/libvirt_private.syms | 5 + 5 files changed, 311 insertions(+) create mode 100644 src/conf/storage_event.c create mode 100644 src/conf/storage_event.h
diff --git a/src/Makefile.am b/src/Makefile.am index ee4a7bf..cb9bf9b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -342,6 +342,9 @@ DOMAIN_EVENT_SOURCES = \ NETWORK_EVENT_SOURCES = \ conf/network_event.c conf/network_event.h
+STORAGE_POOL_EVENT_SOURCES = \
Variable name contains "POOL"
+ conf/storage_event.c conf/storage_event.h
but files don't
+ # Network driver generic impl APIs NETWORK_CONF_SOURCES = \ conf/network_conf.c conf/network_conf.h \
diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 54116a6..185ae5e 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -31,6 +31,7 @@ # include "virthread.h" # include "device_conf.h" # include "node_device_conf.h" +# include "object_event.h"
# include <libxml/tree.h>
@@ -296,6 +297,9 @@ struct _virStorageDriverState { char *autostartDir; char *stateDir; bool privileged; + + /* Immutable pointer, self-locking APIs */ + virObjectEventStatePtr storageEventState;
This isn't used anywhere as of this patch.
};
typedef struct _virStoragePoolSourceList virStoragePoolSourceList; diff --git a/src/conf/storage_event.c b/src/conf/storage_event.c new file mode 100644 index 0000000..c5688be --- /dev/null +++ b/src/conf/storage_event.c @@ -0,0 +1,237 @@ +/* + * storage_event.c: storage 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 copyright was copied somewhere but not updated/cleaned up.
+ * 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/>. +*/
[...]
diff --git a/src/conf/storage_event.h b/src/conf/storage_event.h new file mode 100644 index 0000000..6f244be --- /dev/null +++ b/src/conf/storage_event.h @@ -0,0 +1,60 @@ +/* + * storage_event.h: storage event queue processing helpers + * + * Copyright (C) 2010-2014 Red Hat, Inc. + * Copyright (C) 2008 VirtualIron + * Copyright (C) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
Same as above regarding copyright.
+ * + * 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 __STORAGE_POOL_EVENT_H__
Macro name contains POOL but file name doesn't
+# define __STORAGE_POOL_EVENT_H__ +

Also includes unittests for storage pool lifecycle events API --- src/test/test_driver.c | 71 +++++++++++++++++++ tests/objecteventtest.c | 177 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 248 insertions(+) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 6ab939a..3b1e004 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -49,6 +49,7 @@ #include "snapshot_conf.h" #include "fdstream.h" #include "storage_conf.h" +#include "storage_event.h" #include "node_device_conf.h" #include "virxml.h" #include "virthread.h" @@ -4114,6 +4115,7 @@ testStoragePoolCreate(virStoragePoolPtr pool, testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr privpool; int ret = -1; + virObjectEventPtr event = NULL; virCheckFlags(0, -1); @@ -4134,9 +4136,14 @@ testStoragePoolCreate(virStoragePoolPtr pool, } privpool->active = 1; + + event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid, + VIR_STORAGE_POOL_EVENT_STARTED, + 0); ret = 0; cleanup: + testObjectEventQueue(privconn, event); if (privpool) virStoragePoolObjUnlock(privpool); return ret; @@ -4204,6 +4211,7 @@ testStoragePoolCreateXML(virConnectPtr conn, virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; + virObjectEventPtr event = NULL; virCheckFlags(0, NULL); @@ -4231,11 +4239,16 @@ testStoragePoolCreateXML(virConnectPtr conn, } pool->active = 1; + event = virStoragePoolEventLifecycleNew(pool->def->name, pool->def->uuid, + VIR_STORAGE_POOL_EVENT_STARTED, + 0); + ret = virGetStoragePool(conn, pool->def->name, pool->def->uuid, NULL, NULL); cleanup: virStoragePoolDefFree(def); + testObjectEventQueue(privconn, event); if (pool) virStoragePoolObjUnlock(pool); testDriverUnlock(privconn); @@ -4251,6 +4264,7 @@ testStoragePoolDefineXML(virConnectPtr conn, virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; + virObjectEventPtr event = NULL; virCheckFlags(0, NULL); @@ -4266,6 +4280,10 @@ testStoragePoolDefineXML(virConnectPtr conn, goto cleanup; def = NULL; + event = virStoragePoolEventLifecycleNew(pool->def->name, pool->def->uuid, + VIR_STORAGE_POOL_EVENT_DEFINED, + 0); + if (testStoragePoolObjSetDefaults(pool) == -1) { virStoragePoolObjRemove(&privconn->pools, pool); pool = NULL; @@ -4277,6 +4295,7 @@ testStoragePoolDefineXML(virConnectPtr conn, cleanup: virStoragePoolDefFree(def); + testObjectEventQueue(privconn, event); if (pool) virStoragePoolObjUnlock(pool); testDriverUnlock(privconn); @@ -4289,6 +4308,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool) testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr privpool; int ret = -1; + virObjectEventPtr event = NULL; testDriverLock(privconn); privpool = virStoragePoolObjFindByName(&privconn->pools, @@ -4305,6 +4325,10 @@ testStoragePoolUndefine(virStoragePoolPtr pool) goto cleanup; } + event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid, + VIR_STORAGE_POOL_EVENT_UNDEFINED, + 0); + virStoragePoolObjRemove(&privconn->pools, privpool); privpool = NULL; ret = 0; @@ -4312,6 +4336,7 @@ testStoragePoolUndefine(virStoragePoolPtr pool) cleanup: if (privpool) virStoragePoolObjUnlock(privpool); + testObjectEventQueue(privconn, event); testDriverUnlock(privconn); return ret; } @@ -4356,6 +4381,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool) testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr privpool; int ret = -1; + virObjectEventPtr event = NULL; testDriverLock(privconn); privpool = virStoragePoolObjFindByName(&privconn->pools, @@ -4373,6 +4399,9 @@ testStoragePoolDestroy(virStoragePoolPtr pool) } privpool->active = 0; + event = virStoragePoolEventLifecycleNew(privpool->def->name, privpool->def->uuid, + VIR_STORAGE_POOL_EVENT_STOPPED, + 0); if (privpool->configFile == NULL) { virStoragePoolObjRemove(&privconn->pools, privpool); @@ -4381,6 +4410,7 @@ testStoragePoolDestroy(virStoragePoolPtr pool) ret = 0; cleanup: + testObjectEventQueue(privconn, event); if (privpool) virStoragePoolObjUnlock(privpool); testDriverUnlock(privconn); @@ -4430,6 +4460,7 @@ testStoragePoolRefresh(virStoragePoolPtr pool, testDriverPtr privconn = pool->conn->privateData; virStoragePoolObjPtr privpool; int ret = -1; + virObjectEventPtr event = NULL; virCheckFlags(0, -1); @@ -4448,9 +4479,14 @@ testStoragePoolRefresh(virStoragePoolPtr pool, _("storage pool '%s' is not active"), pool->name); goto cleanup; } + + event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid, + VIR_STORAGE_POOL_EVENT_REFRESHED, + 0); ret = 0; cleanup: + testObjectEventQueue(privconn, event); if (privpool) virStoragePoolObjUnlock(privpool); return ret; @@ -5644,6 +5680,39 @@ testConnectNetworkEventDeregisterAny(virConnectPtr conn, return ret; } +static int +testConnectStoragePoolEventRegisterAny(virConnectPtr conn, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + testDriverPtr driver = conn->privateData; + int ret; + + if (virStoragePoolEventStateRegisterID(conn, driver->eventState, + pool, eventID, callback, + opaque, freecb, &ret) < 0) + ret = -1; + + return ret; +} + +static int +testConnectStoragePoolEventDeregisterAny(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) @@ -6751,6 +6820,8 @@ static virStorageDriver testStorageDriver = { .connectListDefinedStoragePools = testConnectListDefinedStoragePools, /* 0.5.0 */ .connectListAllStoragePools = testConnectListAllStoragePools, /* 0.10.2 */ .connectFindStoragePoolSources = testConnectFindStoragePoolSources, /* 0.5.0 */ + .connectStoragePoolEventRegisterAny = testConnectStoragePoolEventRegisterAny, /*1.3.6*/ + .connectStoragePoolEventDeregisterAny = testConnectStoragePoolEventDeregisterAny, /*1.3.6*/ .storagePoolLookupByName = testStoragePoolLookupByName, /* 0.5.0 */ .storagePoolLookupByUUID = testStoragePoolLookupByUUID, /* 0.5.0 */ .storagePoolLookupByVolume = testStoragePoolLookupByVolume, /* 0.5.0 */ diff --git a/tests/objecteventtest.c b/tests/objecteventtest.c index c77b0cd..ac8cce3 100644 --- a/tests/objecteventtest.c +++ b/tests/objecteventtest.c @@ -53,12 +53,21 @@ static const char networkDef[] = " </ip>\n" "</network>\n"; +static const char storagePoolDef[] = +"<pool type='dir'>\n" +" <name>P</name>\n" +" <target>\n" +" <path>/target-path</path>\n" +" </target>\n" +"</pool>\n"; + typedef struct { int startEvents; int stopEvents; int defineEvents; int undefineEvents; int unexpectedEvents; + int refreshEvents; } lifecycleEventCounter; static void @@ -69,11 +78,13 @@ lifecycleEventCounter_reset(lifecycleEventCounter *counter) counter->defineEvents = 0; counter->undefineEvents = 0; counter->unexpectedEvents = 0; + counter->refreshEvents = 0; } typedef struct { virConnectPtr conn; virNetworkPtr net; + virStoragePoolPtr pool; } objecteventTest; @@ -125,6 +136,26 @@ networkLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED, counter->undefineEvents++; } +static void +storagePoolLifecycleCb(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolPtr pool ATTRIBUTE_UNUSED, + int event, + int detail ATTRIBUTE_UNUSED, + void* opaque) +{ + lifecycleEventCounter *counter = opaque; + + if (event == VIR_STORAGE_POOL_EVENT_STARTED) + counter->startEvents++; + else if (event == VIR_STORAGE_POOL_EVENT_STOPPED) + counter->stopEvents++; + else if (event == VIR_STORAGE_POOL_EVENT_DEFINED) + counter->defineEvents++; + else if (event == VIR_STORAGE_POOL_EVENT_UNDEFINED) + counter->undefineEvents++; + else if (event == VIR_STORAGE_POOL_EVENT_REFRESHED) + counter->refreshEvents++; +} static int testDomainCreateXMLOld(const void *data) @@ -523,6 +554,130 @@ testNetworkStartStopEvent(const void *data) return ret; } +static int +testStoragePoolCreateXML(const void *data) +{ + const objecteventTest *test = data; + lifecycleEventCounter counter; + virStoragePoolPtr pool; + int id; + int ret = 0; + + lifecycleEventCounter_reset(&counter); + + id = virConnectStoragePoolEventRegisterAny(test->conn, NULL, + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE, + VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb), + &counter, NULL); + pool = virStoragePoolCreateXML(test->conn, storagePoolDef, 0); + + if (!pool || virEventRunDefaultImpl() < 0) { + ret = -1; + goto cleanup; + } + + if (counter.startEvents != 1 || counter.unexpectedEvents > 0) { + ret = -1; + goto cleanup; + } + + cleanup: + virConnectStoragePoolEventDeregisterAny(test->conn, id); + if (pool) { + virStoragePoolDestroy(pool); + virStoragePoolFree(pool); + } + return ret; +} + +static int +testStoragePoolDefine(const void *data) +{ + const objecteventTest *test = data; + lifecycleEventCounter counter; + virStoragePoolPtr pool; + int id; + int ret = 0; + + lifecycleEventCounter_reset(&counter); + + id = virConnectStoragePoolEventRegisterAny(test->conn, NULL, + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE, + VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb), + &counter, NULL); + + /* Make sure the define event is triggered */ + pool = virStoragePoolDefineXML(test->conn, storagePoolDef, 0); + + if (!pool || virEventRunDefaultImpl() < 0) { + ret = -1; + goto cleanup; + } + + if (counter.defineEvents != 1 || counter.unexpectedEvents > 0) { + ret = -1; + goto cleanup; + } + + /* Make sure the undefine event is triggered */ + virStoragePoolUndefine(pool); + + if (virEventRunDefaultImpl() < 0) { + ret = -1; + goto cleanup; + } + + if (counter.undefineEvents != 1 || counter.unexpectedEvents > 0) { + ret = -1; + goto cleanup; + } + + + cleanup: + virConnectStoragePoolEventDeregisterAny(test->conn, id); + if (pool) + virStoragePoolFree(pool); + + return ret; +} + +static int +testStoragePoolStartStopEvent(const void *data) +{ + const objecteventTest *test = data; + lifecycleEventCounter counter; + int id; + int ret = 0; + + if (!test->pool) + return -1; + + lifecycleEventCounter_reset(&counter); + + id = virConnectStoragePoolEventRegisterAny(test->conn, test->pool, + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE, + VIR_STORAGE_POOL_EVENT_CALLBACK(&storagePoolLifecycleCb), + &counter, NULL); + virStoragePoolCreate(test->pool, 0); + virStoragePoolRefresh(test->pool, 0); + virStoragePoolDestroy(test->pool); + + if (virEventRunDefaultImpl() < 0) { + ret = -1; + goto cleanup; + } + + if (counter.startEvents != 1 || counter.stopEvents != 1 || + counter.refreshEvents != 1 || counter.unexpectedEvents > 0) { + ret = -1; + goto cleanup; + } + + cleanup: + virConnectStoragePoolEventDeregisterAny(test->conn, id); + return ret; +} + static void timeout(int id ATTRIBUTE_UNUSED, void *opaque ATTRIBUTE_UNUSED) { @@ -581,6 +736,28 @@ mymain(void) virNetworkUndefine(test.net); virNetworkFree(test.net); } + + /* Storage pool event tests */ + if (virTestRun("Storage pool createXML start event ", + testStoragePoolCreateXML, &test) < 0) + ret = EXIT_FAILURE; + if (virTestRun("Storage pool (un)define events", + testStoragePoolDefine, &test) < 0) + ret = EXIT_FAILURE; + + /* Define a test storage pool */ + if (!(test.pool = virStoragePoolDefineXML(test.conn, storagePoolDef, 0))) + ret = EXIT_FAILURE; + if (virTestRun("Storage pool start stop events ", + testStoragePoolStartStopEvent, &test) < 0) + ret = EXIT_FAILURE; + + /* Cleanup */ + if (test.pool) { + virStoragePoolUndefine(test.pool); + virStoragePoolFree(test.pool); + } + virConnectClose(test.conn); virEventRemoveTimeout(timer); -- 2.5.5

--- daemon/libvirtd.h | 2 + daemon/remote.c | 201 ++++++++++++++++++++++++++++++++++++++++++- src/remote/remote_driver.c | 128 +++++++++++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++++- src/remote_protocol-structs | 16 ++++ 5 files changed, 388 insertions(+), 2 deletions(-) diff --git a/daemon/libvirtd.h b/daemon/libvirtd.h index 7271b0f..cc91266 100644 --- a/daemon/libvirtd.h +++ b/daemon/libvirtd.h @@ -60,6 +60,8 @@ struct daemonClientPrivate { size_t nnetworkEventCallbacks; daemonClientEventCallbackPtr *qemuEventCallbacks; size_t nqemuEventCallbacks; + daemonClientEventCallbackPtr *storageEventCallbacks; + size_t nstorageEventCallbacks; bool closeRegistered; # if WITH_SASL diff --git a/daemon/remote.c b/daemon/remote.c index b2a420b..d6f5f1e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -181,6 +181,32 @@ remoteRelayNetworkEventCheckACL(virNetServerClientPtr client, return ret; } +static bool +remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr client, + virConnectPtr conn, + virStoragePoolPtr pool) +{ + virStoragePoolDef def; + virIdentityPtr identity = NULL; + bool ret = false; + + /* For now, we just create a virStoragePoolDef with enough contents to + * * satisfy what viraccessdriverpolkit.c references. This is a bit + * * fragile, but I don't know of anything better. */ + def.name = pool->name; + memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); + + if (!(identity = virNetServerClientGetIdentity(client))) + goto cleanup; + if (virIdentitySetCurrent(identity) < 0) + goto cleanup; + ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, &def); + + cleanup: + ignore_value(virIdentitySetCurrent(NULL)); + virObjectUnref(identity); + return ret; +} static bool remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, @@ -208,7 +234,6 @@ remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, return ret; } - static int remoteRelayDomainEventLifecycle(virConnectPtr conn, virDomainPtr dom, @@ -1236,6 +1261,44 @@ static virConnectNetworkEventGenericCallback networkEventCallbacks[] = { verify(ARRAY_CARDINALITY(networkEventCallbacks) == VIR_NETWORK_EVENT_ID_LAST); +static int +remoteRelayStoragePoolEventLifecycle(virConnectPtr conn, + virStoragePoolPtr pool, + int event, + int detail, + void *opaque) +{ + daemonClientEventCallbackPtr callback = opaque; + remote_storage_pool_event_lifecycle_msg data; + + if (callback->callbackID < 0 || + !remoteRelayStoragePoolEventCheckACL(callback->client, conn, pool)) + return -1; + + VIR_DEBUG("Relaying storage pool lifecycle event %d, detail %d, callback %d", + event, detail, callback->callbackID); + + /* build return data */ + memset(&data, 0, sizeof(data)); + make_nonnull_storage_pool(&data.pool, pool); + data.callbackID = callback->callbackID; + data.event = event; + data.detail = detail; + + remoteDispatchObjectEventSend(callback->client, remoteProgram, + REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE, + (xdrproc_t)xdr_remote_storage_pool_event_lifecycle_msg, + &data); + + return 0; +} + +static virConnectStoragePoolEventGenericCallback storageEventCallbacks[] = { + VIR_STORAGE_POOL_EVENT_CALLBACK(remoteRelayStoragePoolEventLifecycle), +}; + +verify(ARRAY_CARDINALITY(storageEventCallbacks) == VIR_STORAGE_POOL_EVENT_ID_LAST); + static void remoteRelayDomainQemuMonitorEvent(virConnectPtr conn, virDomainPtr dom, @@ -1343,6 +1406,21 @@ void remoteClientFreeFunc(void *data) } VIR_FREE(priv->networkEventCallbacks); + for (i = 0; i < priv->nstorageEventCallbacks; i++) { + int callbackID = priv->storageEventCallbacks[i]->callbackID; + if (callbackID < 0) { + VIR_WARN("unexpected incomplete storage pool callback %zu", i); + continue; + } + VIR_DEBUG("Deregistering remote storage pool event relay %d", + callbackID); + priv->storageEventCallbacks[i]->callbackID = -1; + if (virConnectStoragePoolEventDeregisterAny(priv->conn, + callbackID) < 0) + VIR_WARN("unexpected storage pool event deregister failure"); + } + VIR_FREE(priv->storageEventCallbacks); + for (i = 0; i < priv->nqemuEventCallbacks; i++) { int callbackID = priv->qemuEventCallbacks[i]->callbackID; if (callbackID < 0) { @@ -5434,6 +5512,127 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ return rv; } +static int +remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_connect_storage_pool_event_register_any_args *args, + remote_connect_storage_pool_event_register_any_ret *ret) +{ + int callbackID; + int rv = -1; + daemonClientEventCallbackPtr callback = NULL; + daemonClientEventCallbackPtr ref; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + virStoragePoolPtr pool = NULL; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + if (args->pool && + !(pool = get_nonnull_storage_pool(priv->conn, *args->pool))) + goto cleanup; + + if (args->eventID >= VIR_STORAGE_POOL_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->storageEventCallbacks, + priv->nstorageEventCallbacks, + callback) < 0) + goto cleanup; + + if ((callbackID = virConnectStoragePoolEventRegisterAny(priv->conn, + pool, + args->eventID, + storageEventCallbacks[args->eventID], + ref, + remoteEventCallbackFree)) < 0) { + VIR_SHRINK_N(priv->storageEventCallbacks, + priv->nstorageEventCallbacks, 1); + callback = ref; + goto cleanup; + } + + ref->callbackID = callbackID; + ret->callbackID = callbackID; + + rv = 0; + + cleanup: + VIR_FREE(callback); + if (rv < 0) + virNetMessageSaveError(rerr); + virObjectUnref(pool); + virMutexUnlock(&priv->lock); + return rv; +} + +static int +remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_connect_storage_pool_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->nstorageEventCallbacks; i++) { + if (priv->storageEventCallbacks[i]->callbackID == args->callbackID) + break; + } + if (i == priv->nstorageEventCallbacks) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("storage pool event callback %d not registered"), + args->callbackID); + goto cleanup; + } + + if (virConnectStoragePoolEventDeregisterAny(priv->conn, args->callbackID) < 0) + goto cleanup; + + VIR_DELETE_ELEMENT(priv->storageEventCallbacks, i, + priv->nstorageEventCallbacks); + + rv = 0; + + cleanup: + if (rv < 0) + virNetMessageSaveError(rerr); + virMutexUnlock(&priv->lock); + return rv; +} + static int qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED, diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f494cbf..6fae72e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -34,6 +34,7 @@ #include "datatypes.h" #include "domain_event.h" #include "network_event.h" +#include "storage_event.h" #include "driver.h" #include "virbuffer.h" #include "remote_driver.h" @@ -356,6 +357,11 @@ remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, void *evdata, void *opaque); static void +remoteStoragePoolBuildEventLifecycle(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); @@ -438,6 +444,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteNetworkBuildEventLifecycle, sizeof(remote_network_event_lifecycle_msg), (xdrproc_t)xdr_remote_network_event_lifecycle_msg }, + { REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE, + remoteStoragePoolBuildEventLifecycle, + sizeof(remote_storage_pool_event_lifecycle_msg), + (xdrproc_t)xdr_remote_storage_pool_event_lifecycle_msg }, { REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE, remoteDomainBuildEventCallbackLifecycle, sizeof(remote_domain_event_callback_lifecycle_msg), @@ -3040,6 +3050,101 @@ remoteConnectNetworkEventDeregisterAny(virConnectPtr conn, return rv; } +static int +remoteConnectStoragePoolEventRegisterAny(virConnectPtr conn, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + int rv = -1; + struct private_data *priv = conn->privateData; + remote_connect_storage_pool_event_register_any_args args; + remote_connect_storage_pool_event_register_any_ret ret; + int callbackID; + int count; + remote_nonnull_storage_pool storage_pool; + + remoteDriverLock(priv); + + if ((count = virStoragePoolEventStateRegisterClient(conn, priv->eventState, + pool, 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 (pool) { + make_nonnull_storage_pool(&storage_pool, pool); + args.pool = &storage_pool; + } else { + args.pool = NULL; + } + + memset(&ret, 0, sizeof(ret)); + if (call(conn, priv, 0, REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY, + (xdrproc_t) xdr_remote_connect_storage_pool_event_register_any_args, (char *) &args, + (xdrproc_t) xdr_remote_connect_storage_pool_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 +remoteConnectStoragePoolEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + struct private_data *priv = conn->privateData; + int rv = -1; + remote_connect_storage_pool_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_STORAGE_POOL_EVENT_DEREGISTER_ANY, + (xdrproc_t) xdr_remote_connect_storage_pool_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, @@ -5013,6 +5118,27 @@ remoteNetworkBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, remoteEventQueue(priv, event, msg->callbackID); } +static void +remoteStoragePoolBuildEventLifecycle(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, + virNetClientPtr client ATTRIBUTE_UNUSED, + void *evdata, void *opaque) +{ + virConnectPtr conn = opaque; + struct private_data *priv = conn->privateData; + remote_storage_pool_event_lifecycle_msg *msg = evdata; + virStoragePoolPtr pool; + virObjectEventPtr event = NULL; + + pool = get_nonnull_storage_pool(conn, msg->pool); + if (!pool) + return; + + event = virStoragePoolEventLifecycleNew(pool->name, pool->uuid, msg->event, + msg->detail); + virObjectUnref(pool); + + remoteEventQueue(priv, event, msg->callbackID); +} static void remoteDomainBuildQemuMonitorEvent(virNetClientProgramPtr prog ATTRIBUTE_UNUSED, @@ -7908,6 +8034,8 @@ static virStorageDriver storage_driver = { .connectListDefinedStoragePools = remoteConnectListDefinedStoragePools, /* 0.4.1 */ .connectListAllStoragePools = remoteConnectListAllStoragePools, /* 0.10.2 */ .connectFindStoragePoolSources = remoteConnectFindStoragePoolSources, /* 0.4.5 */ + .connectStoragePoolEventDeregisterAny = remoteConnectStoragePoolEventDeregisterAny, /* 1.3.6 */ + .connectStoragePoolEventRegisterAny = remoteConnectStoragePoolEventRegisterAny, /* 1.3.6 */ .storagePoolLookupByName = remoteStoragePoolLookupByName, /* 0.4.1 */ .storagePoolLookupByUUID = remoteStoragePoolLookupByUUID, /* 0.4.1 */ .storagePoolLookupByVolume = remoteStoragePoolLookupByVolume, /* 0.4.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index bab8ef2..0170c0c 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -3098,6 +3098,26 @@ struct remote_network_event_lifecycle_msg { int detail; }; +struct remote_connect_storage_pool_event_register_any_args { + int eventID; + remote_storage_pool pool; +}; + +struct remote_connect_storage_pool_event_register_any_ret { + int callbackID; +}; + +struct remote_connect_storage_pool_event_deregister_any_args { + int callbackID; +}; + +struct remote_storage_pool_event_lifecycle_msg { + int callbackID; + remote_nonnull_storage_pool pool; + int event; + int detail; +}; + struct remote_domain_fsfreeze_args { remote_nonnull_domain dom; remote_nonnull_string mountpoints<REMOTE_DOMAIN_FSFREEZE_MOUNTPOINTS_MAX>; /* (const char **) */ @@ -5793,5 +5813,26 @@ enum remote_procedure { * @generate: both * @acl: none */ - REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367 + REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367, + + /** + * @generate: none + * @priority: high + * @acl: connect:search_storage_pools + * @aclfilter: storage_pool:getattr + */ + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY = 368, + + /** + * @generate: none + * @priority: high + * @acl: connect:read + */ + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY = 369, + + /** + * @generate: both + * @acl: none + */ + REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index fe1b8a8..85bc62d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2551,6 +2551,22 @@ struct remote_network_event_lifecycle_msg { int event; int detail; }; +struct remote_connect_storage_pool_event_register_any_args { + int eventID; + remote_storage_pool pool; +}; +struct remote_connect_storage_pool_event_register_any_ret { + int callbackID; +}; +struct remote_connect_storage_pool_event_deregister_any_args { + int callbackID; +}; +struct remote_storage_pool_event_lifecycle_msg { + int callbackID; + remote_nonnull_storage_pool pool; + int event; + int detail; +}; struct remote_domain_fsfreeze_args { remote_nonnull_domain dom; struct { -- 2.5.5

On Mon, Jun 13, 2016 at 18:38:41 +0200, Jovanka Gulicoska wrote:a It may be worth noting that along with the lifecycle event this patch also implements all the dispatching stuff for events in the storage driver. It may be also worth splitting those two additions but I'm not going to insist.
--- daemon/libvirtd.h | 2 + daemon/remote.c | 201 ++++++++++++++++++++++++++++++++++++++++++- src/remote/remote_driver.c | 128 +++++++++++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++++- src/remote_protocol-structs | 16 ++++ 5 files changed, 388 insertions(+), 2 deletions(-)
[...]
diff --git a/daemon/remote.c b/daemon/remote.c index b2a420b..d6f5f1e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -181,6 +181,32 @@ remoteRelayNetworkEventCheckACL(virNetServerClientPtr client, return ret; }
+static bool +remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr client, + virConnectPtr conn, + virStoragePoolPtr pool) +{ + virStoragePoolDef def; + virIdentityPtr identity = NULL; + bool ret = false; + + /* For now, we just create a virStoragePoolDef with enough contents to + * * satisfy what viraccessdriverpolkit.c references. This is a bit + * * fragile, but I don't know of anything better. */
Very broken comment formatting.
+ def.name = pool->name; + memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); + + if (!(identity = virNetServerClientGetIdentity(client))) + goto cleanup; + if (virIdentitySetCurrent(identity) < 0) + goto cleanup; + ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, &def); + + cleanup: + ignore_value(virIdentitySetCurrent(NULL)); + virObjectUnref(identity); + return ret; +}
static bool remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, @@ -208,7 +234,6 @@ remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, return ret; }
-
Spurious whitespace change.
static int remoteRelayDomainEventLifecycle(virConnectPtr conn, virDomainPtr dom,
[...]
@@ -5434,6 +5512,127 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ return rv; }
+static int +remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_connect_storage_pool_event_register_any_args *args, + remote_connect_storage_pool_event_register_any_ret *ret) +{ + int callbackID; + int rv = -1; + daemonClientEventCallbackPtr callback = NULL; + daemonClientEventCallbackPtr ref; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + virStoragePoolPtr pool = NULL; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + if (args->pool && + !(pool = get_nonnull_storage_pool(priv->conn, *args->pool))) + goto cleanup; + + if (args->eventID >= VIR_STORAGE_POOL_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
Or you can use VIR_APPEND_ELEMENT_COPY to avoid clearing 'callback' and having to juggle the pointer between 'ref' and 'callback'
+ * 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->storageEventCallbacks, + priv->nstorageEventCallbacks, + callback) < 0) + goto cleanup; + + if ((callbackID = virConnectStoragePoolEventRegisterAny(priv->conn, + pool, + args->eventID, + storageEventCallbacks[args->eventID], + ref, + remoteEventCallbackFree)) < 0) { + VIR_SHRINK_N(priv->storageEventCallbacks, + priv->nstorageEventCallbacks, 1); + callback = ref; + goto cleanup; + } + + ref->callbackID = callbackID; + ret->callbackID = callbackID; + + rv = 0; + + cleanup: + VIR_FREE(callback); + if (rv < 0) + virNetMessageSaveError(rerr); + virObjectUnref(pool); + virMutexUnlock(&priv->lock); + return rv; +} +
[...]
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f494cbf..6fae72e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c
[...]
@@ -438,6 +444,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteNetworkBuildEventLifecycle, sizeof(remote_network_event_lifecycle_msg), (xdrproc_t)xdr_remote_network_event_lifecycle_msg }, + { REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE, + remoteStoragePoolBuildEventLifecycle, + sizeof(remote_storage_pool_event_lifecycle_msg), + (xdrproc_t)xdr_remote_storage_pool_event_lifecycle_msg },
We are usually adding at the tail of the list.
{ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE, remoteDomainBuildEventCallbackLifecycle, sizeof(remote_domain_event_callback_lifecycle_msg),
[...]
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index fe1b8a8..85bc62d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2551,6 +2551,22 @@ struct remote_network_event_lifecycle_msg { int event; int detail; }; +struct remote_connect_storage_pool_event_register_any_args { + int eventID; + remote_storage_pool pool; +}; +struct remote_connect_storage_pool_event_register_any_ret { + int callbackID; +}; +struct remote_connect_storage_pool_event_deregister_any_args { + int callbackID; +}; +struct remote_storage_pool_event_lifecycle_msg { + int callbackID; + remote_nonnull_storage_pool pool; + int event; + int detail; +};
Fails make check: --- remote_protocol-structs 2016-06-14 07:01:24.847495995 +0200 +++ remote_protocol-struct-t3 2016-06-14 07:01:59.265494534 +0200 @@ -2552,8 +2552,8 @@ int detail; }; struct remote_connect_storage_pool_event_register_any_args { - int eventID; - remote_storage_pool pool; + int eventID; + remote_storage_pool pool; }; struct remote_connect_storage_pool_event_register_any_ret { int callbackID; @@ -2562,10 +2562,10 @@ int callbackID; }; struct remote_storage_pool_event_lifecycle_msg { - int callbackID; - remote_nonnull_storage_pool pool; - int event; - int detail; + int callbackID; + remote_nonnull_storage_pool pool; + int event; + int detail; }; struct remote_domain_fsfreeze_args { remote_nonnull_domain dom; @@ -3119,4 +3119,7 @@ REMOTE_PROC_DOMAIN_GET_PERF_EVENTS = 365, REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 366, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367, + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY = 368, + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY = 369, + REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370, }; make[3]: *** [Makefile:10977: remote_protocol-struct] Error 1

On 06/14/2016 01:41 AM, Peter Krempa wrote:
On Mon, Jun 13, 2016 at 18:38:41 +0200, Jovanka Gulicoska wrote:a
It may be worth noting that along with the lifecycle event this patch also implements all the dispatching stuff for events in the storage driver.
It may be also worth splitting those two additions but I'm not going to insist.
--- daemon/libvirtd.h | 2 + daemon/remote.c | 201 ++++++++++++++++++++++++++++++++++++++++++- src/remote/remote_driver.c | 128 +++++++++++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++++- src/remote_protocol-structs | 16 ++++ 5 files changed, 388 insertions(+), 2 deletions(-)
[...]
diff --git a/daemon/remote.c b/daemon/remote.c index b2a420b..d6f5f1e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -181,6 +181,32 @@ remoteRelayNetworkEventCheckACL(virNetServerClientPtr client, return ret; }
+static bool +remoteRelayStoragePoolEventCheckACL(virNetServerClientPtr client, + virConnectPtr conn, + virStoragePoolPtr pool) +{ + virStoragePoolDef def; + virIdentityPtr identity = NULL; + bool ret = false; + + /* For now, we just create a virStoragePoolDef with enough contents to + * * satisfy what viraccessdriverpolkit.c references. This is a bit + * * fragile, but I don't know of anything better. */
Very broken comment formatting.
+ def.name = pool->name; + memcpy(def.uuid, pool->uuid, VIR_UUID_BUFLEN); + + if (!(identity = virNetServerClientGetIdentity(client))) + goto cleanup; + if (virIdentitySetCurrent(identity) < 0) + goto cleanup; + ret = virConnectStoragePoolEventRegisterAnyCheckACL(conn, &def); + + cleanup: + ignore_value(virIdentitySetCurrent(NULL)); + virObjectUnref(identity); + return ret; +}
static bool remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, @@ -208,7 +234,6 @@ remoteRelayDomainQemuMonitorEventCheckACL(virNetServerClientPtr client, return ret; }
-
Spurious whitespace change.
static int remoteRelayDomainEventLifecycle(virConnectPtr conn, virDomainPtr dom,
[...]
@@ -5434,6 +5512,127 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ return rv; }
+static int +remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNUSED, + virNetServerClientPtr client, + virNetMessagePtr msg ATTRIBUTE_UNUSED, + virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED, + remote_connect_storage_pool_event_register_any_args *args, + remote_connect_storage_pool_event_register_any_ret *ret) +{ + int callbackID; + int rv = -1; + daemonClientEventCallbackPtr callback = NULL; + daemonClientEventCallbackPtr ref; + struct daemonClientPrivate *priv = + virNetServerClientGetPrivateData(client); + virStoragePoolPtr pool = NULL; + + if (!priv->conn) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); + goto cleanup; + } + + virMutexLock(&priv->lock); + + if (args->pool && + !(pool = get_nonnull_storage_pool(priv->conn, *args->pool))) + goto cleanup; + + if (args->eventID >= VIR_STORAGE_POOL_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
Or you can use VIR_APPEND_ELEMENT_COPY to avoid clearing 'callback' and having to juggle the pointer between 'ref' and 'callback'
+ * 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->storageEventCallbacks, + priv->nstorageEventCallbacks, + callback) < 0) + goto cleanup; + + if ((callbackID = virConnectStoragePoolEventRegisterAny(priv->conn, + pool, + args->eventID, + storageEventCallbacks[args->eventID], + ref, + remoteEventCallbackFree)) < 0) { + VIR_SHRINK_N(priv->storageEventCallbacks, + priv->nstorageEventCallbacks, 1); + callback = ref; + goto cleanup; + } + + ref->callbackID = callbackID; + ret->callbackID = callbackID; + + rv = 0; + + cleanup: + VIR_FREE(callback); + if (rv < 0) + virNetMessageSaveError(rerr); + virObjectUnref(pool); + virMutexUnlock(&priv->lock); + return rv; +} +
[...]
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f494cbf..6fae72e 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c
[...]
@@ -438,6 +444,10 @@ static virNetClientProgramEvent remoteEvents[] = { remoteNetworkBuildEventLifecycle, sizeof(remote_network_event_lifecycle_msg), (xdrproc_t)xdr_remote_network_event_lifecycle_msg }, + { REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE, + remoteStoragePoolBuildEventLifecycle, + sizeof(remote_storage_pool_event_lifecycle_msg), + (xdrproc_t)xdr_remote_storage_pool_event_lifecycle_msg },
We are usually adding at the tail of the list.
{ REMOTE_PROC_DOMAIN_EVENT_CALLBACK_LIFECYCLE, remoteDomainBuildEventCallbackLifecycle, sizeof(remote_domain_event_callback_lifecycle_msg),
[...]
diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index fe1b8a8..85bc62d 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2551,6 +2551,22 @@ struct remote_network_event_lifecycle_msg { int event; int detail; }; +struct remote_connect_storage_pool_event_register_any_args { + int eventID; + remote_storage_pool pool; +}; +struct remote_connect_storage_pool_event_register_any_ret { + int callbackID; +}; +struct remote_connect_storage_pool_event_deregister_any_args { + int callbackID; +}; +struct remote_storage_pool_event_lifecycle_msg { + int callbackID; + remote_nonnull_storage_pool pool; + int event; + int detail; +};
Fails make check:
--- remote_protocol-structs 2016-06-14 07:01:24.847495995 +0200 +++ remote_protocol-struct-t3 2016-06-14 07:01:59.265494534 +0200 @@ -2552,8 +2552,8 @@ int detail; }; struct remote_connect_storage_pool_event_register_any_args { - int eventID; - remote_storage_pool pool; + int eventID; + remote_storage_pool pool; }; struct remote_connect_storage_pool_event_register_any_ret { int callbackID; @@ -2562,10 +2562,10 @@ int callbackID; }; struct remote_storage_pool_event_lifecycle_msg { - int callbackID; - remote_nonnull_storage_pool pool; - int event; - int detail; + int callbackID; + remote_nonnull_storage_pool pool; + int event; + int detail; }; struct remote_domain_fsfreeze_args { remote_nonnull_domain dom; @@ -3119,4 +3119,7 @@ REMOTE_PROC_DOMAIN_GET_PERF_EVENTS = 365, REMOTE_PROC_DOMAIN_SET_PERF_EVENTS = 366, REMOTE_PROC_DOMAIN_EVENT_CALLBACK_DEVICE_REMOVAL_FAILED = 367, + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_REGISTER_ANY = 368, + REMOTE_PROC_CONNECT_STORAGE_POOL_EVENT_DEREGISTER_ANY = 369, + REMOTE_PROC_STORAGE_POOL_EVENT_LIFECYCLE = 370, }; make[3]: *** [Makefile:10977: remote_protocol-struct] Error 1
Strange. Tried figuring out why I don't see this. If I do cd src; make check-protocol I get: $ make check-protocol GEN remote_protocol-struct GEN lxc_protocol-struct GEN qemu_protocol-struct GEN virnetprotocol-struct GEN virkeepaliveprotocol-struct GEN admin_protocol-struct GEN lxc_monitor_protocol-struct GEN lock_protocol-struct WARNING: pdwtags appears broken; skipping the qemu_protocol-struct test WARNING: pdwtags appears broken; skipping the remote_protocol-struct test WARNING: pdwtags appears broken; skipping the lxc_protocol-struct test WARNING: pdwtags appears broken; skipping the lock_protocol-struct test WARNING: pdwtags appears broken; skipping the virnetprotocol-struct test WARNING: pdwtags appears broken; skipping the virkeepaliveprotocol-struct test WARNING: pdwtags appears broken; skipping the admin_protocol-struct test WARNING: pdwtags appears broken; skipping the lxc_monitor_protocol-struct test So I guess that's why I don't see the error. Makefile.am has some notes about handling different pdwtags output formats, but the last commit is from 2011 from Eric. Maybe the format changed again: $ rpm -qf `which pdwtags` dwarves-1.10-9.fc24.x86_64 Thanks, Cole

Implement storage pool event callbacks for START, STOP, DEFINE, UNDEFINED and REFRESHED in functions when a storage pool is created/started/stopped etc. accordingly --- src/storage/storage_driver.c | 110 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index fb1b1a2..d336bd6 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -41,6 +41,7 @@ #include "driver.h" #include "storage_driver.h" #include "storage_conf.h" +#include "storage_event.h" #include "viralloc.h" #include "storage_backend.h" #include "virlog.h" @@ -276,6 +277,8 @@ storageStateInitialize(bool privileged, storagePoolUpdateAllState(); + driver->storageEventState = virObjectEventStateNew(); + storageDriverUnlock(); ret = 0; @@ -344,6 +347,8 @@ storageStateCleanup(void) storageDriverLock(); + virObjectEventStateFree(driver->storageEventState); + /* free inactive pools */ virStoragePoolObjListFree(&driver->pools); @@ -668,6 +673,7 @@ storagePoolCreateXML(virConnectPtr conn, virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; virStorageBackendPtr backend; + virObjectEventPtr event = NULL; char *stateFile = NULL; unsigned int build_flags = 0; @@ -735,6 +741,12 @@ storagePoolCreateXML(virConnectPtr conn, pool = NULL; goto cleanup; } + + event = virStoragePoolEventLifecycleNew(pool->def->name, + pool->def->uuid, + VIR_STORAGE_POOL_EVENT_STARTED, + 0); + VIR_INFO("Creating storage pool '%s'", pool->def->name); pool->active = true; @@ -744,6 +756,8 @@ storagePoolCreateXML(virConnectPtr conn, cleanup: VIR_FREE(stateFile); virStoragePoolDefFree(def); + if (event) + virObjectEventStateQueue(driver->storageEventState, event); if (pool) virStoragePoolObjUnlock(pool); storageDriverUnlock(); @@ -758,6 +772,7 @@ storagePoolDefineXML(virConnectPtr conn, virStoragePoolDefPtr def; virStoragePoolObjPtr pool = NULL; virStoragePoolPtr ret = NULL; + virObjectEventPtr event = NULL; virCheckFlags(0, NULL); @@ -786,6 +801,11 @@ storagePoolDefineXML(virConnectPtr conn, pool = NULL; goto cleanup; } + + event = virStoragePoolEventLifecycleNew(def->name, def->uuid, + VIR_STORAGE_POOL_EVENT_DEFINED, + 0); + def = NULL; VIR_INFO("Defining storage pool '%s'", pool->def->name); @@ -793,6 +813,8 @@ storagePoolDefineXML(virConnectPtr conn, NULL, NULL); cleanup: + if (event) + virObjectEventStateQueue(driver->storageEventState, event); virStoragePoolDefFree(def); if (pool) virStoragePoolObjUnlock(pool); @@ -804,6 +826,7 @@ static int storagePoolUndefine(virStoragePoolPtr obj) { virStoragePoolObjPtr pool; + virObjectEventPtr event = NULL; int ret = -1; storageDriverLock(); @@ -847,12 +870,19 @@ storagePoolUndefine(virStoragePoolPtr obj) VIR_FREE(pool->configFile); VIR_FREE(pool->autostartLink); + event = virStoragePoolEventLifecycleNew(pool->def->name, + pool->def->uuid, + VIR_STORAGE_POOL_EVENT_UNDEFINED, + 0); + VIR_INFO("Undefining storage pool '%s'", pool->def->name); virStoragePoolObjRemove(&driver->pools, pool); pool = NULL; ret = 0; cleanup: + if (event) + virObjectEventStateQueue(driver->storageEventState, event); if (pool) virStoragePoolObjUnlock(pool); storageDriverUnlock(); @@ -865,6 +895,7 @@ storagePoolCreate(virStoragePoolPtr obj, { virStoragePoolObjPtr pool; virStorageBackendPtr backend; + virObjectEventPtr event = NULL; int ret = -1; char *stateFile = NULL; unsigned int build_flags = 0; @@ -926,11 +957,18 @@ storagePoolCreate(virStoragePoolPtr obj, goto cleanup; } + event = virStoragePoolEventLifecycleNew(pool->def->name, + pool->def->uuid, + VIR_STORAGE_POOL_EVENT_STARTED, + 0); + pool->active = true; ret = 0; cleanup: VIR_FREE(stateFile); + if (event) + virObjectEventStateQueue(driver->storageEventState, event); if (pool) virStoragePoolObjUnlock(pool); return ret; @@ -976,6 +1014,7 @@ storagePoolDestroy(virStoragePoolPtr obj) { virStoragePoolObjPtr pool; virStorageBackendPtr backend; + virObjectEventPtr event = NULL; char *stateFile = NULL; int ret = -1; @@ -1024,6 +1063,11 @@ storagePoolDestroy(virStoragePoolPtr obj) virStoragePoolObjClearVols(pool); + event = virStoragePoolEventLifecycleNew(pool->def->name, + pool->def->uuid, + VIR_STORAGE_POOL_EVENT_STOPPED, + 0); + pool->active = false; if (pool->configFile == NULL) { @@ -1038,6 +1082,8 @@ storagePoolDestroy(virStoragePoolPtr obj) ret = 0; cleanup: + if (event) + virObjectEventStateQueue(driver->storageEventState, event); if (pool) virStoragePoolObjUnlock(pool); storageDriverUnlock(); @@ -1109,6 +1155,7 @@ storagePoolRefresh(virStoragePoolPtr obj, virStoragePoolObjPtr pool; virStorageBackendPtr backend; int ret = -1; + virObjectEventPtr event = NULL; virCheckFlags(0, -1); @@ -1146,6 +1193,10 @@ storagePoolRefresh(virStoragePoolPtr obj, if (backend->stopPool) backend->stopPool(obj->conn, pool); + event = virStoragePoolEventLifecycleNew(pool->def->name, + pool->def->uuid, + VIR_STORAGE_POOL_EVENT_STOPPED, + 0); pool->active = false; if (pool->configFile == NULL) { @@ -1154,9 +1205,16 @@ storagePoolRefresh(virStoragePoolPtr obj, } goto cleanup; } + + event = virStoragePoolEventLifecycleNew(pool->def->name, + pool->def->uuid, + VIR_STORAGE_POOL_EVENT_REFRESHED, + 0); ret = 0; cleanup: + if (event) + virObjectEventStateQueue(driver->storageEventState, event); if (pool) virStoragePoolObjUnlock(pool); storageDriverUnlock(); @@ -2266,6 +2324,7 @@ virStorageVolPoolRefreshThread(void *opaque) virStorageVolStreamInfoPtr cbdata = opaque; virStoragePoolObjPtr pool = NULL; virStorageBackendPtr backend; + virObjectEventPtr event = NULL; storageDriverLock(); if (cbdata->vol_path) { @@ -2283,7 +2342,14 @@ virStorageVolPoolRefreshThread(void *opaque) if (backend->refreshPool(NULL, pool) < 0) VIR_DEBUG("Failed to refresh storage pool"); + event = virStoragePoolEventLifecycleNew(pool->def->name, + pool->def->uuid, + VIR_STORAGE_POOL_EVENT_REFRESHED, + 0); + cleanup: + if (event) + virObjectEventStateQueue(driver->storageEventState, event); if (pool) virStoragePoolObjUnlock(pool); storageDriverUnlock(); @@ -2662,6 +2728,48 @@ storageConnectListAllStoragePools(virConnectPtr conn, return ret; } +static int +storageConnectStoragePoolEventRegisterAny(virConnectPtr conn, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + int ret = -1; + + if (virConnectStoragePoolEventRegisterAnyEnsureACL(conn) < 0) + goto cleanup; + + if (virStoragePoolEventStateRegisterID(conn, driver->storageEventState, + pool, eventID, callback, + opaque, freecb, &ret) < 0) + ret = -1; + cleanup: + return ret; +} + +static int +storageConnectStoragePoolEventDeregisterAny(virConnectPtr conn, + int callbackID) +{ + int ret = -1; + + if (virConnectStoragePoolEventDeregisterAnyEnsureACL(conn) < 0) + goto cleanup; + + if (virObjectEventStateDeregisterID(conn, + driver->storageEventState, + callbackID) < 0) + goto cleanup; + + ret = 0; + + cleanup: + return ret; +} + + static virStorageDriver storageDriver = { .name = "storage", @@ -2670,6 +2778,8 @@ static virStorageDriver storageDriver = { .connectNumOfDefinedStoragePools = storageConnectNumOfDefinedStoragePools, /* 0.4.0 */ .connectListDefinedStoragePools = storageConnectListDefinedStoragePools, /* 0.4.0 */ .connectListAllStoragePools = storageConnectListAllStoragePools, /* 0.10.2 */ + .connectStoragePoolEventRegisterAny = storageConnectStoragePoolEventRegisterAny, /* 1.3.6 */ + .connectStoragePoolEventDeregisterAny = storageConnectStoragePoolEventDeregisterAny, /* 1.3.6 */ .connectFindStoragePoolSources = storageConnectFindStoragePoolSources, /* 0.4.0 */ .storagePoolLookupByName = storagePoolLookupByName, /* 0.4.0 */ .storagePoolLookupByUUID = storagePoolLookupByUUID, /* 0.4.0 */ -- 2.5.5

On Mon, Jun 13, 2016 at 18:38:42 +0200, Jovanka Gulicoska wrote:
Implement storage pool event callbacks for START, STOP, DEFINE, UNDEFINED and REFRESHED in functions when a storage pool is created/started/stopped etc. accordingly --- src/storage/storage_driver.c | 110 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 110 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index fb1b1a2..d336bd6 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -41,6 +41,7 @@ #include "driver.h" #include "storage_driver.h" #include "storage_conf.h" +#include "storage_event.h" #include "viralloc.h" #include "storage_backend.h" #include "virlog.h" @@ -276,6 +277,8 @@ storageStateInitialize(bool privileged,
storagePoolUpdateAllState();
+ driver->storageEventState = virObjectEventStateNew();
So. This patch needs to add 'storageEventState' rather than 2/6.
+ storageDriverUnlock();
ret = 0;
[...]
@@ -2662,6 +2728,48 @@ storageConnectListAllStoragePools(virConnectPtr conn, return ret; }
+static int +storageConnectStoragePoolEventRegisterAny(virConnectPtr conn, + virStoragePoolPtr pool, + int eventID, + virConnectStoragePoolEventGenericCallback callback, + void *opaque, + virFreeCallback freecb) +{ + int ret = -1;
I'd call this 'callbackID' since. Otherwise the usage is a bit confusing.
+ + if (virConnectStoragePoolEventRegisterAnyEnsureACL(conn) < 0) + goto cleanup; + + if (virStoragePoolEventStateRegisterID(conn, driver->storageEventState, + pool, eventID, callback, + opaque, freecb, &ret) < 0) + ret = -1; + cleanup: + return ret; +} +

--- examples/object-events/event-test.c | 46 ++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/examples/object-events/event-test.c b/examples/object-events/event-test.c index c1ff4a7..a951230 100644 --- a/examples/object-events/event-test.c +++ b/examples/object-events/event-test.c @@ -337,6 +337,26 @@ guestAgentLifecycleEventReasonToString(int event) return "unknown"; } +static const char * +storagePoolEventToString(int event) +{ + switch ((virStoragePoolEventLifecycleType) event) { + case VIR_STORAGE_POOL_EVENT_DEFINED: + return "Defined"; + case VIR_STORAGE_POOL_EVENT_UNDEFINED: + return "Undefined"; + case VIR_STORAGE_POOL_EVENT_STARTED: + return "Started"; + case VIR_STORAGE_POOL_EVENT_STOPPED: + return "Stopped"; + case VIR_STORAGE_POOL_EVENT_REFRESHED: + return "Refreshed"; + case VIR_STORAGE_POOL_EVENT_LAST: + break; + } + return "unknown"; +} + static int myDomainEventCallback1(virConnectPtr conn ATTRIBUTE_UNUSED, @@ -644,6 +664,20 @@ myNetworkEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, return 0; } +static int +myStoragePoolEventCallback(virConnectPtr conn ATTRIBUTE_UNUSED, + virStoragePoolPtr dom, + int event, + int detail, + void *opaque ATTRIBUTE_UNUSED) +{ + printf("%s EVENT: Storage pool %s %s %d\n", + __func__, virStoragePoolGetName(dom), + storagePoolEventToString(event), detail); + return 0; +} + + static void eventTypedParamsPrint(virTypedParameterPtr params, int nparams) @@ -899,6 +933,7 @@ main(int argc, char **argv) virConnectPtr dconn = NULL; int callback1ret = -1; int callback16ret = -1; + int callback17ret = -1; struct sigaction action_stop; size_t i; @@ -966,8 +1001,16 @@ main(int argc, char **argv) VIR_NETWORK_EVENT_CALLBACK(myNetworkEventCallback), strdup("net callback"), myFreeFunc); + callback17ret = virConnectStoragePoolEventRegisterAny(dconn, + NULL, + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE, + VIR_STORAGE_POOL_EVENT_CALLBACK(myStoragePoolEventCallback), + strdup("storage pool callback"), myFreeFunc); + + if ((callback1ret == -1) || - (callback16ret == -1)) + (callback16ret == -1) || + (callback17ret == -1)) goto cleanup; if (virConnectSetKeepAlive(dconn, 5, 3) < 0) { @@ -986,6 +1029,7 @@ main(int argc, char **argv) printf("Deregistering event callbacks\n"); virConnectDomainEventDeregister(dconn, myDomainEventCallback1); virConnectNetworkEventDeregisterAny(dconn, callback16ret); + virConnectStoragePoolEventDeregisterAny(dconn, callback17ret); printf("Deregistering domain event callbacks\n"); -- 2.5.5

On Mon, Jun 13, 2016 at 18:38:43 +0200, Jovanka Gulicoska wrote:
--- examples/object-events/event-test.c | 46 ++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
[...]
@@ -899,6 +933,7 @@ main(int argc, char **argv) virConnectPtr dconn = NULL; int callback1ret = -1; int callback16ret = -1; + int callback17ret = -1; struct sigaction action_stop; size_t i;
@@ -966,8 +1001,16 @@ main(int argc, char **argv) VIR_NETWORK_EVENT_CALLBACK(myNetworkEventCallback), strdup("net callback"), myFreeFunc);
+ callback17ret = virConnectStoragePoolEventRegisterAny(dconn,
If you follow the approach used by domain events rather than network events the new code will automatically trigger a build failure when you add new events and also will make addition of new storage pool callback types much easier.
+ NULL, + VIR_STORAGE_POOL_EVENT_ID_LIFECYCLE, + VIR_STORAGE_POOL_EVENT_CALLBACK(myStoragePoolEventCallback), + strdup("storage pool callback"), myFreeFunc); + + if ((callback1ret == -1) || - (callback16ret == -1)) + (callback16ret == -1) || + (callback17ret == -1)) goto cleanup;
if (virConnectSetKeepAlive(dconn, 5, 3) < 0) {

On 06/13/2016 12:38 PM, Jovanka Gulicoska wrote:
Changes follow comments on v1 patches.
Introducing implementation of storage pool event APIs. Code changes follow network event APIs.
Implemented functions: virStoragePoolEventRegisterAny(), virStoragePoolEventDeregisterAny(), virStoragePoolLifeCycleEventNew(), introduced STARTED, STOPPED, DEFINE, UNDEFINE and REFRESHED.
STARTED signal is emiited in storagePoolCreateXML() and storagePoolCreate() DEFINED signal is emitted in storagePoolDefineXML() UNDEFINED signal is emitted in storagePoolUndefine() STOPPED signal is emitted in storagePoolDestroy() and storagePoolRefresh() REFRESHED signal is emitted in storagePoolRefresh()
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 As part of the project there should also be implementation for storage volume events. For now there's no signal when volumes are created or deleted, they can also be implemented, but probably the easiest way is to have apps watch for REFRESH signal, and later extend storage driver code to refresh a pool after volume APIs are called.
Jovanka Gulicoska (6): Introduce storage lifecycle event APIs conf: add storage_event handling test: implement storage lifecycle event APIs remote: implement storage lifecycle event APIs storage: implement storage lifecycle event APIs event-test: support storage lifecycle event APIs
daemon/libvirtd.h | 2 + daemon/remote.c | 201 +++++++++++++++++++++++++++++- examples/object-events/event-test.c | 46 ++++++- include/libvirt/libvirt-storage.h | 94 ++++++++++++++ src/Makefile.am | 5 + src/conf/storage_conf.h | 4 + src/conf/storage_event.c | 237 ++++++++++++++++++++++++++++++++++++ src/conf/storage_event.h | 60 +++++++++ src/datatypes.h | 13 ++ src/driver-storage.h | 14 +++ src/libvirt-storage.c | 125 +++++++++++++++++++ src/libvirt_private.syms | 5 + src/libvirt_public.syms | 7 ++ src/remote/remote_driver.c | 128 +++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++- src/remote_protocol-structs | 16 +++ src/storage/storage_driver.c | 110 +++++++++++++++++ src/test/test_driver.c | 71 +++++++++++ tests/objecteventtest.c | 177 +++++++++++++++++++++++++++ 19 files changed, 1355 insertions(+), 3 deletions(-) create mode 100644 src/conf/storage_event.c create mode 100644 src/conf/storage_event.h
I just reviewed the diff between v1 and v2 and made a couple small changes, diff attached. I copied the docstring from the network events since your version still had several typos, and fixed one spacing issue in comments in the test driver. Otherwise it looks like all my review bits were addressed, so this looks good to me. Let's give it some time to see if anyone else has comments, otherwise I'll push it at the end of the week (with those minor changes) Thanks, Cole

On 06/13/2016 01:35 PM, Cole Robinson wrote:
On 06/13/2016 12:38 PM, Jovanka Gulicoska wrote:
Changes follow comments on v1 patches.
Introducing implementation of storage pool event APIs. Code changes follow network event APIs.
Implemented functions: virStoragePoolEventRegisterAny(), virStoragePoolEventDeregisterAny(), virStoragePoolLifeCycleEventNew(), introduced STARTED, STOPPED, DEFINE, UNDEFINE and REFRESHED.
STARTED signal is emiited in storagePoolCreateXML() and storagePoolCreate() DEFINED signal is emitted in storagePoolDefineXML() UNDEFINED signal is emitted in storagePoolUndefine() STOPPED signal is emitted in storagePoolDestroy() and storagePoolRefresh() REFRESHED signal is emitted in storagePoolRefresh()
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 As part of the project there should also be implementation for storage volume events. For now there's no signal when volumes are created or deleted, they can also be implemented, but probably the easiest way is to have apps watch for REFRESH signal, and later extend storage driver code to refresh a pool after volume APIs are called.
Jovanka Gulicoska (6): Introduce storage lifecycle event APIs conf: add storage_event handling test: implement storage lifecycle event APIs remote: implement storage lifecycle event APIs storage: implement storage lifecycle event APIs event-test: support storage lifecycle event APIs
daemon/libvirtd.h | 2 + daemon/remote.c | 201 +++++++++++++++++++++++++++++- examples/object-events/event-test.c | 46 ++++++- include/libvirt/libvirt-storage.h | 94 ++++++++++++++ src/Makefile.am | 5 + src/conf/storage_conf.h | 4 + src/conf/storage_event.c | 237 ++++++++++++++++++++++++++++++++++++ src/conf/storage_event.h | 60 +++++++++ src/datatypes.h | 13 ++ src/driver-storage.h | 14 +++ src/libvirt-storage.c | 125 +++++++++++++++++++ src/libvirt_private.syms | 5 + src/libvirt_public.syms | 7 ++ src/remote/remote_driver.c | 128 +++++++++++++++++++ src/remote/remote_protocol.x | 43 ++++++- src/remote_protocol-structs | 16 +++ src/storage/storage_driver.c | 110 +++++++++++++++++ src/test/test_driver.c | 71 +++++++++++ tests/objecteventtest.c | 177 +++++++++++++++++++++++++++ 19 files changed, 1355 insertions(+), 3 deletions(-) create mode 100644 src/conf/storage_event.c create mode 100644 src/conf/storage_event.h
I just reviewed the diff between v1 and v2 and made a couple small changes, diff attached. I copied the docstring from the network events since your version still had several typos, and fixed one spacing issue in comments in the test driver. Otherwise it looks like all my review bits were addressed, so this looks good to me.
Let's give it some time to see if anyone else has comments, otherwise I'll push it at the end of the week (with those minor changes)
I will also adjust the 1.3.6 version number to match whatever new version number we end up going with in libvirt.git, likely 2.0.0, from the other discussion on list. Same for in libvirt-python - Cole

On Mon, Jun 13, 2016 at 13:35:04 -0400, Cole Robinson wrote:
On 06/13/2016 12:38 PM, Jovanka Gulicoska wrote:
[...]
I just reviewed the diff between v1 and v2 and made a couple small changes, diff attached. I copied the docstring from the network events since your version still had several typos, and fixed one spacing issue in comments in the test driver. Otherwise it looks like all my review bits were addressed, so this looks good to me.
Let's give it some time to see if anyone else has comments, otherwise I'll push it at the end of the week (with those minor changes)
Apart from needing to make sure that this series passes 'make check' and the few nits pointed out through the code there's one thing that should be added. Every event implementation has a virsh command allowing to watch them and the virsh change is still missing. It's okay to add it later but it should be in this release.

On 06/14/2016 02:09 AM, Peter Krempa wrote:
On Mon, Jun 13, 2016 at 13:35:04 -0400, Cole Robinson wrote:
On 06/13/2016 12:38 PM, Jovanka Gulicoska wrote:
[...]
I just reviewed the diff between v1 and v2 and made a couple small changes, diff attached. I copied the docstring from the network events since your version still had several typos, and fixed one spacing issue in comments in the test driver. Otherwise it looks like all my review bits were addressed, so this looks good to me.
Let's give it some time to see if anyone else has comments, otherwise I'll push it at the end of the week (with those minor changes)
Apart from needing to make sure that this series passes 'make check' and the few nits pointed out through the code there's one thing that should be added. Every event implementation has a virsh command allowing to watch them and the virsh change is still missing. It's okay to add it later but it should be in this release.
Ah I noticed that there was 'net-event' but I didn't realize we had 'event' for domains as well. Thanks for pointing that out, I agree a 'pool-event' command should be part of this series - Cole

On 06/14/2016 02:09 AM, Peter Krempa wrote:
On Mon, Jun 13, 2016 at 13:35:04 -0400, Cole Robinson wrote:
On 06/13/2016 12:38 PM, Jovanka Gulicoska wrote:
[...]
I just reviewed the diff between v1 and v2 and made a couple small changes, diff attached. I copied the docstring from the network events since your version still had several typos, and fixed one spacing issue in comments in the test driver. Otherwise it looks like all my review bits were addressed, so this looks good to me.
Let's give it some time to see if anyone else has comments, otherwise I'll push it at the end of the week (with those minor changes)
Apart from needing to make sure that this series passes 'make check' and the few nits pointed out through the code there's one thing that should be added. Every event implementation has a virsh command allowing to watch them and the virsh change is still missing. It's okay to add it later but it should be in this release.
Also, where/how does 'make check' fail for you? Neither it nor 'make syntax-check' show any error for me. Thanks, Cole
participants (3)
-
Cole Robinson
-
Jovanka Gulicoska
-
Peter Krempa