[PATCH 0 of 2] 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
# HG changeset patch # User Heidi Eckhart <heidieck@linux.vnet.ibm.com> # Date 1197378558 -3600 # Node ID 838bd46416cc73ddf67eadce7dbda852927832ed # Parent 6fdfdb1a22c5f790cfd08538a3e29923ccd2237c SDC: returned instances are not of type specific subclass Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 6fdfdb1a22c5 -r 838bd46416cc src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Mon Dec 10 13:28:51 2007 -0800 +++ b/src/Virt_SettingsDefineCapabilities.c Tue Dec 11 14:09:18 2007 +0100 @@ -659,6 +659,7 @@ static CMPIInstance *sdc_rasd_inst(const struct sdc_rasd_prop *prop_list; int i; char *inst_id; + char *base = NULL; uint16_t resource_type; /* Defaults for the following are from CIM_SettingsDefineCapabilities.mof. */ @@ -705,10 +706,26 @@ static CMPIInstance *sdc_rasd_inst(const if (s->rc != CMPI_RC_OK) goto out; - inst = get_typed_instance(broker, - CLASSNAME(ref), - "ResourceAllocationSettingData", - NAMESPACE(ref)); + switch(rasd->resource_type) { + case CIM_RASD_TYPE_MEM: + base = "MemResourceAllocationSettingData"; + break; + case CIM_RASD_TYPE_PROC: + base = "ProcResourceAllocationSettingData"; + break; + case CIM_RASD_TYPE_NET: + base = "NetResourceAllocationSettingData"; + break; + case CIM_RASD_TYPE_DISK: + base = "DiskResourceAllocationSettingData"; + break; + } + + if (base) + inst = get_typed_instance(broker, + CLASSNAME(ref), + base, + NAMESPACE(ref)); CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); CMSetProperty(inst, "PropertyPolicy", &policy, CMPI_uint16);
Heidi Eckhart wrote:
+ if (base) + inst = get_typed_instance(broker, + CLASSNAME(ref), + base, + NAMESPACE(ref));
CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); CMSetProperty(inst, "PropertyPolicy", &policy, CMPI_uint16);
Should there be an else statement here? If the if statement fails, then an instance isn't created. CMSetProperty() would be trying to get the value of a NULL inst, which would most likely fail. -- Kaitlin Rupert IBM Linux Technology Center karupert@us.ibm.com
Kaitlin Rupert wrote:
Heidi Eckhart wrote:
+ if (base) + inst = get_typed_instance(broker, + CLASSNAME(ref), + base, + NAMESPACE(ref)); CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); CMSetProperty(inst, "PropertyPolicy", &policy, CMPI_uint16);
Should there be an else statement here? If the if statement fails, then an instance isn't created. CMSetProperty() would be trying to get the value of a NULL inst, which would most likely fail.
Seems like a good idea to me. Just something simple that sets status as appropriate and bails is all that should be necessary there. -- -Jay
Kaitlin Rupert wrote:
Heidi Eckhart wrote:
+ if (base) + inst = get_typed_instance(broker, + CLASSNAME(ref), + base, + NAMESPACE(ref)); CMSetProperty(inst, "InstanceID", inst_id, CMPI_chars); CMSetProperty(inst, "PropertyPolicy", &policy, CMPI_uint16);
Should there be an else statement here? If the if statement fails, then an instance isn't created. CMSetProperty() would be trying to get the value of a NULL inst, which would most likely fail.
Seems like a good idea to me. Just something simple that sets status as appropriate and bails is all that should be necessary there. Thanks for this very good catch :). I have added a default phrase to the switch statement for resource_type,
Jay Gagnon wrote: that returns an error in case of an unknown resource type and exits right afterwards. -- 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 1197378563 -3600 # Node ID e2644a839312b446a3621327118667b33fca6057 # Parent 838bd46416cc73ddf67eadce7dbda852927832ed SDC: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK Signed-off-by: Heidi Eckhart <heidieck@linux.vnet.ibm.com> diff -r 838bd46416cc -r e2644a839312 src/Virt_SettingsDefineCapabilities.c --- a/src/Virt_SettingsDefineCapabilities.c Tue Dec 11 14:09:18 2007 +0100 +++ b/src/Virt_SettingsDefineCapabilities.c Tue Dec 11 14:09:23 2007 +0100 @@ -832,8 +832,8 @@ static CMPIStatus rasd_to_alloc_cap(cons struct std_assoc_info *info, struct inst_list *list) { - CMPIStatus s = {CMPI_RC_OK}; - + CMPIStatus s = {CMPI_RC_ERR_NOT_SUPPORTED}; + /* This direction of the association currently not supported. */ return s;
participants (3)
-
Heidi Eckhart -
Jay Gagnon -
Kaitlin Rupert