3:33 p.m.
Another crack at the integration of CSMI and CSI. It appears that a lot of work went into
not much change from the last set really. This new set really just fixes a couple of very
annoying bugs from the old one, as detailed in the individual patches.
3:33 p.m.
New subject: [PATCH 1 of 2] Add Modified to the list of available indications in ComputerSystemIndication
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1200951153 18000
# Node ID 8faf3edab86112a5a9e77faa297db21f5f38eb44
# Parent ca8dc23eacc44970a2d79978bc9de65aaa600b92
Add Modified to the list of available indications in ComputerSystemIndication.
This is the integration of the previously discussed CSMI provider into the existing CSI
provider. Most of the integration changes are actually done to how CSI works, as CSMI
featured several improvements.
The schema changes are in here too because they're just so small and simple compared
to everything else that I didn't bother to break them out this time. If keeping them
separate is preferred I can do that.
Changes since last time:
Fixed up ind_args so we don't try to pass around stale pointers.
Made sure lifecycle_thread_id variable is reset when event loop exits, allowing
unsubscribe/re-subscribe to happen successfully.
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r ca8dc23eacc4 -r 8faf3edab861 schema/ComputerSystemIndication.mof
--- a/schema/ComputerSystemIndication.mof Wed Jan 09 13:08:17 2008 +0100
+++ b/schema/ComputerSystemIndication.mof Mon Jan 21 16:32:33 2008 -0500
@@ -14,6 +14,13 @@ class Xen_ComputerSystemDeletedIndicatio
{
};
+[Description ("Xen_ComputerSystem modified"),
+ Provider("cmpi::Virt_ComputerSystemIndication")
+]
+class Xen_ComputerSystemModifiedIndication : CIM_InstModification
+{
+};
+
[Description ("KVM_ComputerSystem lifecycle"),
Provider("cmpi::Virt_ComputerSystemIndication")
]
@@ -27,3 +34,10 @@ class KVM_ComputerSystemDeletedIndicatio
class KVM_ComputerSystemDeletedIndication : CIM_InstDeletion
{
};
+
+[Description ("KVM_ComputerSystem modified"),
+ Provider("cmpi::Virt_ComputerSystemIndication")
+]
+class KVM_ComputerSystemModifiedIndication : CIM_InstModification
+{
+};
diff -r ca8dc23eacc4 -r 8faf3edab861 schema/ComputerSystemIndication.registration
--- a/schema/ComputerSystemIndication.registration Wed Jan 09 13:08:17 2008 +0100
+++ b/schema/ComputerSystemIndication.registration Mon Jan 21 16:32:33 2008 -0500
@@ -2,5 +2,7 @@
# Classname Namespace ProviderName ProviderModule ProviderTypes
Xen_ComputerSystemCreatedIndication root/virt Virt_ComputerSystemIndication
Virt_ComputerSystemIndication indication method
Xen_ComputerSystemDeletedIndication root/virt Virt_ComputerSystemIndication
Virt_ComputerSystemIndication indication method
+Xen_ComputerSystemModifiedIndication root/virt Virt_ComputerSystemIndication
Virt_ComputerSystemIndication indication method
KVM_ComputerSystemCreatedIndication root/virt Virt_ComputerSystemIndication
Virt_ComputerSystemIndication indication method
KVM_ComputerSystemDeletedIndication root/virt Virt_ComputerSystemIndication
Virt_ComputerSystemIndication indication method
+KVM_ComputerSystemModifiedIndication root/virt Virt_ComputerSystemIndication
Virt_ComputerSystemIndication indication method
diff -r ca8dc23eacc4 -r 8faf3edab861 src/Virt_ComputerSystemIndication.c
--- a/src/Virt_ComputerSystemIndication.c Wed Jan 09 13:08:17 2008 +0100
+++ b/src/Virt_ComputerSystemIndication.c Mon Jan 21 16:32:33 2008 -0500
@@ -62,6 +62,108 @@ static bool lifecycle_enabled = 0;
# define _EI_RET() return (CMPIStatus){CMPI_RC_OK, NULL}
#endif
+struct dom_xml {
+ char uuid[VIR_UUID_STRING_BUFLEN];
+ char *xml;
+};
+
+struct ind_args {
+ CMPIContext *context;
+ char *ns;
+ char *classname;
+};
+
+static void free_dom_xml (struct dom_xml dom)
+{
+ free(dom.xml);
+}
+
+static void free_ind_args (struct ind_args args)
+{
+ free(args.ns);
+ free(args.classname);
+}
+
+static void cleanup_domain_list(virDomainPtr *list, int size)
+{
+ int i;
+
+ for (i = 0; i < size; i++) {
+ virDomainFree(list[i]);
+ }
+}
+
+static char *sys_name_from_xml(char *xml)
+{
+ char *tmp = NULL;
+ char *name = NULL;
+ int rc;
+
+ tmp = strstr(xml, "<name>");
+ if (tmp == NULL)
+ goto out;
+
+ rc = sscanf(tmp, "<name>%a[^<]s</name>", &name);
+ if (rc != 1)
+ name = NULL;
+
+ out:
+ return name;
+}
+
+static CMPIStatus doms_to_xml(struct dom_xml **dom_xml_list,
+ virDomainPtr *dom_ptr_list,
+ int dom_ptr_count)
+{
+ int i;
+ int rc;
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+
+ *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;
+ }
+
+ (*dom_xml_list)[i].xml = virDomainGetXMLDesc(dom_ptr_list[i],
+ 0);
+ if ((*dom_xml_list)[i].xml == NULL) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "Failed to get xml desc");
+ break;
+ }
+ }
+
+ return s;
+}
+
+static bool dom_changed(struct dom_xml prev_dom,
+ struct dom_xml *cur_xml,
+ int cur_count)
+{
+ 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)
+ ret = true;
+
+ break;
+ }
+
+ return ret;
+}
+
static bool _lifecycle_indication(const CMPIBroker *broker,
const CMPIContext *ctx,
const CMPIObjectPath *newsystem,
@@ -97,6 +199,45 @@ static bool _lifecycle_indication(const
NAMESPACE(newsystem),
ind);
+
+ return true;
+}
+
+static bool _do_modified_indication(const CMPIBroker *broker,
+ const CMPIContext *ctx,
+ CMPIInstance *mod_inst,
+ char *prefix,
+ char *ns)
+{
+ CMPIObjectPath *ind_op;
+ CMPIInstance *ind;
+ CMPIStatus s;
+
+ ind = get_typed_instance(broker,
+ prefix,
+ "ComputerSystemModifiedIndication",
+ ns);
+ if (ind == NULL) {
+ CU_DEBUG("Failed to create ind");
+ return false;
+ }
+
+ ind_op = CMGetObjectPath(ind, &s);
+ if (s.rc != CMPI_RC_OK) {
+ CU_DEBUG("Failed to get ind_op");
+ return false;
+ }
+
+ CMSetProperty(ind, "PreviousInstance",
+ (CMPIValue *)&mod_inst, CMPI_instance);
+
+ CU_DEBUG("Delivering Indication: %s\n",
+ CMGetCharPtr(CMObjectPathToString(ind_op, NULL)));
+
+ CBDeliverIndication(_BROKER,
+ ctx,
+ CIM_VIRT_NS,
+ ind);
return true;
}
@@ -177,17 +318,56 @@ static bool async_ind(CMPIContext *conte
return _lifecycle_indication(_BROKER, context, op, type_name);
}
+static bool mod_ind(CMPIContext *context,
+ virConnectPtr conn,
+ struct dom_xml prev_dom,
+ char *prefix,
+ char *ns)
+{
+ bool rc;
+ char *name = NULL;
+ CMPIInstance *mod_inst;
+
+ mod_inst = get_typed_instance(_BROKER,
+ prefix,
+ "ComputerSystem",
+ ns);
+
+ name = sys_name_from_xml(prev_dom.xml);
+ CU_DEBUG("Name for system: '%s'", name);
+ if (name == NULL) {
+ rc = false;
+ goto out;
+ }
+
+ CMSetProperty(mod_inst, "Name",
+ (CMPIValue *)name, CMPI_chars);
+ CMSetProperty(mod_inst, "UUID",
+ (CMPIValue *)prev_dom.uuid, CMPI_chars);
+
+ rc = _do_modified_indication(_BROKER, context, mod_inst, prefix, ns);
+
+ out:
+ free(name);
+ return rc;
+}
+
static CMPI_THREAD_RETURN lifecycle_thread(void *params)
{
- CMPIContext *context = (CMPIContext *)params;
+ struct ind_args *args = (struct ind_args *)params;
+ CMPIContext *context = args->context;
CMPIStatus s;
int prev_count;
int cur_count;
virDomainPtr *prev_list;
virDomainPtr *cur_list;
+ struct dom_xml *cur_xml = NULL;
+ struct dom_xml *prev_xml = NULL;
virConnectPtr conn;
+ char *prefix = class_prefix_name(args->classname);
+ char *ns = args->ns;
- conn = lv_connect(_BROKER, &s);
+ conn = connect_by_classname(_BROKER, args->classname, &s);
if (conn == NULL) {
CU_DEBUG("Failed to connect: %s", CMGetCharPtr(s.msg));
return NULL;
@@ -198,12 +378,20 @@ static CMPI_THREAD_RETURN lifecycle_thre
CBAttachThread(_BROKER, context);
prev_count = get_domain_list(conn, &prev_list);
+ s = doms_to_xml(&prev_xml, prev_list, prev_count);
+ if (s.rc != CMPI_RC_OK)
+ CU_DEBUG("doms_to_xml failed. Attempting to continue.");
+
+ CU_DEBUG("entering event loop");
while (lifecycle_enabled) {
int i;
bool res;
cur_count = get_domain_list(conn, &cur_list);
+ s = doms_to_xml(&cur_xml, cur_list, cur_count);
+ if (s.rc != CMPI_RC_OK)
+ CU_DEBUG("doms_to_xml failed.");
for (i = 0; i < cur_count; i++) {
res = dom_in_list(cur_list[i], prev_count, prev_list);
@@ -221,15 +409,34 @@ static CMPI_THREAD_RETURN lifecycle_thre
conn,
virDomainGetName(prev_list[i]),
CS_DELETED);
- virDomainFree(prev_list[i]);
}
+ for (i = 0; i < prev_count; i++) {
+ res = dom_changed(prev_xml[i], cur_xml, cur_count);
+ if (res) {
+ CU_DEBUG("Domain '%s' modified.",
prev_xml[i].uuid);
+ mod_ind(context, conn, prev_xml[i], prefix, ns);
+ }
+ free_dom_xml(prev_xml[i]);
+ }
+
+ cleanup_domain_list(prev_list, prev_count);
+
free(prev_list);
+ free(prev_xml);
prev_list = cur_list;
+ prev_xml = cur_xml;
prev_count = cur_count;
wait_for_event();
}
+ pthread_mutex_unlock(&lifecycle_mutex);
+ free_ind_args(*args);
+ free(prefix);
+ virConnectClose(conn);
+ /* Should I free args as well here, since it's malloced in activate? */
+
+ lifecycle_thread_id = 0;
return NULL;
}
@@ -241,17 +448,29 @@ static CMPIStatus ActivateFilter(CMPIInd
const CMPIObjectPath* op,
CMPIBoolean first)
{
+ CU_DEBUG("ActivateFilter");
+ CMPIStatus s = {CMPI_RC_OK, NULL};
+ struct ind_args *args = malloc(sizeof(struct ind_args));
+
+ if (CMIsNullObject(op)) {
+ cu_statusf(_BROKER, &s,
+ CMPI_RC_ERR_FAILED,
+ "No ObjectPath given");
+ goto out;
+ }
+ args->ns = strdup(NAMESPACE(op));
+ args->classname = strdup(CLASSNAME(op));
+
if (lifecycle_thread_id == 0) {
- CMPIContext *thread_context;
-
- thread_context = CBPrepareAttachThread(_BROKER, ctx);
+ args->context = CBPrepareAttachThread(_BROKER, ctx);
lifecycle_thread_id = _BROKER->xft->newThread(lifecycle_thread,
- thread_context,
+ args,
0);
}
- return (CMPIStatus){CMPI_RC_OK, NULL};
+ out:
+ return s;
}
static CMPIStatus DeActivateFilter(CMPIIndicationMI* mi,
9:26 a.m.
New subject: [PATCH 1 of 2] Add Modified to the list of available indications in ComputerSystemIndication
JG> +static void cleanup_domain_list(virDomainPtr *list, int size)
JG> +{
JG> + int i;
JG> +
JG> + for (i = 0; i < size; i++) {
JG> + virDomainFree(list[i]);
JG> + }
JG> +}
This should be in misc_util with the other domain_list functions.
JG> @@ -221,15 +409,34 @@ static CMPI_THREAD_RETURN lifecycle_thre
JG> conn,
JG> virDomainGetName(prev_list[i]),
JG> CS_DELETED);
JG> - virDomainFree(prev_list[i]);
JG> }
Was this intentional? Cleaning up the list as we go prevents us from
having to go back through it again. I'm not sure if I see the items
used later, so it seems like we could eliminate the second trip
through with cleanup_domain_list().
JG> + /* Should I free args as well here, since it's malloced in activate?
*/
Yes :)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:43 a.m.
New subject: [PATCH 1 of 2] Add Modified to the list of available indications in ComputerSystemIndication
Dan Smith wrote:
JG> +static void cleanup_domain_list(virDomainPtr *list, int
size)
JG> +{
JG> + int i;
JG> +
JG> + for (i = 0; i < size; i++) {
JG> + virDomainFree(list[i]);
JG> + }
JG> +}
This should be in misc_util with the other domain_list functions.
Okay.
JG> @@ -221,15 +409,34 @@ static CMPI_THREAD_RETURN
lifecycle_thre
JG> conn,
JG> virDomainGetName(prev_list[i]),
JG> CS_DELETED);
JG> - virDomainFree(prev_list[i]);
JG> }
Was this intentional? Cleaning up the list as we go prevents us from
having to go back through it again. I'm not sure if I see the items
used later, so it seems like we could eliminate the second trip
through with cleanup_domain_list().
I think this is a victim of the two patches working on the same code,
and/or this change probably belongs in the second patch. When I do the
more complete merge, all three loops work on the lists of dom_xml
structs, so the only time the virDomaintPtr lists get used is when they
are converted to dom_xml lists.
JG> + /* Should I free args as well here, since it's
malloced in activate? */
Yes :)
Yea I was pretty sure on that. I mean it definitely needed to be freed,
I just wasn't positive that this was the right spot.
--
-Jay
10:06 a.m.
New subject: [PATCH 1 of 2] Add Modified to the list of available indications in ComputerSystemIndication
JG> I think this is a victim of the two patches working on the same
JG> code, and/or this change probably belongs in the second patch.
JG> When I do the more complete merge, all three loops work on the
JG> lists of dom_xml structs, so the only time the virDomaintPtr lists
JG> get used is when they are converted to dom_xml lists.
Ah, okay.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
3:33 p.m.
New subject: [PATCH 2 of 2] Clean up CSI/CSMI integration
# HG changeset patch
# User Jay Gagnon <grendel(a)linux.vnet.ibm.com>
# Date 1200951155 18000
# Node ID b0d6bb6f6b8946c821aaccd7425397153db63735
# Parent 8faf3edab86112a5a9e77faa297db21f5f38eb44
Clean up CSI/CSMI integration
The functions responsible for actually firing the indication weren't really integrated
in the first patch, so much as co-existing. This one attempts to reconcile the two
approaches.
Changes since last time:
Fix a segfault produced by a hard-coded constant (CS_MODIFIED) that should have been a
variable (ind_type).
Signed-off-by: Jay Gagnon <grendel(a)linux.vnet.ibm.com>
diff -r 8faf3edab861 -r b0d6bb6f6b89 src/Virt_ComputerSystemIndication.c
--- a/src/Virt_ComputerSystemIndication.c Mon Jan 21 16:32:33 2008 -0500
+++ b/src/Virt_ComputerSystemIndication.c Mon Jan 21 16:32:35 2008 -0500
@@ -48,6 +48,7 @@ static CMPI_THREAD_TYPE lifecycle_thread
enum CS_EVENTS {CS_CREATED,
CS_DELETED,
+ CS_MODIFIED,
};
static pthread_cond_t lifecycle_cond = PTHREAD_COND_INITIALIZER;
@@ -164,82 +165,67 @@ static bool dom_changed(struct dom_xml p
return ret;
}
-static bool _lifecycle_indication(const CMPIBroker *broker,
- const CMPIContext *ctx,
- const CMPIObjectPath *newsystem,
- const char *type)
+static bool _do_indication(const CMPIBroker *broker,
+ const CMPIContext *ctx,
+ CMPIInstance *affected_inst,
+ int ind_type,
+ const char *ind_type_name,
+ char *prefix,
+ char *ns)
{
+ CMPIObjectPath *affected_op;
CMPIObjectPath *ind_op;
CMPIInstance *ind;
CMPIStatus s;
+ bool ret = true;
ind = get_typed_instance(broker,
- "Xen", /* Temporary hack */
- type,
- NAMESPACE(newsystem));
+ prefix,
+ ind_type_name,
+ ns);
+
+ /* 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");
- return false;
+ ret = false;
}
ind_op = CMGetObjectPath(ind, &s);
if (s.rc != CMPI_RC_OK) {
CU_DEBUG("Failed to get ind_op");
- return false;
+ ret = false;
}
- CMSetProperty(ind, "AffectedSystem",
- (CMPIValue *)&newsystem, CMPI_ref);
+ switch (ind_type) {
+ case CS_CREATED:
+ case CS_DELETED:
+ affected_op = CMGetObjectPath(affected_inst, &s);
+ if (s.rc != CMPI_RC_OK) {
+ ret = false;
+ CU_DEBUG("problem getting affected_op: '%s'",
s.msg);
+ goto out;
+ }
+ CMSetProperty(ind, "AffectedSystem",
+ (CMPIValue *)&affected_op, CMPI_ref);
+ break;
+ case CS_MODIFIED:
+ CMSetProperty(ind, "PreviousInstance",
+ (CMPIValue *)&affected_inst, CMPI_instance);
+ break;
+ }
CU_DEBUG("Delivering Indication: %s",
- CMGetCharPtr(CMObjectPathToString(ind_op, NULL)));
-
- CBDeliverIndication(_BROKER,
- ctx,
- NAMESPACE(newsystem),
- ind);
-
-
- return true;
-}
-
-static bool _do_modified_indication(const CMPIBroker *broker,
- const CMPIContext *ctx,
- CMPIInstance *mod_inst,
- char *prefix,
- char *ns)
-{
- CMPIObjectPath *ind_op;
- CMPIInstance *ind;
- CMPIStatus s;
-
- ind = get_typed_instance(broker,
- prefix,
- "ComputerSystemModifiedIndication",
- ns);
- if (ind == NULL) {
- CU_DEBUG("Failed to create ind");
- return false;
- }
-
- ind_op = CMGetObjectPath(ind, &s);
- if (s.rc != CMPI_RC_OK) {
- CU_DEBUG("Failed to get ind_op");
- return false;
- }
-
- CMSetProperty(ind, "PreviousInstance",
- (CMPIValue *)&mod_inst, CMPI_instance);
-
- CU_DEBUG("Delivering Indication: %s\n",
CMGetCharPtr(CMObjectPathToString(ind_op, NULL)));
- CBDeliverIndication(_BROKER,
+ CBDeliverIndication(broker,
ctx,
- CIM_VIRT_NS,
+ ns,
ind);
+ CU_DEBUG("Delivered");
- return true;
+ out:
+ return ret;
}
static bool wait_for_event(void)
@@ -258,18 +244,12 @@ static bool wait_for_event(void)
return true;
}
-static bool dom_in_list(virDomainPtr dom, int count, virDomainPtr *list)
+static bool dom_in_list(char *uuid, int count, struct dom_xml *list)
{
- const char *_name;
int i;
- _name = virDomainGetName(dom);
-
for (i = 0; i < count; i++) {
- const char *name;
-
- name = virDomainGetName(list[i]);
- if (STREQ(name, _name))
+ if (STREQ(uuid, list[i].uuid))
return true;
}
@@ -278,60 +258,20 @@ static bool dom_in_list(virDomainPtr dom
static bool async_ind(CMPIContext *context,
virConnectPtr conn,
- const char *name,
- int type)
-{
- CMPIInstance *newinst;
- CMPIObjectPath *op;
- CMPIStatus s;
- const char *type_name;
- char *type_cn = NULL;
- const char *ns = CIM_VIRT_NS;
-
- /* FIXME: Hmm, need to get the namespace a better way */
-
- if (type == CS_CREATED) {
- type_name = "ComputerSystemCreatedIndication";
- type_cn = get_typed_class(pfx_from_conn(conn), type_name);
-
- op = CMNewObjectPath(_BROKER, ns, type_cn, &s);
- } else if (type == CS_DELETED) {
- type_name = "ComputerSystemDeletedIndication";
- type_cn = get_typed_class(pfx_from_conn(conn), type_name);
-
- op = CMNewObjectPath(_BROKER, ns, type_cn, &s);
- } else {
- CU_DEBUG("Unknown event type: %i", type);
- return false;
- }
-
- if (type != CS_DELETED)
- newinst = instance_from_name(_BROKER, conn, (char *)name, op);
- else
- /* A deleted domain will have no instance to lookup */
- newinst = CMNewInstance(_BROKER, op, &s);
-
- op = CMGetObjectPath(newinst, NULL);
-
- free(type_cn);
-
- return _lifecycle_indication(_BROKER, context, op, type_name);
-}
-
-static bool mod_ind(CMPIContext *context,
- virConnectPtr conn,
- struct dom_xml prev_dom,
- char *prefix,
- char *ns)
+ int ind_type,
+ struct dom_xml prev_dom,
+ char *prefix,
+ char *ns)
{
bool rc;
char *name = NULL;
- CMPIInstance *mod_inst;
+ char *type_name = NULL;
+ CMPIInstance *affected_inst;
- mod_inst = get_typed_instance(_BROKER,
- prefix,
- "ComputerSystem",
- ns);
+ affected_inst = get_typed_instance(_BROKER,
+ prefix,
+ "ComputerSystem",
+ ns);
name = sys_name_from_xml(prev_dom.xml);
CU_DEBUG("Name for system: '%s'", name);
@@ -340,12 +280,25 @@ static bool mod_ind(CMPIContext *context
goto out;
}
- CMSetProperty(mod_inst, "Name",
+ switch (ind_type) {
+ case CS_CREATED:
+ type_name = "ComputerSystemCreatedIndication";
+ break;
+ case CS_DELETED:
+ type_name = "ComputerSystemDeletedIndication";
+ break;
+ case CS_MODIFIED:
+ type_name = "ComputerSystemModifiedIndication";
+ break;
+ }
+
+ CMSetProperty(affected_inst, "Name",
(CMPIValue *)name, CMPI_chars);
- CMSetProperty(mod_inst, "UUID",
+ CMSetProperty(affected_inst, "UUID",
(CMPIValue *)prev_dom.uuid, CMPI_chars);
- rc = _do_modified_indication(_BROKER, context, mod_inst, prefix, ns);
+ rc = _do_indication(_BROKER, context, affected_inst,
+ ind_type, type_name, prefix, ns);
out:
free(name);
@@ -359,8 +312,7 @@ static CMPI_THREAD_RETURN lifecycle_thre
CMPIStatus s;
int prev_count;
int cur_count;
- virDomainPtr *prev_list;
- virDomainPtr *cur_list;
+ virDomainPtr *tmp_list;
struct dom_xml *cur_xml = NULL;
struct dom_xml *prev_xml = NULL;
virConnectPtr conn;
@@ -370,66 +322,63 @@ static CMPI_THREAD_RETURN lifecycle_thre
conn = connect_by_classname(_BROKER, args->classname, &s);
if (conn == NULL) {
CU_DEBUG("Failed to connect: %s", CMGetCharPtr(s.msg));
- return NULL;
+ goto out;
}
pthread_mutex_lock(&lifecycle_mutex);
CBAttachThread(_BROKER, context);
- prev_count = get_domain_list(conn, &prev_list);
- s = doms_to_xml(&prev_xml, prev_list, prev_count);
+ 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.");
-
+ cleanup_domain_list(tmp_list, prev_count);
+ free(tmp_list);
CU_DEBUG("entering event loop");
while (lifecycle_enabled) {
int i;
bool res;
- cur_count = get_domain_list(conn, &cur_list);
- s = doms_to_xml(&cur_xml, cur_list, cur_count);
+ 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.");
+ cleanup_domain_list(tmp_list, cur_count);
+ free(tmp_list);
for (i = 0; i < cur_count; i++) {
- res = dom_in_list(cur_list[i], prev_count, prev_list);
+ res = dom_in_list(cur_xml[i].uuid, prev_count, prev_xml);
if (!res)
- async_ind(context,
- conn,
- virDomainGetName(cur_list[i]),
- CS_CREATED);
+ async_ind(context, conn, CS_CREATED,
+ cur_xml[i], prefix, ns);
}
for (i = 0; i < prev_count; i++) {
- res = dom_in_list(prev_list[i], cur_count, cur_list);
+ res = dom_in_list(prev_xml[i].uuid, cur_count, cur_xml);
if (!res)
- async_ind(context,
- conn,
- virDomainGetName(prev_list[i]),
- CS_DELETED);
+ async_ind(context, conn, CS_DELETED,
+ prev_xml[i], prefix, ns);
}
for (i = 0; i < prev_count; i++) {
res = dom_changed(prev_xml[i], cur_xml, cur_count);
if (res) {
- CU_DEBUG("Domain '%s' modified.",
prev_xml[i].uuid);
- mod_ind(context, conn, prev_xml[i], prefix, ns);
+ async_ind(context, conn, CS_MODIFIED,
+ prev_xml[i], prefix, ns);
}
free_dom_xml(prev_xml[i]);
}
- cleanup_domain_list(prev_list, prev_count);
-
- free(prev_list);
free(prev_xml);
- prev_list = cur_list;
prev_xml = cur_xml;
prev_count = cur_count;
wait_for_event();
}
+
+ out:
pthread_mutex_unlock(&lifecycle_mutex);
free_ind_args(*args);
free(prefix);
9:38 a.m.
New subject: [PATCH 2 of 2] Clean up CSI/CSMI integration
JG> + /* Generally report errors and hope to continue, since we have no one
JG> + to actually return status to. */
JG> if (ind == NULL) {
JG> CU_DEBUG("Failed to create ind");
JG> - return false;
JG> + ret = false;
JG> }
I know you didn't change this, but can we report a little more
information in the debug? Type of indication, affected domain (if
it's easy to get), etc., would be nice, I think.
JG> ind_op = CMGetObjectPath(ind, &s);
JG> if (s.rc != CMPI_RC_OK) {
JG> CU_DEBUG("Failed to get ind_op");
JG> - return false;
JG> + ret = false;
JG> }
Here, printing s.msg would also be good, I think. Since indications
are kinda off in their own world, it seems like printing the CIMOM's
reason for not creating objects would help debugging down the road.
Also, don't we need a trigger_indication() in VSMS in the modify path?
Otherwise looks pretty good, I'd say.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:46 a.m.
New subject: [PATCH 2 of 2] Clean up CSI/CSMI integration
Dan Smith wrote:
JG> + /* Generally report errors and hope to continue,
since we have no one
JG> + to actually return status to. */
JG> if (ind == NULL) {
JG> CU_DEBUG("Failed to create ind");
JG> - return false;
JG> + ret = false;
JG> }
I know you didn't change this, but can we report a little more
information in the debug? Type of indication, affected domain (if
it's easy to get), etc., would be nice, I think.
Sure, no prob.
JG> ind_op = CMGetObjectPath(ind, &s);
JG> if (s.rc != CMPI_RC_OK) {
JG> CU_DEBUG("Failed to get ind_op");
JG> - return false;
JG> + ret = false;
JG> }
Here, printing s.msg would also be good, I think. Since indications
are kinda off in their own world, it seems like printing the CIMOM's
reason for not creating objects would help debugging down the road.
Okay.
Also, don't we need a trigger_indication() in VSMS in the modify
path?
Yup. I wanted to get this approved and in the tree first, then have a
small patch that adds the trigger call and increases the sleep period on
the event loop to something more reasonable.
--
-Jay
10:06 a.m.
New subject: [PATCH 2 of 2] Clean up CSI/CSMI integration
JG> Yup. I wanted to get this approved and in the tree first, then
JG> have a small patch that adds the trigger call and increases the
JG> sleep period on the event loop to something more reasonable.
Okay, cool, just didn't want it to go unnoticed.
Thanks!
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
3:53 a.m.
Jay Gagnon wrote:
Another crack at the integration of CSMI and CSI. It appears that a
lot of work went into not much change from the last set really. This new set really just
fixes a couple of very annoying bugs from the old one, as detailed in the individual
patches.
I like the very clean and well integrated structure of
Virt_ComputerSystemIndication.c.
An excellent work ... +1.
--
Regards
Heidi Eckhart
Software Engineer
IBM Linux Technology Center - Open Hypervisor
6178
days inactive
6179
days old
9 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon