2:40 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
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
2:40 a.m.
New subject: [PATCH 1 of 3] RASD: Added helper function to check for the given resource type
# HG changeset patch
# User Heidi Eckhart <heidieck(a)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(a)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,
2:40 a.m.
New subject: [PATCH 2 of 3] SDC: returned instances are not of type specific subclass
# HG changeset patch
# User Heidi Eckhart <heidieck(a)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(a)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:
8:42 a.m.
New subject: [PATCH 2 of 3] SDC: returned instances are not of type specific subclass
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(a)us.ibm.com
1:07 p.m.
New subject: [PATCH 2 of 3] SDC: returned instances are not of type specific subclass
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(a)us.ibm.com
2:28 a.m.
New subject: [PATCH 2 of 3] SDC: returned instances are not of type specific subclass
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(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
2:40 a.m.
New subject: [PATCH 3 of 3] SDC: RASD to AllocationCapabilities should return NOT_SUPPORTED instead of OK
# HG changeset patch
# User Heidi Eckhart <heidieck(a)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(a)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()
6173
days inactive
6176
days old
6 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert