[PATCH 0 of 2] Fix problem with EnabledLogicalElementCapabilities EnumInstances.
This set of patches fixes the problem where ein/ei on EnabledLogicalElementCapabilities returns a "Missing key: Name" error.
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1196199118 28800 # Node ID 651039e9392c60f96b6cbaa63f0a239df5357550 # Parent 140613919317633ad4b325b6321809849ecfbd41 Fix an issue where EnabledLogicalElementCapabilities ein/ei returns an error. Enumerating this class currently returns: "Missing key: Name". The problem is that get_ele_cap() expects the ref to have a Name property, which isn't the case on an ein/ei call. This fix makes the get_ele_cap() call more generic by accepting a char * to use in building the InstanceID. Also, the return_ele_cap() function wasn't properly returning an instance for each domain on the system. This fix grabs the domain list and returns an instance accordingly. NOTE: I don't like how I handled the virDomainFree() - not sure how else to do this except for changing goto end to goto out and then looping through the list to free. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 140613919317 -r 651039e9392c src/Virt_EnabledLogicalElementCapabilities.c --- a/src/Virt_EnabledLogicalElementCapabilities.c Wed Nov 21 08:23:33 2007 -0800 +++ b/src/Virt_EnabledLogicalElementCapabilities.c Tue Nov 27 13:31:58 2007 -0800 @@ -31,6 +31,7 @@ #include "misc_util.h" #include "device_parsing.h" +#include "cs_util.h" #include "Virt_EnabledLogicalElementCapabilities.h" @@ -103,20 +104,12 @@ static CMPIStatus set_inst_properties(co CMPIStatus get_ele_cap(const CMPIBroker *broker, const CMPIObjectPath *ref, + const char *sys_name, CMPIInstance **inst) { CMPIStatus s; CMPIObjectPath *op; char *classname = NULL; - char *sys_name = NULL; - - sys_name = cu_get_str_path(ref, "Name"); - if (sys_name == NULL) { - CMSetStatusWithChars(broker, &s, - CMPI_RC_ERR_FAILED, - "Missing key: Name"); - goto out; - } classname = get_typed_class(CLASSNAME(ref), "EnabledLogicalElementCapabilities"); @@ -147,7 +140,6 @@ CMPIStatus get_ele_cap(const CMPIBroker out: free(classname); - free(sys_name); return s; } @@ -158,16 +150,50 @@ static CMPIStatus return_ele_cap(const C { CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *inst = NULL; - - s = get_ele_cap(_BROKER, ref, &inst); - if (s.rc != CMPI_RC_OK) - goto out; - - if (names_only) - cu_return_instance_name(results, inst); - else - CMReturnInstance(results, inst); + virConnectPtr conn = NULL; + virDomainPtr *list = NULL; + int count; + int i; + const char *name; + + conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); + if (conn == NULL) + goto out; + + count = get_domain_list(conn, &list); + if (count <= 0) + goto out; + + for (i = 0; i < count; i++) { + name = virDomainGetName(list[i]); + if (name == NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Unable to get domain names"); + goto end; + } + + s = get_ele_cap(_BROKER, ref, name, &inst); + if (s.rc != CMPI_RC_OK) + goto end; + + if (names_only) + cu_return_instance_name(results, inst); + else + CMReturnInstance(results, inst); + + end: + virDomainFree(list[i]); + + if (s.rc != CMPI_RC_OK) + goto out; + } + out: + free(list); + + virConnectClose(conn); + return s; } diff -r 140613919317 -r 651039e9392c src/Virt_EnabledLogicalElementCapabilities.h --- a/src/Virt_EnabledLogicalElementCapabilities.h Wed Nov 21 08:23:33 2007 -0800 +++ b/src/Virt_EnabledLogicalElementCapabilities.h Tue Nov 27 13:31:58 2007 -0800 @@ -20,6 +20,7 @@ */ CMPIStatus get_ele_cap(const CMPIBroker *broker, const CMPIObjectPath *ref, + const char *sys_name, CMPIInstance **inst); /* * Local Variables:
KR> + count = get_domain_list(conn, &list); KR> + if (count <= 0) KR> + goto out; You create the list before the for() loop, KR> + KR> + for (i = 0; i < count; i++) { KR> + name = virDomainGetName(list[i]); KR> + if (name == NULL) { KR> + cu_statusf(_BROKER, &s, KR> + CMPI_RC_ERR_FAILED, KR> + "Unable to get domain names"); KR> + goto end; KR> + } KR> + KR> + s = get_ele_cap(_BROKER, ref, name, &inst); KR> + if (s.rc != CMPI_RC_OK) KR> + goto end; KR> + KR> + if (names_only) KR> + cu_return_instance_name(results, inst); KR> + else KR> + CMReturnInstance(results, inst); KR> + KR> + end: KR> + virDomainFree(list[i]); KR> + KR> + if (s.rc != CMPI_RC_OK) KR> + goto out; But then clean it up every time through the loop, which should be causing a crash on the 1..n iterations. If you're not seeing a crash, I imagine you're getting lucky. virDomainFree() protects against a double-free with some magic, I think, but using list after the first time through here is broken. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
DS> But then clean it up every time through the loop, which should be DS> causing a crash on the 1..n iterations. If you're not seeing a DS> crash, I imagine you're getting lucky. virDomainFree() protects DS> against a double-free with some magic, I think, but using list DS> after the first time through here is broken. Okay, that was just dumb of me. I took something in your commit message and inferred code from it without reading it carefully. What you're doing is correct, and it's how we do it elsewhere in the providers. My mistake :) -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
# HG changeset patch # User Kaitlin Rupert <karupert@us.ibm.com> # Date 1196199121 28800 # Node ID d9fa7a6eb180e5489209a7b893e40c15ad0f1943 # Parent 651039e9392c60f96b6cbaa63f0a239df5357550 Updated to support changes to get_ele_cap(). get_ele_cap() now takes a char * argument to use for building the InstanceID. The function needs to get the Name from the ref and pass that to get_ele_cap(). This bit of code used to be in get_ele_cap() itself, but get_ele_cap() needs to be more generic. Signed-off-by: Kaitlin Rupert <karupert@us.ibm.com> diff -r 651039e9392c -r d9fa7a6eb180 src/Virt_ElementCapabilities.c --- a/src/Virt_ElementCapabilities.c Tue Nov 27 13:31:58 2007 -0800 +++ b/src/Virt_ElementCapabilities.c Tue Nov 27 13:32:01 2007 -0800 @@ -133,10 +133,22 @@ static CMPIStatus cs_to_cap(const CMPIOb { CMPIInstance *inst; CMPIStatus s = {CMPI_RC_OK, NULL}; - - s = get_ele_cap(_BROKER, ref, &inst); + char *sys_name = NULL; + + sys_name = cu_get_str_path(ref, "Name"); + if (sys_name == NULL) { + CMSetStatusWithChars(_BROKER, &s, + CMPI_RC_ERR_FAILED, + "Missing key: Name"); + goto out; + } + + s = get_ele_cap(_BROKER, ref, sys_name, &inst); if (s.rc == CMPI_RC_OK) inst_list_add(list, inst); + + out: + free(sys_name); return s; }
Kaitlin Rupert wrote:
This set of patches fixes the problem where ein/ei on EnabledLogicalElementCapabilities returns a "Missing key: Name" error.
works for me :) ... +1 -- Regards Heidi Eckhart Software Engineer Linux Technology Center - Open Hypervisor heidieck@linux.vnet.ibm.com ************************************************** IBM Deutschland Entwicklung GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschaeftsfuehrung: Herbert Kircher Sitz der Gesellschaft: Boeblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert