11:05 a.m.
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
11:05 a.m.
New subject: [PATCH 1 of 2] SDC: returned instances are not of type specific subclass
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1197543884 -3600
# Node ID 733f1e73fe50ea102d9c8942e3dfb0bbca2e6f91
# Parent 86999a1e8fac6f6cee5ca62a4005ca37759713ff
SDC: returned instances are not of type specific subclass
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 86999a1e8fac -r 733f1e73fe50 src/Virt_SettingsDefineCapabilities.c
--- a/src/Virt_SettingsDefineCapabilities.c Wed Dec 12 17:22:06 2007 -0800
+++ b/src/Virt_SettingsDefineCapabilities.c Thu Dec 13 12:04:44 2007 +0100
@@ -658,6 +658,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. */
@@ -704,9 +705,29 @@ static CMPIInstance *sdc_rasd_inst(const
if (s->rc != CMPI_RC_OK)
goto out;
+ 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;
+ default:
+ 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 +744,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:
6:05 p.m.
New subject: [PATCH 1 of 2] SDC: returned instances are not of type specific subclass
HE> + char *base = NULL;
This should be a const char *.
HE> + switch(rasd->resource_type) {
HE> + case CIM_RASD_TYPE_MEM:
HE> + base = "MemResourceAllocationSettingData";
HE> + break;
HE> + case CIM_RASD_TYPE_PROC:
HE> + base = "ProcResourceAllocationSettingData";
HE> + break;
HE> + case CIM_RASD_TYPE_NET:
HE> + base = "NetResourceAllocationSettingData";
HE> + break;
HE> + case CIM_RASD_TYPE_DISK:
HE> + base = "DiskResourceAllocationSettingData";
HE> + break;
HE> + default:
HE> + cu_statusf(broker, s,
HE> + CMPI_RC_ERR_FAILED,
HE> + "Resource type not known");
HE> + goto err;
HE> + }
HE> +
Maybe this should be a helper function exposed in Virt_RASD.h, instead
of buried in SDC?
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:59 a.m.
New subject: [PATCH 1 of 2] SDC: returned instances are not of type specific subclass
Dan Smith wrote:
HE> + char *base = NULL;
This should be a const char *.
Good catch.
HE> + switch(rasd->resource_type) {
HE> + case CIM_RASD_TYPE_MEM:
HE> + base = "MemResourceAllocationSettingData";
HE> + break;
HE> + case CIM_RASD_TYPE_PROC:
HE> + base = "ProcResourceAllocationSettingData";
HE> + break;
HE> + case CIM_RASD_TYPE_NET:
HE> + base = "NetResourceAllocationSettingData";
HE> + break;
HE> + case CIM_RASD_TYPE_DISK:
HE> + base = "DiskResourceAllocationSettingData";
HE> + break;
HE> + default:
HE> + cu_statusf(broker, s,
HE> + CMPI_RC_ERR_FAILED,
HE> + "Resource type not known");
HE> + goto err;
HE> + }
HE> +
Maybe this should be a helper function exposed in Virt_RASD.h, instead
of buried in SDC?
A very good idea. I also did not like the length this function
got due
to this change and putting it into a helper of RASD makes much more
sense. I will update the patch and resend it. Thanks.
--
Regards
Heidi Eckhart
Software Engineer
Linux Technology Center - Open Hypervisor
heidieck(a)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
11:05 a.m.
New subject: [PATCH 2 of 2] SDC: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1197543894 -3600
# Node ID 346a3df2e978f42fc44e89c2d60e685befdda5ff
# Parent 733f1e73fe50ea102d9c8942e3dfb0bbca2e6f91
SDC: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 733f1e73fe50 -r 346a3df2e978 src/Virt_SettingsDefineCapabilities.c
--- a/src/Virt_SettingsDefineCapabilities.c Thu Dec 13 12:04:44 2007 +0100
+++ b/src/Virt_SettingsDefineCapabilities.c Thu Dec 13 12:04:54 2007 +0100
@@ -836,8 +836,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;
6:06 p.m.
New subject: [PATCH 2 of 2] SDC: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK
HE> - CMPIStatus s = {CMPI_RC_OK};
HE> -
HE> + CMPIStatus s = {CMPI_RC_ERR_NOT_SUPPORTED};
HE> +
HE> /* This direction of the association currently not supported. */
Not that it's a big deal or anything, but we do have
RETURN_UNSUPPORTED() in libcmpiutil for this scenario. If you like it
and want to re-spin the patch with it, that would be cool. Your
call... :)
Sorry I didn't get to reviewing this set until the second round...
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
9:56 a.m.
New subject: [PATCH 2 of 2] SDC: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK
Dan Smith wrote:
HE> - CMPIStatus s = {CMPI_RC_OK};
HE> -
HE> + CMPIStatus s = {CMPI_RC_ERR_NOT_SUPPORTED};
HE> +
HE> /* This direction of the association currently not supported. */
Not that it's a big deal or anything, but we do have
RETURN_UNSUPPORTED() in libcmpiutil for this scenario.
Sure, thanks for reminding
me :). That's much better than setting the
status by hand. Good catch. I will update the patch and resend with the
changes of patch #1.
If you like it
and want to re-spin the patch with it, that would be cool. Your
call... :)
Sorry I didn't get to reviewing this set until the second round...
Nop.
--
Regards
Heidi Eckhart
Software Engineer
Linux Technology Center - Open Hypervisor
heidieck(a)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
5:20 p.m.
Heidi Eckhart wrote:
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
Excellent! I like this change. =) And the set tested just fine. +1
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
6125
days inactive
6126
days old
7 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert