[PATCH 0 of 3] #3 - SDC: Fixed type of returned instance and error code
Patch 1: The returned RASD instances have been of the general classname ResourceAllocationSettingData instead of the type specific one, e.g. ProcResourceAllocationSettingData Patch 2: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK Diff to #1: patch 1: fixed missing check for unknown resource type Diff to #2: - added helper function to RASD that translates the resource type into the classname (patch 1 of 3) - updated patch 2 (#1 before) to make use of the helper function - updated patch 3 (#2 before) to make use of RETURN_UNSUPPORTED macro
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1197624963 -3600 # Node ID 1e73930c047813c85341f2c2be32903c466cc336 # Parent 86999a1e8fac6f6cee5ca62a4005ca37759713ff RASD: Added helper function to check for the given resource type Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 86999a1e8fac -r 1e73930c0478 src/Virt_RASD.c --- a/src/Virt_RASD.c Wed Dec 12 17:22:06 2007 -0800 +++ b/src/Virt_RASD.c Fri Dec 14 10:36:03 2007 +0100 @@ -239,6 +239,30 @@ CMPIrc rasd_type_from_classname(const ch return rc; } +CMPIrc rasd_classname_from_type(uint16_t type, const char **classname) +{ + CMPIrc rc = CMPI_RC_OK; + + switch(type) { + case CIM_RASD_TYPE_MEM: + *classname = "MemResourceAllocationSettingData"; + break; + case CIM_RASD_TYPE_PROC: + *classname = "ProcResourceAllocationSettingData"; + break; + case CIM_RASD_TYPE_NET: + *classname = "NetResourceAllocationSettingData"; + break; + case CIM_RASD_TYPE_DISK: + *classname = "DiskResourceAllocationSettingData"; + break; + default: + rc = CMPI_RC_ERR_FAILED; + } + + return rc; +} + static CMPIStatus GetInstance(CMPIInstanceMI *self, const CMPIContext *context, const CMPIResult *results, diff -r 86999a1e8fac -r 1e73930c0478 src/Virt_RASD.h --- a/src/Virt_RASD.h Wed Dec 12 17:22:06 2007 -0800 +++ b/src/Virt_RASD.h Fri Dec 14 10:36:03 2007 +0100 @@ -40,6 +40,7 @@ int rasds_for_domain(const CMPIBroker *b struct inst_list *_list); CMPIrc rasd_type_from_classname(const char *cn, uint16_t *type); +CMPIrc rasd_classname_from_type(uint16_t type, const char **cn); CMPIInstance *get_rasd_instance(const CMPIContext *context, const CMPIObjectPath *ref,
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1197625019 -3600 # Node ID fed85a12807e46622ffea62938173db4b56706a4 # Parent 1e73930c047813c85341f2c2be32903c466cc336 SDC: returned instances are not of type specific subclass Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 1e73930c0478 -r fed85a12807e src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Fri Dec 14 10:36:03 2007 +0100 +++ b/src/Virt_SettingsDefineCapabilities.c Fri Dec 14 10:36:59 2007 +0100 @@ -42,6 +42,7 @@ #include "Virt_SettingsDefineCapabilities.h" #include "Virt_DevicePool.h" +#include "Virt_RASD.h" const static CMPIBroker *_BROKER; @@ -658,6 +659,7 @@ static CMPIInstance *sdc_rasd_inst(const struct sdc_rasd_prop *prop_list; int i; char *inst_id; + const char *base = NULL; uint16_t resource_type; /* Defaults for the following are from CIM_SettingsDefineCapabilities.mof. */ @@ -704,9 +706,16 @@ static CMPIInstance *sdc_rasd_inst(const if (s->rc != CMPI_RC_OK) goto out; + if (rasd_classname_from_type(rasd->resource_type, &base) != CMPI_RC_OK) { + cu_statusf(broker, s, + CMPI_RC_ERR_FAILED, + "Resource type not known"); + goto err; + } + inst = get_typed_instance(broker, CLASSNAME(ref), - "ResourceAllocationSettingData", + base, NAMESPACE(ref)); CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); @@ -723,6 +732,7 @@ static CMPIInstance *sdc_rasd_inst(const prop_list[i].value, prop_list[i].type); } + err: CU_DEBUG("freeing prop_list"); free_rasd_prop_list(prop_list); out:
HE> + if (rasd_classname_from_type(rasd->resource_type, &base) != CMPI_RC_OK) { HE> + cu_statusf(broker, s, HE> + CMPI_RC_ERR_FAILED, HE> + "Resource type not known"); HE> + goto err; HE> + } HE> + HE> inst = get_typed_instance(broker, HE> CLASSNAME(ref), HE> - "ResourceAllocationSettingData", HE> + base, HE> NAMESPACE(ref)); HE> CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); HE> @@ -723,6 +732,7 @@ static CMPIInstance *sdc_rasd_inst(const HE> prop_list[i].value, prop_list[i].type); HE> } HE> + err: HE> CU_DEBUG("freeing prop_list"); HE> free_rasd_prop_list(prop_list); HE> out: I didn't notice this before, but shouldn't we initialize prop_list to NULL, make sure free_rasd_prop_list() can handle NULL, and then put the free call below the out: target? There is another place in the function that looks like it jumps to out after prop_list could have been initialized. Further, it would be better to have it below out, so that additions like this one don't have to add or utilize an err: target. Heidi, I know this isn't really a part of your patch, but I think it's worth looking at while we're here. Jay, can you comment? Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
Dan Smith wrote:
I didn't notice this before, but shouldn't we initialize prop_list to NULL, make sure free_rasd_prop_list() can handle NULL, and then put the free call below the out: target? There is another place in the function that looks like it jumps to out after prop_list could have been initialized. Further, it would be better to have it below out, so that additions like this one don't have to add or utilize an err: target.
Heidi, I know this isn't really a part of your patch, but I think it's worth looking at while we're here.
Jay, can you comment?
Thanks!
This seems like a reasonable request to me. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
Dan Smith wrote:
I didn't notice this before, but shouldn't we initialize prop_list to NULL, make sure free_rasd_prop_list() can handle NULL, and then put the free call below the out: target? There is another place in the function that looks like it jumps to out after prop_list could have been initialized. Further, it would be better to have it below out, so that additions like this one don't have to add or utilize an err: target.
Heidi, I know this isn't really a part of your patch, but I think it's worth looking at while we're here.
A very good idea ! I will prepare the patch and sent it out with the updates for this one.
Jay, can you comment?
Thanks!
-- 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
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1197625021 -3600 # Node ID 26266b51af6a2e435dc7e5aad6df89df4cf23db9 # Parent fed85a12807e46622ffea62938173db4b56706a4 SDC: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r fed85a12807e -r 26266b51af6a src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Fri Dec 14 10:36:59 2007 +0100 +++ b/src/Virt_SettingsDefineCapabilities.c Fri Dec 14 10:37:01 2007 +0100 @@ -824,11 +824,7 @@ static CMPIStatus rasd_to_alloc_cap(cons struct std_assoc_info *info, struct inst_list *list) { - CMPIStatus s = {CMPI_RC_OK}; - - /* This direction of the association currently not supported. */ - - return s; + RETURN_UNSUPPORTED(); } LIBVIRT_CIM_DEFAULT_MAKEREF()
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert