| Message ID | fe71b37f-e136-2153-d2d5-f476e580ae6c@redhat.com |
|---|---|
| State | New |
| Headers | show |
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
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 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index bc8cee5..f593385 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -412,11 +412,10 @@ typedef enum { * @opaque: application specified data * * A generic storage pool event callback handler, for use with - * virConnectStoragePoolEventRegisterAny(). - * Spcific events usually have a customization with extra paramenters, - * often with @opaque being passed in a different specific possition; - * use VIR_STORAGE_POOL_EVENT_CALLBACK() when registering an - * appropriate handler. + * virConnectStoragePoolEventRegisterAny(). Specific events usually + * have a customization with extra parameters, often with @opaque being + * passed in a different parameter position; use + * VIR_STORAGE_POOL_EVENT_CALLBACK() when registering an appropriate handler. */ typedef void (*virConnectStoragePoolEventGenericCallback)(virConnectPtr conn, virStoragePoolPtr pool, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 3b1e004..8994f83 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6820,8 +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*/ + .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 */