+#define CREATED "ResourceAllocationSettingDataCreatedIndication" +#define DELETED "ResourceAllocationSettingDataDeletedIndication" +#define MODIFIED "ResourceAllocationSettingDataModifiedIndication"
These are generic names - can you make them more specific to RASD indications?
+ else if (rc == IM_RC_NOT_SUPPORTED) + virt_set_status(_BROKER, &status, + CMPI_RC_ERR_NOT_FOUND, + conn, + "Unable to raise resource indication");
IM_RC_NOT_SUPPORTED is set because a connection to libvirt cannot be made. Can you change the error message here to reflect this?
else if (rc == IM_RC_FAILED) virt_set_status(_BROKER, &status, CMPI_RC_ERR_NOT_FOUND,
@@ -2116,6 +2238,54 @@ if (xml != NULL) { CU_DEBUG("New XML:\n%s", xml); connect_and_create(xml, ref, &s); + + if (func == &resource_add) { + indication = strdup(CREATED); + } + else if (func == &resource_del) { + indication = strdup(DELETED); + } + else { + indication = strdup(MODIFIED); + + s = enum_rasds(_BROKER, + ref, + dominfo->name, + type, + props, + &list); + if (s.rc != CMPI_RC_OK) { + CU_DEBUG("Failed to enumerate rasd"); + goto out; + } + + for(i=0; i < list.cur; i++) {
Space needed between i and =, also between = and 0.
+ prev_inst = list.list[i]; + ret = cu_get_str_prop(prev_inst, + "InstanceID", + &inst_id); + + if (ret != CMPI_RC_OK) + continue; + + if (STREQ(inst_id, + get_fq_devid(dominfo->name, + (char *)devid))) + break; + }
Can you break this out into a separate function? There's lots of indention here, an this else statement makes the function quite long.
+ + } + + inst_list_init(&list); + if (inst_list_add(&list, rasd) == 0) { + CU_DEBUG("Unable to add RASD instance to the list\n"); + goto out; + } + raise_rasd_indication(context, + indication, + prev_inst, + ref, + &list); } else { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, @@ -2125,6 +2295,8 @@ out: cleanup_dominfo(&dominfo); free(xml); + free(indication);
Why not make indication a const char *? Then you wouldn't need to free it. -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com