From: Eduardo Lima (Etrunko)&lt;eblima(a)br.ibm.com&gt; Signed-off-by: Eduardo Lima (Etrunko)&lt;eblima(a)br.ibm.com&gt; --- src/Virt_VirtualSystemManagementService.c | 64 ----------------------------- 1 files changed, 0 insertions(+), 64 deletions(-) diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c index 0141515..4d26429 100644 --- a/src/Virt_VirtualSystemManagementService.c +++ b/src/Virt_VirtualSystemManagementService.c @@ -768,70 +768,6 @@ static const char *_net_rand_mac(const CMPIObjectPath *ref) return _mac; } -/* -static const char *filter_by_address(struct inst_list *src, - const char *address) -{ - int i; - CMPIrc ret; - const char *addr; - const char *msg = NULL; - CMPIInstance *inst; - - for (i = 0; i< src->cur; i++) { - inst = src->list[i]; - ret = cu_get_str_prop(inst, "Address",&addr); - - if (ret != CMPI_RC_OK) - continue; - - if (STREQ(addr, address)) { - msg = "Conflicting MAC Addresses"; - goto out; - } - } -out: - return msg; - -} - -static const char *check_duplicate_mac(CMPIInstance *inst, - const char *mac, - const char *ns) -{ - CMPIObjectPath *op = NULL; - CMPIStatus s; - const char *msg = NULL; - const char *props[] = {"Address", NULL}; - struct inst_list in_list; - - op = CMGetObjectPath(inst,&s); - if ((op == NULL) || (s.rc != CMPI_RC_OK)) { - msg = "Unable to get a reference to the guest"; - goto out; - } - - FIXME: This is a Pegasus work around. Pegsus loses the namespace - when an ObjectPath is pulled from an instance - - if (STREQ(NAMESPACE(op), "")) - CMSetNameSpace(op, ns); - - inst_list_init(&in_list); - s = enum_rasds(_BROKER, op, NULL, CIM_RES_TYPE_NET, props,&in_list); - if (s.rc != CMPI_RC_OK) { - msg = "Failed to enumerate network instances"; - goto out; - } - - msg = filter_by_address(&in_list, mac); - -out: - inst_list_free(&in_list); - return msg; -} -*/ - static const char *net_rasd_to_vdev(CMPIInstance *inst, struct virt_device *dev, const char *ns)

viewed the codes and no obvious problem found. Just one thing uncertain: this require libvirt0.9.0, does this affect existing user who have not latest libvirt installed? If not maybe old code should be kept.

And is there some expected result I can get in wbemexec or wbemcli to test the patch easily? 于 2011-12-30 1:55, Eduardo Lima (Etrunko) 写道: > From: Eduardo Lima (Etrunko)&lt;eblima(a)br.ibm.com&gt; > > Port current implementation to use domain events implementation provided by > libvirt. It requires libvirt version 0.9.0 or newer, when the > virEventRegisterDefaultImpl and virEventRunDefaultImpl API's were introduced. > > Now that all events are generated by libvirt itself, we don't need to trigger > them manually anymore. Thus, this patch also removes trigger and raise calls > from VirtualSystemManagementService and ComputerSystem providers. > > Signed-off-by: Eduardo Lima (Etrunko)&lt;eblima(a)br.ibm.com&gt; > --- > acinclude.m4 | 2 +- > libvirt-cim.spec.in | 4 +- > libxkutil/misc_util.c | 2 + > src/Virt_ComputerSystem.c | 51 +-- > src/Virt_ComputerSystemIndication.c | 759 ++++++++++++++--------------- > src/Virt_ComputerSystemIndication.h | 5 - > src/Virt_VirtualSystemManagementService.c | 30 +- > 7 files changed, 373 insertions(+), 480 deletions(-) > > diff --git a/acinclude.m4 b/acinclude.m4 > index e313891..e0f76b6 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -243,7 +243,7 @@ AC_DEFUN([CHECK_LIBCU], > > AC_DEFUN([CHECK_LIBVIRT], > [ > - PKG_CHECK_MODULES([LIBVIRT], [libvirt>= 0.3.2]) > + PKG_CHECK_MODULES([LIBVIRT], [libvirt>= 0.9.0]) > AC_SUBST([LIBVIRT_CFLAGS]) > AC_SUBST([LIBVIRT_LIBS]) > CPPFLAGS="$CPPFLAGS $LIBVIRT_CFLAGS" > diff --git a/libvirt-cim.spec.in b/libvirt-cim.spec.in > index 5b7267b..f3289db 100644 > --- a/libvirt-cim.spec.in > +++ b/libvirt-cim.spec.in > @@ -10,12 +10,12 @@ Source: libvirt-cim-%{version}.tar.gz > BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root > URL: http://libvirt.org/CIM/ > Requires: libxml2>= 2.6.0 > -Requires: libvirt>= 0.6.3 > +Requires: libvirt>= 0.9.0 > Requires: unzip > Requires: tog-pegasus > BuildRequires: libcmpiutil>= 0.5.4 > BuildRequires: tog-pegasus-devel > -BuildRequires: libvirt-devel>= 0.6.3 > +BuildRequires: libvirt-devel>= 0.9.0 > > # In RHEL5 uuid-devel is provided by e2fsprogs > %if 0%{?el5} > diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c > index 61893c3..556fe1f 100644 > --- a/libxkutil/misc_util.c > +++ b/libxkutil/misc_util.c > @@ -607,6 +607,8 @@ bool libvirt_cim_init(void) > ret = virInitialize(); > if (ret == 0) > libvirt_initialized = 1; > + > + virEventRegisterDefaultImpl(); > } > pthread_mutex_unlock(&libvirt_mutex); > } > diff --git a/src/Virt_ComputerSystem.c b/src/Virt_ComputerSystem.c > index 098b07a..582253a 100644 > --- a/src/Virt_ComputerSystem.c > +++ b/src/Virt_ComputerSystem.c > @@ -492,6 +492,7 @@ static CMPIStatus set_properties(const CMPIBroker *broker, > } > > if (!set_name_from_dom(dom, instance)) { > + CU_DEBUG("Unable to get domain name"); > virt_set_status(broker,&s, > CMPI_RC_ERR_FAILED, > virDomainGetConnect(dom), > @@ -500,6 +501,7 @@ static CMPIStatus set_properties(const CMPIBroker *broker, > } > > if (!set_uuid_from_dom(dom, instance,&uuid)) { > + CU_DEBUG("Unable to get domain uuid"); > virt_set_status(broker,&s, > CMPI_RC_ERR_FAILED, > virDomainGetConnect(dom), > @@ -514,6 +516,7 @@ static CMPIStatus set_properties(const CMPIBroker *broker, > } > > if (!set_state_from_dom(broker, dom, instance)) { > + CU_DEBUG("Unable to get domain info"); > virt_set_status(broker,&s, > CMPI_RC_ERR_FAILED, > virDomainGetConnect(dom), > @@ -660,6 +663,7 @@ CMPIStatus get_domain_by_name(const CMPIBroker *broker, > > conn = connect_by_classname(broker, CLASSNAME(reference),&s); > if (conn == NULL) { > + CU_DEBUG("No such instance"); > cu_statusf(broker,&s, > CMPI_RC_ERR_NOT_FOUND, > "No such instance."); > @@ -668,6 +672,7 @@ CMPIStatus get_domain_by_name(const CMPIBroker *broker, > > dom = virDomainLookupByName(conn, name); > if (dom == NULL) { > + CU_DEBUG("Domain '%s' does not exist", name); > virt_set_status(broker,&s, > CMPI_RC_ERR_NOT_FOUND, > conn, > @@ -681,8 +686,10 @@ CMPIStatus get_domain_by_name(const CMPIBroker *broker, > conn, > dom, > &inst); > - if (s.rc != CMPI_RC_OK) > + if (s.rc != CMPI_RC_OK) { > + CU_DEBUG("Unable to retrieve instance from domain"); > goto out; > + } > > *_inst = inst; > > @@ -766,39 +773,6 @@ DEFAULT_DI(); > DEFAULT_EQ(); > DEFAULT_INST_CLEANUP(); > > -static bool trigger_mod_indication(const CMPIContext *context, > - CMPIInstance *prev_inst, > - const CMPIObjectPath *ref) > -{ > - CMPIStatus s = {CMPI_RC_OK, NULL}; > - const char *ind_name = "ComputerSystemModifiedIndication"; > - CMPIInstance *ind = NULL; > - char *type = NULL; > - > - CU_DEBUG("Preparing ComputerSystem indication"); > - > - ind = get_typed_instance(_BROKER, > - CLASSNAME(ref), > - ind_name, > - NAMESPACE(ref)); > - if (ind == NULL) { > - CU_DEBUG("Failed to create ind '%s'", ind_name); > - goto out; > - } > - > - CU_DEBUG("Setting PreviousInstance"); > - CMSetProperty(ind, "PreviousInstance", > - (CMPIValue *)&prev_inst, CMPI_instance); > - > - type = get_typed_class(CLASSNAME(ref), ind_name); > - > - s = stdi_raise_indication(_BROKER, context, type, NAMESPACE(ref), ind); > - > - out: > - free(type); > - return s.rc == CMPI_RC_OK; > -} > - > static int xen_scheduler_params(struct infostore_ctx *ctx, > virSchedParameter **params) > { > @@ -1253,7 +1227,6 @@ static CMPIStatus state_change(CMPIMethodMI *self, > int ret; > const char *name = NULL; > uint32_t rc = 1; > - bool ind_rc; > > ret = cu_get_u16_arg(argsin, "RequestedState",&state); > if (ret != CMPI_RC_OK) { > @@ -1282,13 +1255,9 @@ static CMPIStatus state_change(CMPIMethodMI *self, > > s = __state_change(name, state, reference); > > - if (s.rc == CMPI_RC_OK) { > - ind_rc= trigger_mod_indication(context, prev_inst, reference); > - if (!ind_rc) > - CU_DEBUG("Unable to trigger indication"); > - > + if (s.rc == CMPI_RC_OK) > rc = 0; > - } > + > out: > CMReturnData(results,&rc, CMPI_uint32); > > diff --git a/src/Virt_ComputerSystemIndication.c > b/src/Virt_ComputerSystemIndication.c > index a00444d..0df73f3 100644 > --- a/src/Virt_ComputerSystemIndication.c > +++ b/src/Virt_ComputerSystemIndication.c > @@ -25,13 +25,13 @@ > #include<string.h> > #include<stdlib.h> > #include<pthread.h> > -#include<time.h> > > #include<cmpidt.h> > #include<cmpift.h> > #include<cmpimacs.h> > > #include<libvirt/libvirt.h> > +#include<libvirt/virterror.h> > > #include<libcmpiutil/libcmpiutil.h> > #include<misc_util.h> > @@ -44,158 +44,187 @@ > #include "Virt_ComputerSystemIndication.h" > #include "Virt_HostSystem.h" > > -static const CMPIBroker *_BROKER; > > #define CSI_NUM_PLATFORMS 3 > -enum CSI_PLATFORMS {CSI_XEN, > - CSI_KVM, > - CSI_LXC, > +enum CSI_PLATFORMS { > + CSI_XEN, > + CSI_KVM, > + CSI_LXC, > }; > > -static CMPI_THREAD_TYPE thread_id[CSI_NUM_PLATFORMS]; > -static int active_filters[CSI_NUM_PLATFORMS]; > - > -enum CS_EVENTS {CS_CREATED, > - CS_DELETED, > - CS_MODIFIED, > +#define CS_NUM_EVENTS 3 > +enum CS_EVENTS { > + CS_CREATED, > + CS_DELETED, > + CS_MODIFIED, > }; > > -static pthread_cond_t lifecycle_cond = PTHREAD_COND_INITIALIZER; > -static pthread_mutex_t lifecycle_mutex = PTHREAD_MUTEX_INITIALIZER; > -static bool lifecycle_enabled = 0; > - > -#define WAIT_TIME 60 > -#define FAIL_WAIT_TIME 2 > - > -struct dom_xml { > +typedef struct _csi_dom_xml_t csi_dom_xml_t; > +struct _csi_dom_xml_t { > char uuid[VIR_UUID_STRING_BUFLEN]; > + char *name; > char *xml; > - enum {DOM_OFFLINE, > - DOM_ONLINE, > - DOM_PAUSED, > - DOM_CRASHED, > - DOM_GONE, > - } state; > + csi_dom_xml_t *next; > + csi_dom_xml_t *prev; > +}; > + > +typedef struct _csi_thread_data_t csi_thread_data_t; > +struct _csi_thread_data_t { > + CMPI_THREAD_TYPE id; > + int active_filters; > + int dom_count; > + csi_dom_xml_t *dom_list; > + struct ind_args *args; > }; > > -static void free_dom_xml (struct dom_xml dom) > +static const CMPIBroker *_BROKER; > +static pthread_mutex_t lifecycle_mutex = PTHREAD_MUTEX_INITIALIZER; > +static bool lifecycle_enabled = false; > +static csi_thread_data_t csi_thread_data[CSI_NUM_PLATFORMS] = {{0}, {0}, {0}}; > + > +/* > + * Domain list manipulation > + */ > +static void csi_dom_xml_free(csi_dom_xml_t *dom) > { > - free(dom.xml); > + free(dom->xml); > + free(dom->name); > + free(dom); > } > > -static char *sys_name_from_xml(char *xml) > +static int csi_dom_xml_set(csi_dom_xml_t *dom, virDomainPtr dom_ptr, > CMPIStatus *s) > { > - char *tmp = NULL; > - char *name = NULL; > - int rc; > + const char *name; > > - tmp = strstr(xml, "<name>"); > - if (tmp == NULL) > - goto out; > + name = virDomainGetName(dom_ptr); > + if (name == NULL) { > + cu_statusf(_BROKER, s, > + CMPI_RC_ERR_FAILED, > + "Failed to get domain name"); > + return -1; > + } > > - rc = sscanf(tmp, "<name>%a[^<]s</name>",&name); > - if (rc != 1) > - name = NULL; > + dom->name = strdup(name); > + > + /* xml */ > + dom->xml = virDomainGetXMLDesc(dom_ptr, VIR_DOMAIN_XML_SECURE); > + if (dom->xml == NULL) { > + cu_statusf(_BROKER, s, > + CMPI_RC_ERR_FAILED, > + "Failed to get xml desc"); > + return -1; > + } > > - out: > - return name; > + return 0; > } > > -static int dom_state(virDomainPtr dom) > +static csi_dom_xml_t *csi_dom_xml_new(virDomainPtr dom_ptr, CMPIStatus *s) > { > - virDomainInfo info; > - int ret; > + int rc; > + csi_dom_xml_t *dom; > + > + dom = calloc(1, sizeof(*dom)); > + if (dom == NULL) > + return NULL; > + > + /* uuid */ > + rc = virDomainGetUUIDString(dom_ptr, dom->uuid); > + if (rc == -1) { > + cu_statusf(_BROKER, s, > + CMPI_RC_ERR_FAILED, > + "Failed to get domain UUID"); > + goto error; > + } > + > + if (csi_dom_xml_set(dom, dom_ptr, s) == -1) > + goto error; > > - ret = virDomainGetInfo(dom,&info); > - if (ret != 0) > - return DOM_GONE; > + return dom; > > - switch (info.state) { > - case VIR_DOMAIN_NOSTATE: > - case VIR_DOMAIN_RUNNING: > - case VIR_DOMAIN_BLOCKED: > - return DOM_ONLINE; > + error: > + csi_dom_xml_free(dom); > + return NULL; > +} > > - case VIR_DOMAIN_PAUSED: > - return DOM_PAUSED; > +static void csi_thread_dom_list_append(csi_thread_data_t *thread, > + csi_dom_xml_t *dom) > +{ > + /* empty list */ > + if (thread->dom_list == NULL) { > + dom->next = dom->prev = dom; > + thread->dom_list = dom; > + goto end; > + } > > - case VIR_DOMAIN_SHUTOFF: > - return DOM_OFFLINE; > + dom->next = thread->dom_list; > + dom->prev = thread->dom_list->prev; > > - case VIR_DOMAIN_CRASHED: > - return DOM_CRASHED; > + thread->dom_list->prev->next = dom; > + thread->dom_list->prev = dom; > > - default: > - return DOM_GONE; > - }; > + end: > + thread->dom_count += 1; > } > > -static CMPIStatus doms_to_xml(struct dom_xml **dom_xml_list, > - virDomainPtr *dom_ptr_list, > - int dom_ptr_count) > +static csi_dom_xml_t *csi_thread_dom_list_find(csi_thread_data_t *thread, > + const char *uuid) > { > - int i; > - int rc; > - CMPIStatus s = {CMPI_RC_OK, NULL}; > + csi_dom_xml_t *dom; > > - *dom_xml_list = calloc(dom_ptr_count, sizeof(struct dom_xml)); > - for (i = 0; i< dom_ptr_count; i++) { > - rc = virDomainGetUUIDString(dom_ptr_list[i], > - (*dom_xml_list)[i].uuid); > - if (rc == -1) { > - cu_statusf(_BROKER,&s, > - CMPI_RC_ERR_FAILED, > - "Failed to get UUID"); > - /* If any domain fails, we fail. */ > - break; > - } > + if (thread->dom_list == NULL) > + return NULL; > > - (*dom_xml_list)[i].xml = virDomainGetXMLDesc(dom_ptr_list[i], > - > VIR_DOMAIN_XML_SECURE); > - if ((*dom_xml_list)[i].xml == NULL) { > - cu_statusf(_BROKER,&s, > - CMPI_RC_ERR_FAILED, > - "Failed to get xml desc"); > - break; > - } > + dom = thread->dom_list; > > - (*dom_xml_list)[i].state = dom_state(dom_ptr_list[i]); > + do { > + if (STREQ(dom->uuid, uuid)) > + return dom; > + > + dom = dom->next; > + } while (dom != thread->dom_list); > + > + return NULL; > +} > + > +static void csi_thread_dom_list_remove(csi_thread_data_t *thread, > + csi_dom_xml_t *dom) > +{ > + if (dom->next == dom) { /* Only one node */ > + thread->dom_list = NULL; > + } else { > + if (thread->dom_list == dom) /* First node */ > + thread->dom_list = dom->next; > + > + dom->prev->next = dom->next; > + dom->next->prev = dom->prev; > } > - > - return s; > + > + thread->dom_count -= 1; > + > + csi_dom_xml_free(dom); > } > > -static bool dom_changed(struct dom_xml prev_dom, > - struct dom_xml *cur_xml, > - int cur_count) > +static void csi_thread_dom_list_free(csi_thread_data_t *thread) > { > - int i; > - bool ret = false; > - > - for (i = 0; i< cur_count; i++) { > - if (strcmp(cur_xml[i].uuid, prev_dom.uuid) != 0) > - continue; > - > - if (strcmp(cur_xml[i].xml, prev_dom.xml) != 0) { > - CU_DEBUG("Domain config changed"); > - ret = true; > - } > + while(thread->dom_list != NULL) > + csi_thread_dom_list_remove(thread, thread->dom_list); > +} > > - if (prev_dom.state != cur_xml[i].state) { > - CU_DEBUG("Domain state changed"); > - ret = true; > - } > +static void csi_free_thread_data(void *data) > +{ > + csi_thread_data_t *thread = (csi_thread_data_t *) data; > > - break; > - } > - > - return ret; > + if (data == NULL) > + return; > + > + csi_thread_dom_list_free(thread); > + stdi_free_ind_args(&thread->args); > } > > void set_source_inst_props(const CMPIBroker *broker, > - const CMPIContext *context, > - const CMPIObjectPath *ref, > - CMPIInstance *ind) > + const CMPIContext *context, > + const CMPIObjectPath *ref, > + CMPIInstance *ind) > { > const char *host; > const char *hostccn; > @@ -225,7 +254,7 @@ static bool _do_indication(const CMPIBroker *broker, > CMPIInstance *prev_inst, > CMPIInstance *affected_inst, > int ind_type, > - char *prefix, > + const char *prefix, > struct ind_args *args) > { > const char *ind_type_name = NULL; > @@ -254,10 +283,10 @@ static bool _do_indication(const CMPIBroker *broker, > ind_type_name, > args->ns); > > - /* Generally report errors and hope to continue, since we have no one > + /* Generally report errors and hope to continue, since we have no one > to actually return status to. */ > if (ind == NULL) { > - CU_DEBUG("Failed to create ind, type '%s:%s_%s'", > + CU_DEBUG("Failed to create ind, type '%s:%s_%s'", > args->ns, > prefix, > ind_type_name); > @@ -279,14 +308,15 @@ static bool _do_indication(const CMPIBroker *broker, > CU_DEBUG("problem getting affected_op: '%s'", s.msg); > goto out; > } > + > CMSetNameSpace(affected_op, args->ns); > > uuid = CMGetProperty(affected_inst, "UUID",&s); > - CMSetProperty(ind, "IndicationIdentifier", > + CMSetProperty(ind, "IndicationIdentifier", > (CMPIValue *)&(uuid.value), CMPI_string); > > timestamp = CMNewDateTime(broker,&s); > - CMSetProperty(ind, "IndicationTime", > + CMSetProperty(ind, "IndicationTime", > (CMPIValue *)&timestamp, CMPI_dateTime); > > if (ind_type == CS_MODIFIED) { > @@ -313,34 +343,6 @@ static bool _do_indication(const CMPIBroker *broker, > return ret; > } > > -static bool wait_for_event(int wait_time) > -{ > - struct timespec timeout; > - int ret; > - > - > - clock_gettime(CLOCK_REALTIME,&timeout); > - timeout.tv_sec += wait_time; > - > - ret = pthread_cond_timedwait(&lifecycle_cond, > -&lifecycle_mutex, > -&timeout); > - > - return true; > -} > - > -static bool dom_in_list(char *uuid, int count, struct dom_xml *list) > -{ > - int i; > - > - for (i = 0; i< count; i++) { > - if (STREQ(uuid, list[i].uuid)) > - return true; > - } > - > - return false; > -} > - > static bool set_instance_state(CMPIInstance *instance) > { > CMPIStatus s = {CMPI_RC_OK, NULL}; > @@ -356,7 +358,7 @@ static bool set_instance_state(CMPIInstance *instance) > cim_state_other = CMNewString(_BROKER, "Guest destroyed",&s); > CMSetProperty(instance, "EnabledState", > (CMPIValue *)&cim_state, CMPI_uint16); > - CMSetProperty(instance, "OtherEnabledState", > + CMSetProperty(instance, "OtherEnabledState", > (CMPIValue *)&cim_state_other, CMPI_string); > > health_state = CIM_HEALTH_UNKNOWN; > @@ -382,13 +384,13 @@ static bool set_instance_state(CMPIInstance *instance) > req_state = CIM_STATE_UNKNOWN; > CMSetProperty(instance, "RequestedState", > (CMPIValue *)&req_state, CMPI_uint16); > - > + > return true; > } > > -static bool create_deleted_guest_inst(char *xml, > - char *namespace, > - char *prefix, > +static bool create_deleted_guest_inst(const char *xml, > + const char *namespace, > + const char *prefix, > CMPIInstance **inst) > { > bool rc = false; > @@ -402,18 +404,18 @@ static bool create_deleted_guest_inst(char *xml, > goto out; > } > > - s = instance_from_dominfo(_BROKER, > - namespace, > + s = instance_from_dominfo(_BROKER, > + namespace, > prefix, > - dominfo, > - inst); > + dominfo, > + inst); > if (s.rc != CMPI_RC_OK) { > CU_DEBUG("instance from domain info error: %s", s.msg); > goto out; > } > > rc = set_instance_state(*inst); > - if (!rc) > + if (!rc) > CU_DEBUG("Error setting instance state"); > > out: > @@ -422,14 +424,12 @@ static bool create_deleted_guest_inst(char *xml, > return rc; > } > > -static bool async_ind(CMPIContext *context, > +static bool async_ind(struct ind_args *args, > int ind_type, > - struct dom_xml prev_dom, > - char *prefix, > - struct ind_args *args) > + csi_dom_xml_t *dom, > + const char *prefix) > { > bool rc = false; > - char *name = NULL; > char *cn = NULL; > CMPIObjectPath *op; > CMPIInstance *prev_inst; > @@ -441,13 +441,6 @@ static bool async_ind(CMPIContext *context, > return false; > } > > - name = sys_name_from_xml(prev_dom.xml); > - CU_DEBUG("Name for system: '%s'", name); > - if (name == NULL) { > - rc = false; > - goto out; > - } > - > cn = get_typed_class(prefix, "ComputerSystem"); > > op = CMNewObjectPath(_BROKER, args->ns, cn,&s); > @@ -457,15 +450,32 @@ static bool async_ind(CMPIContext *context, > } > > if (ind_type == CS_CREATED || ind_type == CS_MODIFIED) { > - s = get_domain_by_name(_BROKER, op, name,&affected_inst); > - if (s.rc != CMPI_RC_OK) { > + s = get_domain_by_name(_BROKER, op, dom->name,&affected_inst); > + > + /* If domain is not found, we create the instance from the data > + previously stored */ > + if (s.rc == CMPI_RC_ERR_NOT_FOUND) { > + rc = create_deleted_guest_inst(dom->xml, > + args->ns, > + prefix, > +&affected_inst); > + if (!rc) { > + CU_DEBUG("Could not recreate guest instance"); > + goto out; > + } > + > + s.rc = CMPI_RC_OK; > + } > + > + if (s.rc != CMPI_RC_OK) { > CU_DEBUG("domain by name error"); > goto out; > } > + > } else if (ind_type == CS_DELETED) { > - rc = create_deleted_guest_inst(prev_dom.xml, > - args->ns, > - prefix, > + rc = create_deleted_guest_inst(dom->xml, > + args->ns, > + prefix, > &affected_inst); > if (!rc) { > CU_DEBUG("Could not recreate guest instance"); > @@ -476,52 +486,134 @@ static bool async_ind(CMPIContext *context, > goto out; > } > > - /* FIXME: We are unable to get the previous CS instance after it has > + /* FIXME: We are unable to get the previous CS instance after it has > been modified. Consider keeping track of the previous > - state in the place we keep track of the requested state */ > + state in the place we keep track of the requested state */ > prev_inst = affected_inst; > > - CMSetProperty(affected_inst, "Name", > - (CMPIValue *)name, CMPI_chars); > + CMSetProperty(affected_inst, "Name", > + (CMPIValue *) dom->name, CMPI_chars); > CMSetProperty(affected_inst, "UUID", > - (CMPIValue *)prev_dom.uuid, CMPI_chars); > + (CMPIValue *) dom->uuid, CMPI_chars); > > - rc = _do_indication(_BROKER, context, prev_inst, affected_inst, > + rc = _do_indication(_BROKER, args->context, prev_inst, affected_inst, > ind_type, prefix, args); > > out: > free(cn); > - free(name); > return rc; > } > > -static int platform_from_class(const char *cn) > +static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) > { > - if (STARTS_WITH(cn, "Xen")) > - return CSI_XEN; > - else if (STARTS_WITH(cn, "KVM")) > - return CSI_KVM; > - else if (STARTS_WITH(cn, "LXC")) > - return CSI_LXC; > - else > - return -1; > + virDomainPtr *dom_ptr_list; > + csi_dom_xml_t *dom; > + CMPIStatus s = {CMPI_RC_OK, NULL}; > + int i, count; > + > + csi_thread_dom_list_free(thread); > + > + count = get_domain_list(conn,&dom_ptr_list); > + > + for (i = 0; i< count; i++) { > + dom = csi_dom_xml_new(dom_ptr_list[i],&s); > + if (dom == NULL) { > + CU_DEBUG("Failed to get domain info %s", s.msg); > + break; > + } > + > + csi_thread_dom_list_append(thread, dom); > + } > + > + free_domain_list(dom_ptr_list, count); > + free(dom_ptr_list); > + > + return s.rc; > +} > + > +static int csi_domain_event_cb(virConnectPtr conn, > + virDomainPtr dom, > + int event, > + int detail, > + void *data) > +{ > + int cs_event = CS_MODIFIED; > + csi_thread_data_t *thread = (csi_thread_data_t *) data; > + csi_dom_xml_t *dom_xml = NULL; > + char *prefix = class_prefix_name(thread->args->classname); > + CMPIStatus s = {CMPI_RC_OK, NULL}; > + > + CU_DEBUG("Event: Domain %s(%d) event: %d detail: %d\n", > + virDomainGetName(dom), virDomainGetID(dom), event, detail); > + > + switch (event) { > + case VIR_DOMAIN_EVENT_DEFINED: > + if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED) { > + CU_DEBUG("Domain defined"); > + cs_event = CS_CREATED; > + dom_xml = csi_dom_xml_new(dom,&s); > + } else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED) { > + CU_DEBUG("Domain modified"); > + cs_event = CS_MODIFIED; > + } > + > + break; > + > + case VIR_DOMAIN_EVENT_UNDEFINED: > + CU_DEBUG("Domain undefined"); > + cs_event = CS_DELETED; > + break; > + > + default: /* STARTED, SUSPENDED, RESUMED, STOPPED, SHUTDOWN */ > + CU_DEBUG("Domain modified"); > + cs_event = CS_MODIFIED; > + break; > + } > + > + if (cs_event != CS_CREATED) { > + char uuid[VIR_UUID_STRING_BUFLEN] = {0}; > + virDomainGetUUIDString(dom,&uuid[0]); > + dom_xml = csi_thread_dom_list_find(thread, uuid); > + } > + > + if (dom_xml == NULL) { > + CU_DEBUG("Domain not found in current list"); > + goto end; > + } > + > + async_ind(thread->args, cs_event, dom_xml, prefix); > + > + /* Update the domain list accordingly */ > + if (event == VIR_DOMAIN_EVENT_DEFINED) { > + if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED) { > + csi_thread_dom_list_append(thread, dom_xml); > + } else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED) { > + free(dom_xml->name); > + free(dom_xml->xml); > + csi_dom_xml_set(dom_xml, dom,&s); > + } > + } else if (event == VIR_DOMAIN_EVENT_DEFINED&& > + detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) { > + csi_thread_dom_list_remove(thread, dom_xml); > + } > + > + end: > + free(prefix); > + return 0; > } > > static CMPI_THREAD_RETURN lifecycle_thread(void *params) > { > - struct ind_args *args = (struct ind_args *)params; > - CMPIContext *context = args->context; > - CMPIStatus s; > - int prev_count; > - int cur_count; > - virDomainPtr *tmp_list; > - struct dom_xml *cur_xml = NULL; > - struct dom_xml *prev_xml = NULL; > - virConnectPtr conn; > + csi_thread_data_t *thread = (csi_thread_data_t *) params; > + struct ind_args *args = thread->args; > char *prefix = class_prefix_name(args->classname); > - int platform = platform_from_class(args->classname); > > - if (prefix == NULL || platform == -1) > + virConnectPtr conn; > + > + CMPIStatus s; > + int cb_id; > + > + if (prefix == NULL) > goto init_out; > > conn = connect_by_classname(_BROKER, args->classname,&s); > @@ -531,86 +623,65 @@ static CMPI_THREAD_RETURN lifecycle_thread(void *params) > goto conn_out; > } > > - pthread_mutex_lock(&lifecycle_mutex); > - > - CBAttachThread(_BROKER, context); > - > - prev_count = get_domain_list(conn,&tmp_list); > - s = doms_to_xml(&prev_xml, tmp_list, prev_count); > - if (s.rc != CMPI_RC_OK) > - CU_DEBUG("doms_to_xml failed. Attempting to continue."); > - free_domain_list(tmp_list, prev_count); > - free(tmp_list); > - > - CU_DEBUG("Entering CSI event loop (%s)", prefix); > - while (active_filters[platform]> 0) { > - int i; > - bool res; > - bool failure = false; > - > - cur_count = get_domain_list(conn,&tmp_list); > - s = doms_to_xml(&cur_xml, tmp_list, cur_count); > - if (s.rc != CMPI_RC_OK) { > - CU_DEBUG("doms_to_xml failed. retry in %d seconds", > - FAIL_WAIT_TIME); > - failure = true; > - goto fail; > - } > - > - free_domain_list(tmp_list, cur_count); > - free(tmp_list); > - > - for (i = 0; i< cur_count; i++) { > - res = dom_in_list(cur_xml[i].uuid, prev_count, > prev_xml); > - if (!res) > - async_ind(context, CS_CREATED, > - cur_xml[i], prefix, args); > - > - } > + /* register callback */ > + cb_id = virConnectDomainEventRegisterAny(conn, NULL, > + VIR_DOMAIN_EVENT_ID_LIFECYCLE, > + VIR_DOMAIN_EVENT_CALLBACK(csi_domain_event_cb), > + params, csi_free_thread_data); > > - for (i = 0; i< prev_count; i++) { > - res = dom_in_list(prev_xml[i].uuid, cur_count, > cur_xml); > - if (!res) > - async_ind(context, CS_DELETED, > - prev_xml[i], prefix, args); > - } > + if (cb_id == -1) { > + CU_DEBUG("Failed to register domain event watch for '%s'", > + args->classname) > + goto cb_out; > + } > > - for (i = 0; i< prev_count; i++) { > - res = dom_changed(prev_xml[i], cur_xml, cur_count); > - if (res) { > - async_ind(context, CS_MODIFIED, > - prev_xml[i], prefix, args); > + CBAttachThread(_BROKER, args->context); > > - } > - free_dom_xml(prev_xml[i]); > - } > + /* Get currently defined domains */ > + if (update_domain_list(conn, thread) != CMPI_RC_OK) > + goto end; > > - fail: > - if (failure) { > - wait_for_event(FAIL_WAIT_TIME); > - } else { > - free(prev_xml); > - prev_xml = cur_xml; > - prev_count = cur_count; > > - wait_for_event(WAIT_TIME); > + CU_DEBUG("Entering CSI event loop (%s)", prefix); > + while (thread->active_filters> 0) { > + if (virEventRunDefaultImpl()< 0) { > + virErrorPtr err = virGetLastError(); > + CU_DEBUG("Failed to run event loop: %s\n", > + err&& err->message ? err->message : "Unknown error"); > } > } > > CU_DEBUG("Exiting CSI event loop (%s)", prefix); > > - thread_id[platform] = 0; > + end: > + virConnectDomainEventDeregisterAny(conn, cb_id); > > - pthread_mutex_unlock(&lifecycle_mutex); > - stdi_free_ind_args(&args); > + cb_out: > + > + thread->id = 0; > + thread->active_filters = 0; > + > + if (thread->args != NULL) > + stdi_free_ind_args(&thread->args); > > conn_out: > virConnectClose(conn); > > init_out: > free(prefix); > + return (CMPI_THREAD_RETURN) 0; > +} > > - return NULL; > +static int platform_from_class(const char *cn) > +{ > + if (STARTS_WITH(cn, "Xen")) > + return CSI_XEN; > + else if (STARTS_WITH(cn, "KVM")) > + return CSI_KVM; > + else if (STARTS_WITH(cn, "LXC")) > + return CSI_LXC; > + else > + return -1; > } > > static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, > @@ -622,8 +693,10 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, > { > CMPIStatus s = {CMPI_RC_OK, NULL}; > struct std_indication_ctx *_ctx; > - struct ind_args *args; > + struct ind_args *args = NULL; > int platform; > + bool error = false; > + csi_thread_data_t *thread = NULL; > > CU_DEBUG("ActivateFilter for %s", CLASSNAME(op)); > > @@ -632,7 +705,7 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, > _ctx = (struct std_indication_ctx *)mi->hdl; > > if (CMIsNullObject(op)) { > - cu_statusf(_BROKER,&s, > + cu_statusf(_BROKER,&s, > CMPI_RC_ERR_FAILED, > "No ObjectPath given"); > goto out; > @@ -647,13 +720,17 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, > goto out; > } > > - if (thread_id[platform] == 0) { > - args = malloc(sizeof(struct ind_args)); > + thread =&csi_thread_data[platform]; > + thread->active_filters += 1; > + > + if (thread->id == 0) { > + args = malloc(sizeof(*args)); > if (args == NULL) { > CU_DEBUG("Failed to allocate ind_args"); > cu_statusf(_BROKER,&s, > CMPI_RC_ERR_FAILED, > "Unable to allocate ind_args"); > + error = true; > goto out; > } > > @@ -663,7 +740,7 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, > cu_statusf(_BROKER,&s, > CMPI_RC_ERR_FAILED, > "Unable to create thread context"); > - free(args); > + error = true; > goto out; > } > > @@ -671,15 +748,17 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, > args->classname = strdup(CLASSNAME(op)); > args->_ctx = _ctx; > > - active_filters[platform] += 1; > - > - thread_id[platform] = _BROKER->xft->newThread(lifecycle_thread, > - args, > - 0); > - } else > - active_filters[platform] += 1; > + thread->args = args; > + thread->id = _BROKER->xft->newThread(lifecycle_thread, > + thread, 0); > + } > > out: > + if (error == true) { > + thread->active_filters -= 1; > + free(args); > + } > + > pthread_mutex_unlock(&lifecycle_mutex); > > return s; > @@ -705,11 +784,11 @@ static CMPIStatus DeActivateFilter(CMPIIndicationMI* mi, > goto out; > } > > + > pthread_mutex_lock(&lifecycle_mutex); > - active_filters[platform] -= 1; > + csi_thread_data[platform].active_filters -= 1; > pthread_mutex_unlock(&lifecycle_mutex); > > - pthread_cond_signal(&lifecycle_cond); > out: > return s; > } > @@ -736,13 +815,6 @@ static _EI_RTYPE DisableIndications(CMPIIndicationMI* mi, > _EI_RET(); > } > > -static CMPIStatus trigger_indication(const CMPIContext *context) > -{ > - CU_DEBUG("triggered"); > - pthread_cond_signal(&lifecycle_cond); > - return(CMPIStatus){CMPI_RC_OK, NULL}; > -} > - > DECLARE_FILTER(xen_created, "Xen_ComputerSystemCreatedIndication"); > DECLARE_FILTER(xen_deleted, "Xen_ComputerSystemDeletedIndication"); > DECLARE_FILTER(xen_modified, "Xen_ComputerSystemModifiedIndication"); > @@ -766,126 +838,7 @@ static struct std_ind_filter *filters[] = { > NULL, > }; > > -static CMPIInstance *get_prev_inst(const CMPIBroker *broker, > - const CMPIInstance *ind, > - CMPIStatus *s) > -{ > - CMPIData data; > - CMPIInstance *prev_inst = NULL; > - > - data = CMGetProperty(ind, "PreviousInstance", s); > - if (s->rc != CMPI_RC_OK || CMIsNullValue(data)) { > - cu_statusf(broker, s, > - CMPI_RC_ERR_NO_SUCH_PROPERTY, > - "Unable to get PreviousInstance of the indication"); > - goto out; > - } > - > - if (data.type != CMPI_instance) { > - cu_statusf(broker, s, > - CMPI_RC_ERR_TYPE_MISMATCH, > - "Indication SourceInstance is of unexpected type"); > - goto out; > - } > - > - prev_inst = data.value.inst; > - > - out: > - return prev_inst; > -} > - > -static CMPIStatus raise_indication(const CMPIBroker *broker, > - const CMPIContext *ctx, > - const CMPIObjectPath *ref, > - const CMPIInstance *ind) > -{ > - CMPIStatus s = {CMPI_RC_OK, NULL}; > - CMPIInstance *prev_inst; > - CMPIInstance *src_inst; > - CMPIObjectPath *_ref = NULL; > - struct std_indication_ctx *_ctx = NULL; > - struct ind_args *args = NULL; > - char *prefix = NULL; > - bool rc; > - > - if (!lifecycle_enabled) { > - cu_statusf(_BROKER,&s, > - CMPI_RC_ERR_FAILED, > - "CSI not enabled, skipping indication delivery"); > - goto out; > - } > - > - prev_inst = get_prev_inst(broker, ind,&s); > - if (s.rc != CMPI_RC_OK || CMIsNullObject(prev_inst)) > - goto out; > - > - _ref = CMGetObjectPath(prev_inst,&s); > - if (s.rc != CMPI_RC_OK) { > - cu_statusf(broker,&s, > - CMPI_RC_ERR_FAILED, > - "Unable to get a reference to the guest"); > - goto out; > - } > - > - /* FIXME: This is a Pegasus work around. Pegsus loses the namespace > - when an ObjectPath is pulled from an instance */ > - if (STREQ(NAMESPACE(_ref), "")) > - CMSetNameSpace(_ref, "root/virt"); > - > - s = get_domain_by_ref(broker, _ref,&src_inst); > - if (s.rc != CMPI_RC_OK || CMIsNullObject(src_inst)) > - goto out; > - > - _ctx = malloc(sizeof(struct std_indication_ctx)); > - if (_ctx == NULL) { > - cu_statusf(broker,&s, > - CMPI_RC_ERR_FAILED, > - "Unable to allocate indication context"); > - goto out; > - } > - > - _ctx->brkr = broker; > - _ctx->handler = NULL; > - _ctx->filters = filters; > - _ctx->enabled = lifecycle_enabled; > - > - args = malloc(sizeof(struct ind_args)); > - if (args == NULL) { > - cu_statusf(broker,&s, > - CMPI_RC_ERR_FAILED, > - "Unable to allocate ind_args"); > - goto out; > - } > - > - args->ns = strdup(NAMESPACE(_ref)); > - args->classname = strdup(CLASSNAME(_ref)); > - args->_ctx = _ctx; > - > - prefix = class_prefix_name(args->classname); > - > - rc = _do_indication(broker, ctx, prev_inst, src_inst, > - CS_MODIFIED, prefix, args); > - > - if (!rc) { > - cu_statusf(_BROKER,&s, > - CMPI_RC_ERR_FAILED, > - "Unable to generate indication"); > - } > - > - out: > - if (args != NULL) > - stdi_free_ind_args(&args); > - > - if (_ctx != NULL) > - free(_ctx); > - > - free(prefix); > - return s; > -} > - > static struct std_indication_handler csi = { > - .raise_fn = raise_indication, > - .trigger_fn = trigger_indication, > .activate_fn = ActivateFilter, > .deactivate_fn = DeActivateFilter, > .enable_fn = EnableIndications, > @@ -896,10 +849,10 @@ DEFAULT_IND_CLEANUP(); > DEFAULT_AF(); > DEFAULT_MP(); > > -STDI_IndicationMIStub(, > +STDI_IndicationMIStub(, > Virt_ComputerSystemIndicationProvider, > _BROKER, > - libvirt_cim_init(), > + libvirt_cim_init(), > &csi, > filters); > > diff --git a/src/Virt_ComputerSystemIndication.h > b/src/Virt_ComputerSystemIndication.h > index 0f8f1b7..594c3ed 100644 > --- a/src/Virt_ComputerSystemIndication.h > +++ b/src/Virt_ComputerSystemIndication.h > @@ -24,11 +24,6 @@ > #include<cmpidt.h> > #include<stdbool.h> > > -bool cs_lifecycle_indication(const CMPIBroker *broker, > - const CMPIContext *ctx, > - const CMPIObjectPath *newsystem, > - char *type); > - > void set_source_inst_props(const CMPIBroker *broker, > const CMPIContext *context, > const CMPIObjectPath *ref, > diff --git a/src/Virt_VirtualSystemManagementService.c > b/src/Virt_VirtualSystemManagementService.c > index 4d26429..46bc9b5 100644 > --- a/src/Virt_VirtualSystemManagementService.c > +++ b/src/Virt_VirtualSystemManagementService.c > @@ -1894,7 +1894,7 @@ static CMPIStatus raise_rasd_indication(const > CMPIContext *context, > CMPIObjectPath *op = NULL; > int i; > > - CU_DEBUG("raise_rasd_indication"); > + CU_DEBUG("raise_rasd_indication %s", base_type); > > type = get_typed_class(CLASSNAME(ref), base_type); > > @@ -2105,22 +2105,6 @@ static CMPIInstance *create_system(const CMPIContext > *context, > return inst; > } > > -static bool trigger_indication(const CMPIContext *context, > - const char *base_type, > - const CMPIObjectPath *ref) > -{ > - char *type; > - CMPIStatus s; > - > - type = get_typed_class(CLASSNAME(ref), base_type); > - > - s = stdi_trigger_indication(_BROKER, context, type, NAMESPACE(ref)); > - > - free(type); > - > - return s.rc == CMPI_RC_OK; > -} > - > static CMPIStatus define_system(CMPIMethodMI *self, > const CMPIContext *context, > const CMPIResult *results, > @@ -2156,9 +2140,6 @@ static CMPIStatus define_system(CMPIMethodMI *self, > CMAddArg(argsout, "ResultingSystem",&result, CMPI_ref); > } > > - trigger_indication(context, > - "ComputerSystemCreatedIndication", > - reference); > out: > if (s.rc == CMPI_RC_OK) > rc = CIM_SVPC_RETURN_COMPLETED; > @@ -2261,9 +2242,6 @@ error: > NULL, > reference, > &list); > - trigger_indication(context, > - "ComputerSystemDeletedIndication", > - reference); > } > > virDomainFree(dom); > @@ -2345,12 +2323,8 @@ static CMPIStatus update_system_settings(const > CMPIContext *context, > connect_and_create(xml, ref,&s); > } > > - if (s.rc == CMPI_RC_OK) { > + if (s.rc == CMPI_RC_OK) > set_autostart(vssd, ref, dom); > - trigger_indication(context, > - "ComputerSystemModifiedIndication", > - ref); > - } > > out: > free(xml);

Follow up from my previous comments... I think the problem is related to the use of virEventRegisterDefaultImpl() in libvirt_cim_init(), which is called by all providers, not just indication providers. Given the number of times we call that function, perhaps we're overloading libvirt and/or triggering an underlying bug. When I either comment out that function, I see no crashes. I then moved it to be called once in ComputerSystemIndication->ActivateFilter() (via a static init variable), and I was able to avoid the crashes. Unfortunately, I'm not able to verify the domain events function via cimtest. FWIW, I see the same message w/o the patch so perhaps I need to do something else to make it work.

ComputerSystemIndication - 01_created_indication.py: FAIL ERROR - Waited too long for define indication ERROR - Exception: Poll for indication Failed ERROR - Waited too long for start indication ERROR - Exception: Poll for indication Failed ERROR - Waited too long for destroy indication ERROR - Exception: Poll for indication Failed Here's a quick peek at the changes I made for reference: Remove virEventRegisterDefaultImpl() from init functon diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c index 556fe1f..61893c3 100644 --- a/libxkutil/misc_util.c +++ b/libxkutil/misc_util.c @@ -607,8 +607,6 @@ bool libvirt_cim_init(void) ret = virInitialize(); if (ret == 0) libvirt_initialized = 1; - - virEventRegisterDefaultImpl(); } pthread_mutex_unlock(&libvirt_mutex); } Initialize entire csi_thread_data_t structures. Not sure if a single '{0}' sets *all* members to 0 and I wanted to be sure the active_filters member was set to 0 before incrementing/decrementing.

Also added one time call to virEventRegisterDefaultImpl() to ActiveFilter(). diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index 0df73f3..1ea8520 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -80,7 +80,10 @@ struct _csi_thread_data_t { static const CMPIBroker *_BROKER; static pthread_mutex_t lifecycle_mutex = PTHREAD_MUTEX_INITIALIZER; static bool lifecycle_enabled = false; -static csi_thread_data_t csi_thread_data[CSI_NUM_PLATFORMS] = {{0}, {0}, {0}}; +static csi_thread_data_t csi_thread_data[CSI_NUM_PLATFORMS] = + {{0, 0, 0, 0, 0}, + {0, 0, 0, 0, 0}, + {0, 0, 0, 0, 0}}; /* * Domain list manipulation @@ -698,6 +703,13 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, bool error = false; csi_thread_data_t *thread = NULL; + static int _init = 0; + + if(_init == 0) { + _init = 1; + virEventRegisterDefaultImpl(); + } + CU_DEBUG("ActivateFilter for %s", CLASSNAME(op)); pthread_mutex_lock(&lifecycle_mutex);