[PATCH 0/3] ComputerSystemIndication: code cleanups

From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> Simple series of patches that fix some minor issues found in the CSI code. Eduardo Lima (Etrunko) (3): CSI: Only execute callback if indications are enabled CSI: Cleanup code CSI: Fix log messages src/Virt_ComputerSystemIndication.c | 76 ++++++++++++++++++---------------- 1 files changed, 40 insertions(+), 36 deletions(-) -- 1.7.7.6

From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-) diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index 712e12c..ab6ffe4 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -489,11 +489,11 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) return s.rc; } -static int csi_domain_event_cb(virConnectPtr conn, - virDomainPtr dom, - int event, - int detail, - void *data) +static void 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; @@ -501,6 +501,11 @@ static int csi_domain_event_cb(virConnectPtr conn, char *prefix = class_prefix_name(thread->args->classname); CMPIStatus s = {CMPI_RC_OK, NULL}; + if (lifecycle_enabled == false || thread->active_filters <= 0) { + CU_DEBUG("%s indications deactivated, return"); + return; + } + CU_DEBUG("Event: Domain %s(%d) event: %d detail: %d\n", virDomainGetName(dom), virDomainGetID(dom), event, detail); @@ -557,7 +562,6 @@ static int csi_domain_event_cb(virConnectPtr conn, end: free(prefix); - return 0; } static CMPI_THREAD_RETURN lifecycle_thread(void *params) -- 1.7.7.6

+1. On RHEL 6.2 (libvirt-0.9.4) I was able to run all cimtests without seeing a core, but my local indication tests still do not work properly. It's a configuration issue I need to resolve. I always see the following: 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 Sharad: Can you give this patchset a try and see if it resolves the problem you were seeing? On 02/20/2012 02:36 PM, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)"<eblima@br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index 712e12c..ab6ffe4 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -489,11 +489,11 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) return s.rc; }
-static int csi_domain_event_cb(virConnectPtr conn, - virDomainPtr dom, - int event, - int detail, - void *data) +static void 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; @@ -501,6 +501,11 @@ static int csi_domain_event_cb(virConnectPtr conn, char *prefix = class_prefix_name(thread->args->classname); CMPIStatus s = {CMPI_RC_OK, NULL};
+ if (lifecycle_enabled == false || thread->active_filters<= 0) { + CU_DEBUG("%s indications deactivated, return"); + return; + } + CU_DEBUG("Event: Domain %s(%d) event: %d detail: %d\n", virDomainGetName(dom), virDomainGetID(dom), event, detail);
@@ -557,7 +562,6 @@ static int csi_domain_event_cb(virConnectPtr conn,
end: free(prefix); - return 0; }
static CMPI_THREAD_RETURN lifecycle_thread(void *params)
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

Pushed On 02/22/2012 10:33 AM, Chip Vincent wrote:
+1. On RHEL 6.2 (libvirt-0.9.4) I was able to run all cimtests without seeing a core, but my local indication tests still do not work properly. It's a configuration issue I need to resolve. I always see the following:
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
Sharad: Can you give this patchset a try and see if it resolves the problem you were seeing?
On 02/20/2012 02:36 PM, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)"<eblima@br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index 712e12c..ab6ffe4 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -489,11 +489,11 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) return s.rc; }
-static int csi_domain_event_cb(virConnectPtr conn, - virDomainPtr dom, - int event, - int detail, - void *data) +static void 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; @@ -501,6 +501,11 @@ static int csi_domain_event_cb(virConnectPtr conn, char *prefix = class_prefix_name(thread->args->classname); CMPIStatus s = {CMPI_RC_OK, NULL};
+ if (lifecycle_enabled == false || thread->active_filters<= 0) { + CU_DEBUG("%s indications deactivated, return"); + return; + } + CU_DEBUG("Event: Domain %s(%d) event: %d detail: %d\n", virDomainGetName(dom), virDomainGetID(dom), event, detail);
@@ -557,7 +562,6 @@ static int csi_domain_event_cb(virConnectPtr conn,
end: free(prefix); - return 0; }
static CMPI_THREAD_RETURN lifecycle_thread(void *params)
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 51 +++++++++++++++++------------------ 1 files changed, 25 insertions(+), 26 deletions(-) diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index ab6ffe4..b60138a 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -692,36 +692,35 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, 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; - } - - 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"); - error = true; - goto out; - } + /* Check if thread is already running */ + if (thread->id > 0) + goto out; - args->ns = strdup(NAMESPACE(op)); - args->classname = strdup(CLASSNAME(op)); - args->_ctx = _ctx; + 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; + } - thread->args = args; - thread->id = _BROKER->xft->newThread(lifecycle_thread, - thread, 0); + 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"); + error = true; + goto out; } + args->ns = strdup(NAMESPACE(op)); + args->classname = strdup(CLASSNAME(op)); + args->_ctx = _ctx; + + thread->args = args; + thread->id = _BROKER->xft->newThread(lifecycle_thread, thread, 0); + out: if (error == true) { thread->active_filters -= 1; -- 1.7.7.6

+1 On 02/20/2012 02:36 PM, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)"<eblima@br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 51 +++++++++++++++++------------------ 1 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index ab6ffe4..b60138a 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -692,36 +692,35 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, 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; - } - - 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"); - error = true; - goto out; - } + /* Check if thread is already running */ + if (thread->id> 0) + goto out;
- args->ns = strdup(NAMESPACE(op)); - args->classname = strdup(CLASSNAME(op)); - args->_ctx = _ctx; + 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; + }
- thread->args = args; - thread->id = _BROKER->xft->newThread(lifecycle_thread, - thread, 0); + 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"); + error = true; + goto out; }
+ args->ns = strdup(NAMESPACE(op)); + args->classname = strdup(CLASSNAME(op)); + args->_ctx = _ctx; + + thread->args = args; + thread->id = _BROKER->xft->newThread(lifecycle_thread, thread, 0); + out: if (error == true) { thread->active_filters -= 1;
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

Pushed On 02/22/2012 10:34 AM, Chip Vincent wrote:
+1
On 02/20/2012 02:36 PM, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)"<eblima@br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 51 +++++++++++++++++------------------ 1 files changed, 25 insertions(+), 26 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index ab6ffe4..b60138a 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -692,36 +692,35 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi, 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; - } - - 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"); - error = true; - goto out; - } + /* Check if thread is already running */ + if (thread->id> 0) + goto out;
- args->ns = strdup(NAMESPACE(op)); - args->classname = strdup(CLASSNAME(op)); - args->_ctx = _ctx; + 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; + }
- thread->args = args; - thread->id = _BROKER->xft->newThread(lifecycle_thread, - thread, 0); + 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"); + error = true; + goto out; }
+ args->ns = strdup(NAMESPACE(op)); + args->classname = strdup(CLASSNAME(op)); + args->_ctx = _ctx; + + thread->args = args; + thread->id = _BROKER->xft->newThread(lifecycle_thread, thread, 0); + out: if (error == true) { thread->active_filters -= 1;
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com> To produce correct outputs, it is necessary to call CMGetCharPtr(s.msg) instead of using s.msg directly. Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index b60138a..2d0a94e 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -254,7 +254,7 @@ static bool _do_indication(const CMPIBroker *broker, ind_op = CMGetObjectPath(ind, &s); if (s.rc != CMPI_RC_OK) { - CU_DEBUG("Failed to get ind_op. Error: '%s'", s.msg); + CU_DEBUG("Failed to get ind_op. Error: '%s'", CMGetCharPtr(s.msg)); ret = false; goto out; } @@ -263,7 +263,7 @@ static bool _do_indication(const CMPIBroker *broker, affected_op = CMGetObjectPath(affected_inst, &s); if (s.rc != CMPI_RC_OK) { ret = false; - CU_DEBUG("problem getting affected_op: '%s'", s.msg); + CU_DEBUG("problem getting affected_op: '%s'", CMGetCharPtr(s.msg)); goto out; } @@ -368,7 +368,8 @@ static bool create_deleted_guest_inst(const char *xml, dominfo, inst); if (s.rc != CMPI_RC_OK) { - CU_DEBUG("instance from domain info error: %s", s.msg); + CU_DEBUG("instance from domain info error: %s", + CMGetCharPtr(s.msg)); goto out; } @@ -476,7 +477,7 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) 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); + CU_DEBUG("Failed to get domain info %s", CMGetCharPtr(s.msg)); break; } -- 1.7.7.6

+1 On 02/20/2012 02:36 PM, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)"<eblima@br.ibm.com>
To produce correct outputs, it is necessary to call CMGetCharPtr(s.msg) instead of using s.msg directly.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index b60138a..2d0a94e 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -254,7 +254,7 @@ static bool _do_indication(const CMPIBroker *broker,
ind_op = CMGetObjectPath(ind,&s); if (s.rc != CMPI_RC_OK) { - CU_DEBUG("Failed to get ind_op. Error: '%s'", s.msg); + CU_DEBUG("Failed to get ind_op. Error: '%s'", CMGetCharPtr(s.msg)); ret = false; goto out; } @@ -263,7 +263,7 @@ static bool _do_indication(const CMPIBroker *broker, affected_op = CMGetObjectPath(affected_inst,&s); if (s.rc != CMPI_RC_OK) { ret = false; - CU_DEBUG("problem getting affected_op: '%s'", s.msg); + CU_DEBUG("problem getting affected_op: '%s'", CMGetCharPtr(s.msg)); goto out; }
@@ -368,7 +368,8 @@ static bool create_deleted_guest_inst(const char *xml, dominfo, inst); if (s.rc != CMPI_RC_OK) { - CU_DEBUG("instance from domain info error: %s", s.msg); + CU_DEBUG("instance from domain info error: %s", + CMGetCharPtr(s.msg)); goto out; }
@@ -476,7 +477,7 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) 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); + CU_DEBUG("Failed to get domain info %s", CMGetCharPtr(s.msg)); break; }
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com

Pushed On 02/22/2012 10:34 AM, Chip Vincent wrote:
+1
On 02/20/2012 02:36 PM, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)"<eblima@br.ibm.com>
To produce correct outputs, it is necessary to call CMGetCharPtr(s.msg) instead of using s.msg directly.
Signed-off-by: Eduardo Lima (Etrunko)<eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index b60138a..2d0a94e 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -254,7 +254,7 @@ static bool _do_indication(const CMPIBroker *broker,
ind_op = CMGetObjectPath(ind,&s); if (s.rc != CMPI_RC_OK) { - CU_DEBUG("Failed to get ind_op. Error: '%s'", s.msg); + CU_DEBUG("Failed to get ind_op. Error: '%s'", CMGetCharPtr(s.msg)); ret = false; goto out; } @@ -263,7 +263,7 @@ static bool _do_indication(const CMPIBroker *broker, affected_op = CMGetObjectPath(affected_inst,&s); if (s.rc != CMPI_RC_OK) { ret = false; - CU_DEBUG("problem getting affected_op: '%s'", s.msg); + CU_DEBUG("problem getting affected_op: '%s'", CMGetCharPtr(s.msg)); goto out; }
@@ -368,7 +368,8 @@ static bool create_deleted_guest_inst(const char *xml, dominfo, inst); if (s.rc != CMPI_RC_OK) { - CU_DEBUG("instance from domain info error: %s", s.msg); + CU_DEBUG("instance from domain info error: %s", + CMGetCharPtr(s.msg)); goto out; }
@@ -476,7 +477,7 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) 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); + CU_DEBUG("Failed to get domain info %s", CMGetCharPtr(s.msg)); break; }
-- Chip Vincent Open Virtualization IBM Linux Technology Center cvincent@linux.vnet.ibm.com
participants (2)
-
Chip Vincent
-
Eduardo Lima (Etrunko)