[PATCH 0 of 2] More CSI fixes

# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1215036468 25200 # Node ID b7f53024c5a4d815691aa3112f5effb25df66d20 # Parent d1c55a723ad873d04620a336465d496bbc3f31ea Fix CSI to support a separate thread per platform This is crucial to being able to deliver indications from multiple platforms at the same time. By keeping a separate thread_id for each platform, we ensure that we start one and only one thread per platform, so that they can properly establish their connections to libvirt. Note that pegasus won't hand me a fresh ObjectPath if I subscribe to a (e.g) Xen indication after a KVM one, so this doesn't actually enable subscription of indications from multiple platforms. However, this seems to be a pegasus issue and can be worked independently. Also fix leak of args in ActivateFilter if thread is already started. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r d1c55a723ad8 -r b7f53024c5a4 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Wed Jul 02 10:18:13 2008 -0700 +++ b/src/Virt_ComputerSystemIndication.c Wed Jul 02 15:07:48 2008 -0700 @@ -46,7 +46,13 @@ static const CMPIBroker *_BROKER; -static CMPI_THREAD_TYPE lifecycle_thread_id = 0; +#define CSI_NUM_PLATFORMS 3 +enum CSI_PLATFORMS {CSI_XEN, + CSI_KVM, + CSI_LXC, +}; + +static CMPI_THREAD_TYPE thread_id[CSI_NUM_PLATFORMS]; enum CS_EVENTS {CS_CREATED, CS_DELETED, @@ -362,6 +368,18 @@ return rc; } +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 CMPI_THREAD_RETURN lifecycle_thread(void *params) { struct ind_args *args = (struct ind_args *)params; @@ -374,6 +392,7 @@ struct dom_xml *prev_xml = NULL; virConnectPtr conn; char *prefix = class_prefix_name(args->classname); + int platform = platform_from_class(args->classname); conn = connect_by_classname(_BROKER, args->classname, &s); if (conn == NULL) { @@ -393,7 +412,7 @@ free_domain_list(tmp_list, prev_count); free(tmp_list); - CU_DEBUG("entering event loop"); + CU_DEBUG("Entering CSI event loop (%s)", prefix); while (lifecycle_enabled) { int i; bool res; @@ -449,12 +468,12 @@ } out: + thread_id[platform] = 0; + pthread_mutex_unlock(&lifecycle_mutex); stdi_free_ind_args(&args); free(prefix); virConnectClose(conn); - - lifecycle_thread_id = 0; return NULL; } @@ -469,7 +488,8 @@ CU_DEBUG("ActivateFilter"); CMPIStatus s = {CMPI_RC_OK, NULL}; struct std_indication_ctx *_ctx; - struct ind_args *args = malloc(sizeof(struct ind_args)); + struct ind_args *args; + int platform; _ctx = (struct std_indication_ctx *)mi->hdl; @@ -479,14 +499,41 @@ "No ObjectPath given"); goto out; } - args->ns = strdup(NAMESPACE(op)); - args->classname = strdup(CLASSNAME(op)); - args->_ctx = _ctx; - if (lifecycle_thread_id == 0) { + /* FIXME: op is stale the second time around, for some reason */ + platform = platform_from_class(CLASSNAME(op)); + if (platform < 0) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unknown platform"); + goto out; + } + + if (thread_id[platform] == 0) { + args = malloc(sizeof(struct ind_args)); + if (args == NULL) { + CU_DEBUG("Failed to allocate ind_args"); + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to allocate ind_args"); + goto out; + } + args->context = CBPrepareAttachThread(_BROKER, ctx); + if (args->context == NULL) { + CU_DEBUG("Failed to create thread context"); + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to create thread context"); + free(args); + goto out; + } - lifecycle_thread_id = _BROKER->xft->newThread(lifecycle_thread, + args->ns = strdup(NAMESPACE(op)); + args->classname = strdup(CLASSNAME(op)); + args->_ctx = _ctx; + + thread_id[platform] = _BROKER->xft->newThread(lifecycle_thread, args, 0); } @@ -508,6 +555,7 @@ static _EI_RTYPE EnableIndications(CMPIIndicationMI* mi, const CMPIContext *ctx) { + CU_DEBUG("EnableIndications"); pthread_mutex_lock(&lifecycle_mutex); lifecycle_enabled = true; pthread_mutex_unlock(&lifecycle_mutex);

# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1215036495 25200 # Node ID eeb54ba6f7b6d3b74b67faefa5e35a12ed5945e9 # Parent b7f53024c5a4d815691aa3112f5effb25df66d20 Fix race between EnableIndications and ActivateFilter's thread Accomplish this by decoupling the enabled-ness of the whole provider from the presence of activated filters against the provider. Keep a count of the number of subscribed filters against us (per platform) and keep the thread running until that reaches zero. Use the enabled-ness of the provider to gate actual delivery of any indications (as intended). The original race was because ActivateFilter could be run before EnableIndications (which seems broken to me -- thanks Pegasus) and thus the thread was started before the global flag was set true. Thus, the thread fell right through the while loop and stopped running. Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r b7f53024c5a4 -r eeb54ba6f7b6 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Wed Jul 02 15:07:48 2008 -0700 +++ b/src/Virt_ComputerSystemIndication.c Wed Jul 02 15:08:15 2008 -0700 @@ -53,6 +53,7 @@ }; static CMPI_THREAD_TYPE thread_id[CSI_NUM_PLATFORMS]; +static int active_filters[CSI_NUM_PLATFORMS]; enum CS_EVENTS {CS_CREATED, CS_DELETED, @@ -331,6 +332,11 @@ char *type_name = NULL; CMPIInstance *affected_inst; + if (!lifecycle_enabled) { + CU_DEBUG("CSI not enabled, skipping indication delivery"); + return false; + } + affected_inst = get_typed_instance(_BROKER, prefix, "ComputerSystem", @@ -413,7 +419,7 @@ free(tmp_list); CU_DEBUG("Entering CSI event loop (%s)", prefix); - while (lifecycle_enabled) { + while (active_filters[platform] > 0) { int i; bool res; bool failure = false; @@ -468,6 +474,8 @@ } out: + CU_DEBUG("Exiting CSI event loop (%s)", prefix); + thread_id[platform] = 0; pthread_mutex_unlock(&lifecycle_mutex); @@ -485,11 +493,14 @@ const CMPIObjectPath* op, CMPIBoolean first) { - CU_DEBUG("ActivateFilter"); CMPIStatus s = {CMPI_RC_OK, NULL}; struct std_indication_ctx *_ctx; struct ind_args *args; int platform; + + CU_DEBUG("ActivateFilter for %s", CLASSNAME(op)); + + pthread_mutex_lock(&lifecycle_mutex); _ctx = (struct std_indication_ctx *)mi->hdl; @@ -533,12 +544,17 @@ 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; out: + pthread_mutex_unlock(&lifecycle_mutex); + return s; } @@ -549,7 +565,26 @@ const CMPIObjectPath* op, CMPIBoolean last) { - return (CMPIStatus){CMPI_RC_OK, NULL}; + int platform; + CMPIStatus s = {CMPI_RC_OK, NULL}; + + CU_DEBUG("DeActivateFilter for %s", CLASSNAME(op)); + + platform = platform_from_class(CLASSNAME(op)); + if (platform < 0) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unknown platform"); + goto out; + } + + pthread_mutex_lock(&lifecycle_mutex); + active_filters[platform] -= 1; + pthread_mutex_unlock(&lifecycle_mutex); + + pthread_cond_signal(&lifecycle_cond); + out: + return s; } static _EI_RTYPE EnableIndications(CMPIIndicationMI* mi, @@ -566,6 +601,7 @@ static _EI_RTYPE DisableIndications(CMPIIndicationMI* mi, const CMPIContext *ctx) { + CU_DEBUG("DisableIndications"); pthread_mutex_lock(&lifecycle_mutex); lifecycle_enabled = false; pthread_mutex_unlock(&lifecycle_mutex);
participants (2)
-
Dan Smith
-
Kaitlin Rupert