2:38 a.m.
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_ComputerSystemIndication.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c
index 2d0a94e..b1b9fa0 100644
--- a/src/Virt_ComputerSystemIndication.c
+++ b/src/Virt_ComputerSystemIndication.c
@@ -503,7 +503,7 @@ static void csi_domain_event_cb(virConnectPtr conn,
CMPIStatus s = {CMPI_RC_OK, NULL};
if (lifecycle_enabled == false || thread->active_filters <= 0) {
- CU_DEBUG("%s indications deactivated, return");
+ CU_DEBUG("%s indications deactivated, return", prefix);
return;
}
--
1.7.1
2:38 a.m.
New subject: [PATCH 2/4] CSI, add lock to protect shared data
This patch added lock on shared data without which may trigger
incorrect data operation. Global lock is used so speed may be slowed
down, it can be improved in the future.
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_ComputerSystemIndication.c | 39 ++++++++++++++++++++++++++++++++--
1 files changed, 36 insertions(+), 3 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c
index b1b9fa0..3159ca0 100644
--- a/src/Virt_ComputerSystemIndication.c
+++ b/src/Virt_ComputerSystemIndication.c
@@ -174,9 +174,11 @@ static void csi_free_thread_data(void *data)
if (data == NULL)
return;
+ pthread_mutex_lock(&lifecycle_mutex);
list_free(thread->dom_list);
thread->dom_list = NULL;
stdi_free_ind_args(&thread->args);
+ pthread_mutex_unlock(&lifecycle_mutex);
}
void set_source_inst_props(const CMPIBroker *broker,
@@ -490,6 +492,8 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t
*thread)
return s.rc;
}
+/* following function will protect global data with lifecycle_mutex.
+ TODO: improve it with seperate lock later. */
static void csi_domain_event_cb(virConnectPtr conn,
virDomainPtr dom,
int event,
@@ -502,10 +506,13 @@ static void csi_domain_event_cb(virConnectPtr conn,
char *prefix = class_prefix_name(thread->args->classname);
CMPIStatus s = {CMPI_RC_OK, NULL};
+ pthread_mutex_lock(&lifecycle_mutex);
if (lifecycle_enabled == false || thread->active_filters <= 0) {
CU_DEBUG("%s indications deactivated, return", prefix);
+ pthread_mutex_unlock(&lifecycle_mutex);
return;
}
+ pthread_mutex_unlock(&lifecycle_mutex);
CU_DEBUG("Event: Domain %s(%d) event: %d detail: %d\n",
virDomainGetName(dom), virDomainGetID(dom), event, detail);
@@ -537,7 +544,9 @@ static void csi_domain_event_cb(virConnectPtr conn,
if (cs_event != CS_CREATED) {
char uuid[VIR_UUID_STRING_BUFLEN] = {0};
virDomainGetUUIDString(dom, &uuid[0]);
+ pthread_mutex_lock(&lifecycle_mutex);
dom_xml = list_find(thread->dom_list, uuid);
+ pthread_mutex_unlock(&lifecycle_mutex);
}
if (dom_xml == NULL) {
@@ -545,12 +554,16 @@ static void csi_domain_event_cb(virConnectPtr conn,
goto end;
}
+ pthread_mutex_lock(&lifecycle_mutex);
async_ind(thread->args, cs_event, dom_xml, prefix);
+ pthread_mutex_unlock(&lifecycle_mutex);
/* Update the domain list accordingly */
if (event == VIR_DOMAIN_EVENT_DEFINED) {
if (detail == VIR_DOMAIN_EVENT_DEFINED_ADDED) {
+ pthread_mutex_lock(&lifecycle_mutex);
csi_thread_dom_list_append(thread, dom_xml);
+ pthread_mutex_unlock(&lifecycle_mutex);
} else if (detail == VIR_DOMAIN_EVENT_DEFINED_UPDATED) {
free(dom_xml->name);
free(dom_xml->xml);
@@ -558,7 +571,9 @@ static void csi_domain_event_cb(virConnectPtr conn,
}
} else if (event == VIR_DOMAIN_EVENT_DEFINED &&
detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) {
+ pthread_mutex_lock(&lifecycle_mutex);
list_remove(thread->dom_list, dom_xml);
+ pthread_mutex_unlock(&lifecycle_mutex);
}
end:
@@ -579,10 +594,12 @@ static CMPI_THREAD_RETURN lifecycle_thread(void *params)
if (prefix == NULL)
goto init_out;
+ pthread_mutex_lock(&lifecycle_mutex);
conn = connect_by_classname(_BROKER, args->classname, &s);
if (conn == NULL) {
CU_DEBUG("Unable to start lifecycle thread: "
"Failed to connect (cn: %s)", args->classname);
+ pthread_mutex_unlock(&lifecycle_mutex);
goto conn_out;
}
@@ -594,33 +611,47 @@ static CMPI_THREAD_RETURN lifecycle_thread(void *params)
if (cb_id == -1) {
CU_DEBUG("Failed to register domain event watch for
'%s'",
- args->classname)
+ args->classname);
+ pthread_mutex_unlock(&lifecycle_mutex);
goto cb_out;
}
CBAttachThread(_BROKER, args->context);
/* Get currently defined domains */
- if (update_domain_list(conn, thread) != CMPI_RC_OK)
+ if (update_domain_list(conn, thread) != CMPI_RC_OK) {
+ pthread_mutex_unlock(&lifecycle_mutex);
goto end;
+ }
+ pthread_mutex_unlock(&lifecycle_mutex);
CU_DEBUG("Entering CSI event loop (%s)", prefix);
- while (thread->active_filters > 0) {
+ while (1) {
+ pthread_mutex_lock(&lifecycle_mutex);
+ if (thread->active_filters <= 0) {
+ pthread_mutex_unlock(&lifecycle_mutex);
+ break;
+ }
+ pthread_mutex_unlock(&lifecycle_mutex);
if (virEventRunDefaultImpl() < 0) {
virErrorPtr err = virGetLastError();
CU_DEBUG("Failed to run event loop: %s\n",
err && err->message ? err->message : "Unknown
error");
}
+ usleep(1);
}
CU_DEBUG("Exiting CSI event loop (%s)", prefix);
+ pthread_mutex_lock(&lifecycle_mutex);
CBDetachThread(_BROKER, args->context);
+ pthread_mutex_unlock(&lifecycle_mutex);
end:
virConnectDomainEventDeregisterAny(conn, cb_id);
cb_out:
+ pthread_mutex_lock(&lifecycle_mutex);
thread->id = 0;
thread->active_filters = 0;
@@ -628,6 +659,8 @@ static CMPI_THREAD_RETURN lifecycle_thread(void *params)
if (thread->args != NULL)
stdi_free_ind_args(&thread->args);
+ pthread_mutex_unlock(&lifecycle_mutex);
+
conn_out:
virConnectClose(conn);
--
1.7.1
2:38 a.m.
New subject: [PATCH 3/4] DevicePool, fix debug print crash
Dangerous bug, may trigger strange error, suggest apply it before investigate
any strange bug shown.
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_DevicePool.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index 202e509..79dc108 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -710,7 +710,7 @@ static bool mempool_set_consumed(CMPIInstance *inst, virConnectPtr
conn)
dom = virDomainLookupByID(conn, domain_ids[i]);
if (dom == NULL) {
- CU_DEBUG("Cannot connect to domain %n: excluding",
+ CU_DEBUG("Cannot connect to domain %d: excluding",
domain_ids[i]);
continue;
}
--
1.7.1
1:20 p.m.
New subject: [PATCH 3/4] DevicePool, fix debug print crash
+1
-Sharad Mishra
Quoting Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>:
Dangerous bug, may trigger strange error, suggest apply it before
investigate
any strange bug shown.
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_DevicePool.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index 202e509..79dc108 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -710,7 +710,7 @@ static bool mempool_set_consumed(CMPIInstance
*inst, virConnectPtr conn)
dom = virDomainLookupByID(conn, domain_ids[i]);
if (dom == NULL) {
- CU_DEBUG("Cannot connect to domain %n: excluding",
+ CU_DEBUG("Cannot connect to domain %d: excluding",
domain_ids[i]);
continue;
}
--
1.7.1
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
2:38 a.m.
New subject: [PATCH 4/4] DevicePool, reimplement get_diskpool_config with libvirt
Oringinal implement have risk, this patch should fix it
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_DevicePool.c | 47 +++++++++++++++++++++++++++++++++++------------
1 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index 79dc108..0cb9124 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -117,52 +117,75 @@ int get_disk_pool(virStoragePoolPtr poolptr, struct virt_pool
**pool)
return ret;
}
+/* This function returns the real number of pools, no negative value should be
+ returned, if error happens it returns zero. */
static int get_diskpool_config(virConnectPtr conn,
struct tmp_disk_pool **_pools)
{
- int count = 0;
+ int count = 0, realcount = 0;
int i;
char ** names = NULL;
struct tmp_disk_pool *pools = NULL;
+ int have_err = 0;
count = virConnectNumOfStoragePools(conn);
- if (count <= 0)
+ if (count <= 0) {
+ have_err = 1;
goto out;
+ }
names = calloc(count, sizeof(char *));
if (names == NULL) {
CU_DEBUG("Failed to alloc space for %i pool names", count);
count = 0;
+ have_err = 1;
goto out;
}
- if (virConnectListStoragePools(conn, names, count) == -1) {
+ realcount = virConnectListStoragePools(conn, names, count);
+ if (realcount == -1) {
CU_DEBUG("Failed to get storage pools");
- count = 0;
+ realcount = 0;
+ have_err = 1;
+ goto out;
+ }
+ if (realcount == 0) {
+ CU_DEBUG("zero pools got, but prelist is %d.", count);
goto out;
}
- pools = calloc(count, sizeof(*pools));
+ pools = calloc(realcount, sizeof(*pools));
if (pools == NULL) {
- CU_DEBUG("Failed to alloc space for %i pool structs", count);
+ CU_DEBUG("Failed to alloc space for %i pool structs",
realcount);
+ realcount = 0;
+ have_err = 1;
goto out;
}
- for (i = 0; i < count; i++) {
+ i = 0;
+ while (i < realcount) {
pools[i].tag = strdup(names[i]);
pools[i].primordial = false;
+ i++;
}
out:
- for (i = 0; i < count; i++)
- free(names[i]);
- free(names);
+ if (count > 0) {
+ i = 0;
+ while (i < count) {
+ free(names[i]);
+ i++;
+ }
+ free(names);
+ }
- get_disk_parent(&pools, &count);
+ if (have_err == 0) {
+ get_disk_parent(&pools, &realcount);
+ }
*_pools = pools;
- return count;
+ return realcount;
}
static bool diskpool_set_capacity(virConnectPtr conn,
--
1.7.1
1:22 p.m.
New subject: [PATCH 4/4] DevicePool, reimplement get_diskpool_config with libvirt
-1
This patch makes it impossible for called to know if "0" was returned
as a result of a failure or no diskpools were found.
I recommend returning "-1" in error and update the patch to make sure
that calling functions are handling a return value of -1 properly.
-Sharad Mishra
Quoting Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>:
Oringinal implement have risk, this patch should fix it
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_DevicePool.c | 47 +++++++++++++++++++++++++++++++++++------------
1 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index 79dc108..0cb9124 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -117,52 +117,75 @@ int get_disk_pool(virStoragePoolPtr poolptr,
struct virt_pool **pool)
return ret;
}
+/* This function returns the real number of pools, no negative
value should be
+ returned, if error happens it returns zero. */
static int get_diskpool_config(virConnectPtr conn,
struct tmp_disk_pool **_pools)
{
- int count = 0;
+ int count = 0, realcount = 0;
int i;
char ** names = NULL;
struct tmp_disk_pool *pools = NULL;
+ int have_err = 0;
count = virConnectNumOfStoragePools(conn);
- if (count <= 0)
+ if (count <= 0) {
+ have_err = 1;
goto out;
+ }
names = calloc(count, sizeof(char *));
if (names == NULL) {
CU_DEBUG("Failed to alloc space for %i pool names", count);
count = 0;
+ have_err = 1;
goto out;
}
- if (virConnectListStoragePools(conn, names, count) == -1) {
+ realcount = virConnectListStoragePools(conn, names, count);
+ if (realcount == -1) {
CU_DEBUG("Failed to get storage pools");
- count = 0;
+ realcount = 0;
+ have_err = 1;
+ goto out;
+ }
+ if (realcount == 0) {
+ CU_DEBUG("zero pools got, but prelist is %d.", count);
goto out;
}
- pools = calloc(count, sizeof(*pools));
+ pools = calloc(realcount, sizeof(*pools));
if (pools == NULL) {
- CU_DEBUG("Failed to alloc space for %i pool
structs", count);
+ CU_DEBUG("Failed to alloc space for %i pool
structs", realcount);
+ realcount = 0;
+ have_err = 1;
goto out;
}
- for (i = 0; i < count; i++) {
+ i = 0;
+ while (i < realcount) {
pools[i].tag = strdup(names[i]);
pools[i].primordial = false;
+ i++;
}
out:
- for (i = 0; i < count; i++)
- free(names[i]);
- free(names);
+ if (count > 0) {
+ i = 0;
+ while (i < count) {
+ free(names[i]);
+ i++;
+ }
+ free(names);
+ }
- get_disk_parent(&pools, &count);
+ if (have_err == 0) {
+ get_disk_parent(&pools, &realcount);
+ }
*_pools = pools;
- return count;
+ return realcount;
}
static bool diskpool_set_capacity(virConnectPtr conn,
--
1.7.1
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
11:28 a.m.
New subject: [PATCH 4/4] DevicePool, reimplement get_diskpool_config with libvirt
On Mon, Oct 22, 2012 at 5:38 AM, Wenchao Xia <xiawenc(a)linux.vnet.ibm.com> wrote:
Oringinal implement have risk, this patch should fix it
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_DevicePool.c | 47 +++++++++++++++++++++++++++++++++++------------
1 files changed, 35 insertions(+), 12 deletions(-)
diff --git a/src/Virt_DevicePool.c b/src/Virt_DevicePool.c
index 79dc108..0cb9124 100644
--- a/src/Virt_DevicePool.c
+++ b/src/Virt_DevicePool.c
@@ -117,52 +117,75 @@ int get_disk_pool(virStoragePoolPtr poolptr, struct virt_pool
**pool)
return ret;
}
+/* This function returns the real number of pools, no negative value should be
+ returned, if error happens it returns zero. */
static int get_diskpool_config(virConnectPtr conn,
struct tmp_disk_pool **_pools)
{
- int count = 0;
+ int count = 0, realcount = 0;
int i;
char ** names = NULL;
struct tmp_disk_pool *pools = NULL;
+ int have_err = 0;
count = virConnectNumOfStoragePools(conn);
- if (count <= 0)
+ if (count <= 0) {
+ have_err = 1;
goto out;
+ }
names = calloc(count, sizeof(char *));
if (names == NULL) {
CU_DEBUG("Failed to alloc space for %i pool names", count);
count = 0;
+ have_err = 1;
goto out;
}
- if (virConnectListStoragePools(conn, names, count) == -1) {
+ realcount = virConnectListStoragePools(conn, names, count);
+ if (realcount == -1) {
CU_DEBUG("Failed to get storage pools");
- count = 0;
+ realcount = 0;
+ have_err = 1;
+ goto out;
+ }
+ if (realcount == 0) {
+ CU_DEBUG("zero pools got, but prelist is %d.", count);
goto out;
}
- pools = calloc(count, sizeof(*pools));
+ pools = calloc(realcount, sizeof(*pools));
if (pools == NULL) {
- CU_DEBUG("Failed to alloc space for %i pool structs", count);
+ CU_DEBUG("Failed to alloc space for %i pool structs",
realcount);
+ realcount = 0;
+ have_err = 1;
goto out;
}
- for (i = 0; i < count; i++) {
+ i = 0;
+ while (i < realcount) {
pools[i].tag = strdup(names[i]);
pools[i].primordial = false;
+ i++;
}
Any specific reason for changing the for() loop for a while() one??
out:
- for (i = 0; i < count; i++)
- free(names[i]);
- free(names);
+ if (count > 0) {
+ i = 0;
+ while (i < count) {
+ free(names[i]);
+ i++;
+ }
+ free(names);
+ }
Same here.
Best regards,
--
Eduardo de Barros Lima ◤✠◢
eblima(a)gmail.com
1:01 p.m.
+1
-Sharad Mishra
Quoting Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>:
Signed-off-by: Wenchao Xia <xiawenc(a)linux.vnet.ibm.com>
---
src/Virt_ComputerSystemIndication.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/Virt_ComputerSystemIndication.c
b/src/Virt_ComputerSystemIndication.c
index 2d0a94e..b1b9fa0 100644
--- a/src/Virt_ComputerSystemIndication.c
+++ b/src/Virt_ComputerSystemIndication.c
@@ -503,7 +503,7 @@ static void csi_domain_event_cb(virConnectPtr conn,
CMPIStatus s = {CMPI_RC_OK, NULL};
if (lifecycle_enabled == false || thread->active_filters <= 0) {
- CU_DEBUG("%s indications deactivated, return");
+ CU_DEBUG("%s indications deactivated, return", prefix);
return;
}
--
1.7.1
_______________________________________________
Libvirt-cim mailing list
Libvirt-cim(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvirt-cim
4442
days inactive
4443
days old
7 comments
3 participants
participants (3)
-
Eduardo Lima (Etrunko) -
snmishra＠linux.vnet.ibm.com -
Wenchao Xia