[PATCH 0 of 2] Finish up ComputerSystemIndication

These two patches should be what we need to put the check mark next to ComputerSystemIndication. The first deals with a potential disaster that is essentially a race condition somewhere in the libvirt/xend region. The second adds the Modified trigger to VSMS and, since our usage model relies much more on the trigger than the polling loop, increases the event loop wait time to three minutes (feel free to throw out any other suggestions for that number). Comments are welcome on the first patch; I think it's a pretty good solution but if anyone thinks it can be improved we can certainly talk about it.

# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1201201667 18000 # Node ID 1d9cbc7cfb1f39a08dbf43070d0f1a140faa8fe4 # Parent adf18661f7948a0287a4586d97572793e8e03826 Protect CSI from transient domains Testing has revealed a couple of ways to segfault the CSI provider. One is a domain that is undefined immediately after being defined, the other is a domain that is listed as defined by libvirt but has not actually been fully defined by xend. In either case, the problem boils down to CSI trying to get an XML description of a domain that isn't there. This eventually results in a segfault. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r adf18661f794 -r 1d9cbc7cfb1f src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Thu Jan 24 12:56:45 2008 +0100 +++ b/src/Virt_ComputerSystemIndication.c Thu Jan 24 14:07:47 2008 -0500 @@ -54,6 +54,9 @@ static pthread_cond_t lifecycle_cond = P 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 3 +#define FAIL_WAIT_TIME 2 #ifdef CMPI_EI_VOID # define _EI_RTYPE void @@ -224,14 +227,14 @@ static bool _do_indication(const CMPIBro return ret; } -static bool wait_for_event(void) +static bool wait_for_event(int wait_time) { struct timespec timeout; int ret; clock_gettime(CLOCK_REALTIME, &timeout); - timeout.tv_sec += 3; + timeout.tv_sec += wait_time; ret = pthread_cond_timedwait(&lifecycle_cond, &lifecycle_mutex, @@ -336,11 +339,17 @@ static CMPI_THREAD_RETURN lifecycle_thre while (lifecycle_enabled) { 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."); + 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); @@ -369,11 +378,16 @@ static CMPI_THREAD_RETURN lifecycle_thre free_dom_xml(prev_xml[i]); } - free(prev_xml); - prev_xml = cur_xml; - prev_count = cur_count; + 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_for_event(WAIT_TIME); + } } out:

# HG changeset patch # User Jay Gagnon <grendel@linux.vnet.ibm.com> # Date 1201201824 18000 # Node ID ba7915ed8b0063dd44766ed0ed596f99412761d0 # Parent 1d9cbc7cfb1f39a08dbf43070d0f1a140faa8fe4 Add CSI trigger call to VSMS Now that ComputerSystemIndication is at the "maybe just a few tweaks" stage, it's time to get the trigger calls into VSMS. This way, we can increase the sleep period on CSI's event loop, and when VSMS does something like modify a domain, it can just wake up CSI manually. This gives us a fast response time without near-constant polling. Signed-off-by: Jay Gagnon <grendel@linux.vnet.ibm.com> diff -r 1d9cbc7cfb1f -r ba7915ed8b00 src/Virt_ComputerSystemIndication.c --- a/src/Virt_ComputerSystemIndication.c Thu Jan 24 14:07:47 2008 -0500 +++ b/src/Virt_ComputerSystemIndication.c Thu Jan 24 14:10:24 2008 -0500 @@ -55,7 +55,7 @@ static pthread_mutex_t lifecycle_mutex = static pthread_mutex_t lifecycle_mutex = PTHREAD_MUTEX_INITIALIZER; static bool lifecycle_enabled = 0; -#define WAIT_TIME 3 +#define WAIT_TIME 180 #define FAIL_WAIT_TIME 2 #ifdef CMPI_EI_VOID @@ -469,6 +469,7 @@ static _EI_RTYPE DisableIndications(CMPI static CMPIStatus trigger_indication(const CMPIContext *context) { + CU_DEBUG("triggered"); pthread_cond_signal(&lifecycle_cond); return(CMPIStatus){CMPI_RC_OK, NULL}; } diff -r 1d9cbc7cfb1f -r ba7915ed8b00 src/Virt_VirtualSystemManagementService.c --- a/src/Virt_VirtualSystemManagementService.c Thu Jan 24 14:07:47 2008 -0500 +++ b/src/Virt_VirtualSystemManagementService.c Thu Jan 24 14:10:24 2008 -0500 @@ -551,7 +551,8 @@ static CMPIStatus destroy_system(CMPIMet return status; } -static CMPIStatus update_system_settings(const CMPIObjectPath *ref, +static CMPIStatus update_system_settings(const CMPIContext *context, + const CMPIObjectPath *ref, CMPIInstance *vssd) { CMPIStatus s; @@ -599,6 +600,12 @@ static CMPIStatus update_system_settings connect_and_create(xml, ref, &s); } + if (s.rc == CMPI_RC_OK) { + trigger_indication(context, + "ComputerSystemModifiedIndication", + NAMESPACE(ref)); + } + out: free(xml); virDomainFree(dom); @@ -640,7 +647,7 @@ static CMPIStatus mod_system_settings(CM return s; } - return update_system_settings(reference, inst); + return update_system_settings(context, reference, inst); } typedef CMPIStatus (*resmod_fn)(struct domain *,

Jay Gagnon wrote:
These two patches should be what we need to put the check mark next to ComputerSystemIndication. The first deals with a potential disaster that is essentially a race condition somewhere in the libvirt/xend region. The second adds the Modified trigger to VSMS and, since our usage model relies much more on the trigger than the polling loop, increases the event loop wait time to three minutes (feel free to throw out any other suggestions for that number).
Comments are welcome on the first patch; I think it's a pretty good solution but if anyone thinks it can be improved we can certainly talk about it.
This patchset looks good. +1 -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
participants (2)
-
Jay Gagnon
-
Kaitlin Rupert