[PATCH 0 of 3] #2 - Check client given object pathes in ElementCapabilities provider
To enable the check for the client's given object path in ElementCapabilities, changes to the ComputerSystem and AllocationCapabilities provider have been necessary. Diff to patch #1: - added a new function to libcmpiutil - cu_validate_ref(ref, inst) - that does the check between the system instance and the client given instance - get_domain() is now using the new libcmpiutil function - the parameter CMPIInstance **inst is now used to "configure" get_domain(); for internal usage the inst is returned and for external usage this can be ignored by setting it to NULL - validate_domain_ref() is now only an interface name for the client - removed patch unrelated changes in patch 2 - reworked logic to return instance(s)
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1200052158 -3600 # Node ID ec66d30629f4584ca30751ce3e079f5561f2985a # Parent b69727ddc9a45a32c75547a90364759500280398 Add function to validate the client given object path of a ComputerSystem Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r b69727ddc9a4 -r ec66d30629f4 src/Virt_ComputerSystem.c --- a/src/Virt_ComputerSystem.c Thu Jan 10 11:04:39 2008 +0100 +++ b/src/Virt_ComputerSystem.c Fri Jan 11 12:49:18 2008 +0100 @@ -285,7 +285,7 @@ CMPIInstance *instance_from_name(const C dom = virDomainLookupByName(conn, name); if (dom == NULL) - return 0; + return NULL; instance = get_typed_instance(broker, pfx_from_conn(conn), @@ -374,46 +374,66 @@ static CMPIStatus return_enum_domains(co return s; } -static CMPIStatus get_domain(const CMPIObjectPath *reference, - const CMPIResult *results, - const char *name) -{ - CMPIInstance *inst; - CMPIStatus s; +static CMPIStatus get_domain(const CMPIBroker *broker, + const CMPIObjectPath *reference, + CMPIInstance **inst) +{ + CMPIInstance *_inst; + CMPIStatus s = {CMPI_RC_OK, NULL}; virConnectPtr conn = NULL; - const char *prop = NULL; - - if (!provider_is_responsible(_BROKER, reference, &s)) { - CMSetStatus(&s, CMPI_RC_ERR_NOT_FOUND); + const char *name; + + if (!provider_is_responsible(broker, reference, &s)) return s; - } - - conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); + + if (cu_get_str_path(reference, "Name", &name) != CMPI_RC_OK) { + cu_statusf(broker, &s, + CMPI_RC_ERR_FAILED, + "No domain name specified"); + return s; + } + + conn = connect_by_classname(broker, CLASSNAME(reference), &s); if (conn == NULL) return s; - inst = instance_from_name(_BROKER, conn, name, reference); - if (inst == NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Unable to find `%s'", name); - goto out; - } - - prop = cu_compare_ref(reference, inst); - if (prop != NULL) { - cu_statusf(_BROKER, &s, + _inst = instance_from_name(broker, conn, name, reference); + if (_inst == NULL) { + cu_statusf(broker, &s, CMPI_RC_ERR_NOT_FOUND, - "No such instance (%s)", prop); - goto out; - } - - CMReturnInstance(results, inst); - CMSetStatus(&s, CMPI_RC_OK); + "No such instance (%s)", name); + goto out; + } + + s = cu_validate_ref(broker, reference, _inst); + out: virConnectClose(conn); - - return s; + if (inst) + *inst = _inst; + + return s; +} + +static CMPIStatus return_domain(const CMPIObjectPath *reference, + const CMPIResult *results) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst; + + s = get_domain(_BROKER, reference, &inst); + if (s.rc != CMPI_RC_OK) + return s; + + CMReturnInstance(results, inst); + + return s; +} + +CMPIStatus validate_domain_ref(const CMPIBroker *broker, + const CMPIObjectPath *ref) +{ + return get_domain(broker, ref, NULL); } static CMPIStatus EnumInstanceNames(CMPIInstanceMI *self, @@ -440,19 +460,7 @@ static CMPIStatus GetInstance(CMPIInstan const CMPIObjectPath *reference, const char **properties) { - const char *name; - - if (cu_get_str_path(reference, "Name", &name) != CMPI_RC_OK) { - CMPIStatus s; - - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "No domain name specified"); - - return s; - } - - return get_domain(reference, results, name); + return return_domain(reference, results); } DEFAULT_CI(); diff -r b69727ddc9a4 -r ec66d30629f4 src/Virt_ComputerSystem.h --- a/src/Virt_ComputerSystem.h Thu Jan 10 11:04:39 2008 +0100 +++ b/src/Virt_ComputerSystem.h Fri Jan 11 12:49:18 2008 +0100 @@ -52,6 +52,16 @@ int enum_domains(const CMPIBroker *broke const char *ns, struct inst_list *instlist); +/** + * Validate the client given domain object path + * + * @param broker A pointer to the current broker + * @param ref The client given object path + * @returns CMPIStatus + */ +CMPIStatus validate_domain_ref(const CMPIBroker *broker, + const CMPIObjectPath *ref); + #endif
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1200052159 -3600 # Node ID 224b84cf00ac2e120d86e15d92ce50fddf6a0105 # Parent ec66d30629f4584ca30751ce3e079f5561f2985a Make alloc_cap_instances() available for external use by EC The association provider ElementCapabilities needs the enum_alloc_cap_instances() function to access to the list of available AllocationCapabilities. These are returned by the additional inst_list parameter. Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r ec66d30629f4 -r 224b84cf00ac src/Virt_AllocationCapabilities.c --- a/src/Virt_AllocationCapabilities.c Fri Jan 11 12:49:18 2008 +0100 +++ b/src/Virt_AllocationCapabilities.c Fri Jan 11 12:49:19 2008 +0100 @@ -31,6 +31,7 @@ #include "misc_util.h" +#include "Virt_AllocationCapabilities.h" #include "Virt_DevicePool.h" const static CMPIBroker *_BROKER; @@ -71,30 +72,26 @@ static CMPIStatus ac_from_pool(const CMP return s; } -static CMPIStatus alloc_cap_instances(const CMPIBroker *broker, - const CMPIObjectPath *ref, - const CMPIResult *results, - bool names_only, - const char **properties, - const char *id) -{ +CMPIStatus enum_alloc_cap_instances(const CMPIBroker *broker, + const CMPIObjectPath *ref, + const char **properties, + const char *id, + struct inst_list *list) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *alloc_cap_inst; + virConnectPtr conn = NULL; + struct inst_list device_pool_list; + const char *inst_id; int i; - virConnectPtr conn = NULL; - CMPIInstance *alloc_cap_inst; - struct inst_list alloc_cap_list; - struct inst_list device_pool_list; - CMPIStatus s = {CMPI_RC_OK, NULL}; - const char *inst_id; - - CU_DEBUG("In alloc_cap_instances()"); - + + inst_list_init(list); inst_list_init(&device_pool_list); - inst_list_init(&alloc_cap_list); if (!provider_is_responsible(broker, ref, &s)) goto out; - conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); + conn = connect_by_classname(broker, CLASSNAME(ref), &s); if (conn == NULL) { cu_statusf(broker, &s, CMPI_RC_ERR_FAILED, @@ -129,7 +126,7 @@ static CMPIStatus alloc_cap_instances(co if (s.rc != CMPI_RC_OK) goto out; - inst_list_add(&alloc_cap_list, alloc_cap_inst); + inst_list_add(list, alloc_cap_inst); if (id && (STREQ(inst_id, id))) break; @@ -141,16 +138,40 @@ static CMPIStatus alloc_cap_instances(co "Requested Object could not be found."); goto out; } - - if (names_only) - cu_return_instance_names(results, &alloc_cap_list); - else - cu_return_instances(results, &alloc_cap_list); - + out: virConnectClose(conn); - inst_list_free(&alloc_cap_list); inst_list_free(&device_pool_list); + + return s; +} + +static CMPIStatus return_alloc_cap_instances(const CMPIBroker *broker, + const CMPIObjectPath *ref, + const CMPIResult *results, + bool names_only, + const char **properties, + const char *id) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + struct inst_list *list = NULL; + + s = enum_alloc_cap_instances(broker, + ref, + properties, + id, + list); + if (s.rc != CMPI_RC_OK) + goto out; + + if (names_only) + cu_return_instance_names(results, list); + else + cu_return_instances(results, list); + + out: + inst_list_free(list); + return s; } @@ -170,12 +191,12 @@ static CMPIStatus GetInstance(CMPIInstan return s; } - return alloc_cap_instances(_BROKER, - reference, - results, - false, - properties, - id); + return return_alloc_cap_instances(_BROKER, + reference, + results, + false, + properties, + id); } static CMPIStatus EnumInstanceNames(CMPIInstanceMI *self, @@ -183,12 +204,12 @@ static CMPIStatus EnumInstanceNames(CMPI const CMPIResult *results, const CMPIObjectPath *reference) { - return alloc_cap_instances(_BROKER, - reference, - results, - true, - NULL, - NULL); + return return_alloc_cap_instances(_BROKER, + reference, + results, + true, + NULL, + NULL); } static CMPIStatus EnumInstances(CMPIInstanceMI *self, @@ -197,12 +218,12 @@ static CMPIStatus EnumInstances(CMPIInst const CMPIObjectPath *reference, const char **properties) { - return alloc_cap_instances(_BROKER, - reference, - results, - false, - properties, - NULL); + return return_alloc_cap_instances(_BROKER, + reference, + results, + false, + properties, + NULL); } DEFAULT_CI(); diff -r ec66d30629f4 -r 224b84cf00ac src/Virt_AllocationCapabilities.h --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/src/Virt_AllocationCapabilities.h Fri Jan 11 12:49:19 2008 +0100 @@ -0,0 +1,53 @@ +/* + * Copyright IBM Corp. 2008 + * + * Authors: + * Heidi Eckhart <heidieck@linux.vnet.ibm.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +#ifndef __VIRT_ALLOCATIONCAPABILITIES_H +#define __VIRT_ALLOCATIONCAPABILITIES_H + +#include "misc_util.h" + +/** + * Return the instance of the AllocationCapabilities instance, + * defined by the id + * + * @param broker A pointer to the current broker + * @param ref The reference + * @param properties list of properties to set + * @param id The InstanceID of the AllocationCapabilities + * @param inst The list of instance(s) in case of success + * @returns The status of this operation + */ +CMPIStatus enum_alloc_cap_instances(const CMPIBroker *broker, + const CMPIObjectPath *ref, + const char **properties, + const char *id, + struct inst_list *list); + +#endif + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */
HE> + CMPIStatus s = {CMPI_RC_OK, NULL}; HE> + CMPIInstance *alloc_cap_inst; HE> + virConnectPtr conn = NULL; HE> + struct inst_list device_pool_list; HE> + const char *inst_id; HE> int i; HE> - virConnectPtr conn = NULL; HE> - CMPIInstance *alloc_cap_inst; HE> - struct inst_list alloc_cap_list; HE> - struct inst_list device_pool_list; HE> - CMPIStatus s = {CMPI_RC_OK, NULL}; HE> - const char *inst_id; Reordering this block for no reason makes it really difficult to tell what is changed. Can we please keep this kind of thing separate from the functional change to the code? HE> + inst_list_init(list); In other places, we initialize the list before we pass it into a function. I think it makes more sense that way, as the caller may want the results of this function appended to the list it passes in. HE> +static CMPIStatus return_alloc_cap_instances(const CMPIBroker *broker, HE> + const CMPIObjectPath *ref, HE> + const CMPIResult *results, HE> + bool names_only, HE> + const char **properties, HE> + const char *id) HE> +{ HE> + CMPIStatus s = {CMPI_RC_OK, NULL}; HE> + struct inst_list *list = NULL; HE> + HE> + s = enum_alloc_cap_instances(broker, HE> + ref, HE> + properties, HE> + id, HE> + list); HE> + if (s.rc != CMPI_RC_OK) HE> + goto out; HE> + HE> + if (names_only) HE> + cu_return_instance_names(results, list); HE> + else HE> + cu_return_instances(results, list); This doesn't work, does it? You pass a NULL list pointer in to the function, it allocates a list, but has no way to return it back. You're passing just a single pointer by value here. If you were modifying what the pointer pointed to, then you could get the result, but otherwise the list that the enum_alloc_cap_instances() function generates isn't visible to this caller. You should do the following: struct inst_list list; inst_list_init(&list); s = enum_alloc_cap_instances(broker, ref, properties, id, &list); Which addresses this issue as well as the previous one. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
HE> + CMPIStatus s = {CMPI_RC_OK, NULL}; HE> + CMPIInstance *alloc_cap_inst; HE> + virConnectPtr conn = NULL; HE> + struct inst_list device_pool_list; HE> + const char *inst_id; HE> int i; HE> - virConnectPtr conn = NULL; HE> - CMPIInstance *alloc_cap_inst; HE> - struct inst_list alloc_cap_list; HE> - struct inst_list device_pool_list; HE> - CMPIStatus s = {CMPI_RC_OK, NULL}; HE> - const char *inst_id;
Reordering this block for no reason makes it really difficult to tell what is changed. Can we please keep this kind of thing separate from the functional change to the code?
Sorry, this happened as changes went forward. I will fix this.
HE> + inst_list_init(list);
In other places, we initialize the list before we pass it into a function. I think it makes more sense that way, as the caller may want the results of this function appended to the list it passes in.
HE> +static CMPIStatus return_alloc_cap_instances(const CMPIBroker *broker, HE> + const CMPIObjectPath *ref, HE> + const CMPIResult *results, HE> + bool names_only, HE> + const char **properties, HE> + const char *id) HE> +{ HE> + CMPIStatus s = {CMPI_RC_OK, NULL}; HE> + struct inst_list *list = NULL; HE> + HE> + s = enum_alloc_cap_instances(broker, HE> + ref, HE> + properties, HE> + id, HE> + list); HE> + if (s.rc != CMPI_RC_OK) HE> + goto out; HE> + HE> + if (names_only) HE> + cu_return_instance_names(results, list); HE> + else HE> + cu_return_instances(results, list);
This doesn't work, does it? You pass a NULL list pointer in to the function, it allocates a list, but has no way to return it back. You're passing just a single pointer by value here. If you were modifying what the pointer pointed to, then you could get the result, but otherwise the list that the enum_alloc_cap_instances() function generates isn't visible to this caller.
You should do the following:
struct inst_list list;
inst_list_init(&list);
s = enum_alloc_cap_instances(broker, ref, properties, id, &list);
Which addresses this issue as well as the previous one.
Thanks!
Oh, oh, what a bad day in my development time. You are absolutely right. This setup has no chance to return the result to the caller. And I haven't tested as extensively as I should have done. An excellent catch. Thank you. :) -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1200052159 -3600 # Node ID 224b84cf00ac2e120d86e15d92ce50fddf6a0105 # Parent ec66d30629f4584ca30751ce3e079f5561f2985a Make alloc_cap_instances() available for external use by EC
The association provider ElementCapabilities needs the enum_alloc_cap_instances() function to access to the list of available AllocationCapabilities. These are returned by the additional inst_list parameter.
Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com>
diff -r ec66d30629f4 -r 224b84cf00ac src/Virt_AllocationCapabilities.c --- a/src/Virt_AllocationCapabilities.c Fri Jan 11 12:49:18 2008 +0100 +++ b/src/Virt_AllocationCapabilities.c Fri Jan 11 12:49:19 2008 +0100 @@ -31,6 +31,7 @@
#include "misc_util.h"
+#include "Virt_AllocationCapabilities.h" #include "Virt_DevicePool.h"
const static CMPIBroker *_BROKER; @@ -71,30 +72,26 @@ static CMPIStatus ac_from_pool(const CMP return s; }
-static CMPIStatus alloc_cap_instances(const CMPIBroker *broker, - const CMPIObjectPath *ref, - const CMPIResult *results, - bool names_only, - const char **properties, - const char *id) -{ +CMPIStatus enum_alloc_cap_instances(const CMPIBroker *broker, + const CMPIObjectPath *ref, + const char **properties, + const char *id, + struct inst_list *list) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *alloc_cap_inst; + virConnectPtr conn = NULL; + struct inst_list device_pool_list; + const char *inst_id; int i; - virConnectPtr conn = NULL; - CMPIInstance *alloc_cap_inst; - struct inst_list alloc_cap_list; - struct inst_list device_pool_list; - CMPIStatus s = {CMPI_RC_OK, NULL}; - const char *inst_id; - - CU_DEBUG("In alloc_cap_instances()"); - + + inst_list_init(list); inst_list_init(&device_pool_list); - inst_list_init(&alloc_cap_list);
Time for me to put on the PCO hat here. You've got some serious declaration reordering here, and it's really muddying up the change. I think you've got about three lines changed, but it's producing over 20 lines of diff. That can definitely be cleaned up. -- -Jay
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1200052160 -3600 # Node ID 90746174d66eb4d4ceb2b15633ff348a553ab908 # Parent 224b84cf00ac2e120d86e15d92ce50fddf6a0105 EC: Validation of client given object path missing The validation of the client's given ResourcePool object path is missing. Instead the submitted InstanceID is taken as value for the AllocationCapabilities InstanceID. wbemain -ac CIM_ElementCapabilities 'http://localhost/root/virt:KVM_DiskPool.InstanceID="notthere"' returns localhost:5988/root/virt:KVM_AllocationCapabilities.InstanceID="notthere" Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 224b84cf00ac -r 90746174d66e src/Makefile.am --- a/src/Makefile.am Fri Jan 11 12:49:19 2008 +0100 +++ b/src/Makefile.am Fri Jan 11 12:49:20 2008 +0100 @@ -89,17 +89,18 @@ libVirt_ElementConformsToProfile_la_LIBA libVirt_EnabledLogicalElementCapabilities_la_SOURCES = Virt_EnabledLogicalElementCapabilities.c +libVirt_AllocationCapabilities_la_DEPENDENCIES = libVirt_DevicePool.la +libVirt_AllocationCapabilities_la_SOURCES = Virt_AllocationCapabilities.c +libVirt_AllocationCapabilities_la_LIBADD = -lVirt_DevicePool + libVirt_ElementCapabilities_la_DEPENDENCIES = libVirt_VirtualSystemManagementCapabilities.la libVirt_EnabledLogicalElementCapabilities.la libVirt_ComputerSystem.la libVirt_HostSystem.la libVirt_VSMigrationCapabilities.la libVirt_ElementCapabilities_la_SOURCES = Virt_ElementCapabilities.c libVirt_ElementCapabilities_la_LIBADD = -lVirt_VirtualSystemManagementCapabilities \ -lVirt_EnabledLogicalElementCapabilities \ -lVirt_ComputerSystem \ -lVirt_HostSystem \ - -lVirt_VSMigrationCapabilities - -libVirt_AllocationCapabilities_la_DEPENDENCIES = libVirt_DevicePool.la -libVirt_AllocationCapabilities_la_SOURCES = Virt_AllocationCapabilities.c -libVirt_AllocationCapabilities_la_LIBADD = -lVirt_DevicePool + -lVirt_VSMigrationCapabilities \ + -lVirt_AllocationCapabilities libVirt_SettingsDefineCapabilities_la_DEPENDENCIES = libVirt_RASD.la libVirt_DevicePool.la libVirt_SettingsDefineCapabilities_la_SOURCES = Virt_SettingsDefineCapabilities.c diff -r 224b84cf00ac -r 90746174d66e src/Virt_ElementCapabilities.c --- a/src/Virt_ElementCapabilities.c Fri Jan 11 12:49:19 2008 +0100 +++ b/src/Virt_ElementCapabilities.c Fri Jan 11 12:49:20 2008 +0100 @@ -37,6 +37,7 @@ #include "Virt_ComputerSystem.h" #include "Virt_HostSystem.h" #include "Virt_VSMigrationCapabilities.h" +#include "Virt_AllocationCapabilities.h" /* Associate an XXX_Capabilities to the proper XXX_ManagedElement. * @@ -46,6 +47,37 @@ */ const static CMPIBroker *_BROKER; + +static CMPIStatus validate_host_caps_ref(const CMPIObjectPath *ref) +{ + CMPIStatus s = {CMPI_RC_OK, NULL}; + CMPIInstance *inst; + const char *prop; + char* classname; + + classname = class_base_name(CLASSNAME(ref)); + + if (STREQC(classname, "VirtualSystemManagementCapabilities")) { + s = get_vsm_cap(_BROKER, ref, &inst); + } else if (STREQC(classname, "VirtualSystemMigrationCapabilities")) { + s = get_migration_caps(ref, &inst, _BROKER); + } + + if (s.rc != CMPI_RC_OK) + goto out; + + prop = cu_compare_ref(ref, inst); + if (prop != NULL) { + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", prop); + } + + out: + free(classname); + + return s; +} static CMPIStatus sys_to_cap(const CMPIObjectPath *ref, struct std_assoc_info *info, @@ -53,22 +85,13 @@ static CMPIStatus sys_to_cap(const CMPIO { CMPIInstance *inst; CMPIStatus s = {CMPI_RC_OK, NULL}; - const char *prop; - - if (!match_hypervisor_prefix(ref, info)) - return s; - - s = get_host_cs(_BROKER, ref, &inst); - if (s.rc != CMPI_RC_OK) - goto out; - - prop = cu_compare_ref(ref, inst); - if (prop != NULL) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "No such HostSystem (%s)", prop); - goto out; - } + + if (!match_hypervisor_prefix(ref, info)) + goto out; + + s = validate_host_ref(_BROKER, ref); + if (s.rc != CMPI_RC_OK) + goto out; s = get_vsm_cap(_BROKER, ref, &inst); if (s.rc == CMPI_RC_OK) @@ -90,7 +113,11 @@ static CMPIStatus cap_to_sys(const CMPIO CMPIStatus s = {CMPI_RC_OK, NULL}; if (!match_hypervisor_prefix(ref, info)) - return s; + goto out; + + s = validate_host_caps_ref(ref); + if (s.rc != CMPI_RC_OK) + goto out; s = get_host_cs(_BROKER, ref, &inst); if (s.rc != CMPI_RC_OK) @@ -111,7 +138,11 @@ static CMPIStatus cs_to_cap(const CMPIOb const char *sys_name = NULL; if (!match_hypervisor_prefix(ref, info)) - return s; + goto out; + + s = validate_domain_ref(_BROKER, ref); + if (s.rc != CMPI_RC_OK) + goto out; if (cu_get_str_path(ref, "Name", &sys_name) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, @@ -138,26 +169,30 @@ static CMPIStatus cap_to_cs(const CMPIOb CMPIStatus s = {CMPI_RC_OK, NULL}; if (!match_hypervisor_prefix(ref, info)) - return s; + goto out; if (cu_get_str_path(ref, "InstanceID", &inst_id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_FAILED, "Could not get InstanceID"); - goto error1; + goto out; } conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); if (s.rc != CMPI_RC_OK) - goto error1; + goto out; inst = instance_from_name(_BROKER, conn, inst_id, ref); if (inst) inst_list_add(list, inst); - + else + cu_statusf(_BROKER, &s, + CMPI_RC_ERR_NOT_FOUND, + "No such instance (%s)", inst_id); + virConnectClose(conn); - error1: - + + out: return s; } @@ -173,14 +208,11 @@ static CMPIStatus pool_to_alloc(const CM struct std_assoc_info *info, struct inst_list *list) { - int ret; + CMPIStatus s = {CMPI_RC_OK, NULL}; const char *inst_id; - uint16_t type; - CMPIInstance *inst = NULL; - CMPIStatus s = {CMPI_RC_OK}; - - if (!match_hypervisor_prefix(ref, info)) - return s; + + if (!match_hypervisor_prefix(ref, info)) + goto out; if (cu_get_str_path(ref, "InstanceID", &inst_id) != CMPI_RC_OK) { cu_statusf(_BROKER, &s, @@ -189,22 +221,11 @@ static CMPIStatus pool_to_alloc(const CM goto out; } - inst = get_typed_instance(_BROKER, - CLASSNAME(ref), - "AllocationCapabilities", - NAMESPACE(ref)); - CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); - - ret = cu_get_u16_path(ref, "ResourceType", &type); - if (ret != 1) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_FAILED, - "Could not get ResourceType"); - goto out; - } - CMSetProperty(inst, "ResourceType", &type, CMPI_uint16); - - inst_list_add(list, inst); + s = enum_alloc_cap_instances(_BROKER, + ref, + NULL, + inst_id, + list); out: return s;
Heidi Eckhart wrote:
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1200052160 -3600 # Node ID 90746174d66eb4d4ceb2b15633ff348a553ab908 # Parent 224b84cf00ac2e120d86e15d92ce50fddf6a0105 EC: Validation of client given object path missing
conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); if (s.rc != CMPI_RC_OK) - goto error1; + goto out;
Should this also check for to see if conn == NULL? -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
Kaitlin Rupert wrote:
Heidi Eckhart wrote:
conn = connect_by_classname(_BROKER, CLASSNAME(ref), &s); if (s.rc != CMPI_RC_OK) - goto error1; + goto out;
Should this also check for to see if conn == NULL?
Excellent catch - we always check for conn == NULL. -- Regards Heidi Eckhart Software Engineer IBM Linux Technology Center - Open Hypervisor
participants (4)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon -
Kaitlin Rupert