4:04 a.m.
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1196161060 -3600
# Node ID a9d4ef4c783c788db2b8ebd9da71255e492a0950
# Parent 16ac621eb3b34d11caca00100901d27bede7a7c7
Enhance handling of input parameter of std_association logic
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 16ac621eb3b3 -r a9d4ef4c783c std_association.c
--- a/std_association.c Tue Nov 27 11:45:33 2007 +0100
+++ b/std_association.c Tue Nov 27 11:57:40 2007 +0100
@@ -67,14 +67,20 @@ static bool match_class(const CMPIBroker
char *comp_class;
int i;
+ if (test_class == NULL)
+ return true;
+
+ if (comp_class_list == NULL)
+ return true;
+
rop = CMNewObjectPath(broker, ns, test_class, NULL);
+ if (CMIsNullObject(rop))
+ return false;
for (i = 0; comp_class_list[i]; i++) {
comp_class = comp_class_list[i];
- if ((test_class == NULL) ||
- (comp_class == NULL) ||
- match_op(broker, rop, comp_class))
+ if (!CMClassPathIsA(broker, rop, comp_class, NULL))
return true;
}
@@ -137,18 +143,82 @@ out:
static struct std_assoc *
std_assoc_get_handler(const struct std_assoc_ctx *ctx,
+ struct std_assoc_info *info,
const CMPIObjectPath *ref)
{
struct std_assoc *ptr = NULL;
int i;
+ bool rc;
+
+ CU_DEBUG("Calling Provider: '%s'",
+ info->provider_name);
for (i = 0; ctx->handlers[i]; i++) {
ptr = ctx->handlers[i];
if (match_source_class(ctx->brkr, ref, ptr))
- return ptr;
- }
-
+ break;
+ }
+
+ if (ptr) {
+ if (info->assoc_class) {
+ CU_DEBUG("Check client's assocClass:
'%s'",
+ info->assoc_class);
+
+ rc = match_class(ctx->brkr,
+ NAMESPACE(ref),
+ info->assoc_class,
+ ptr->assoc_class);
+
+ if (!rc) {
+ CU_DEBUG("AssocClass not valid.");
+ goto out;
+ }
+ CU_DEBUG("AssocClass valid.");
+ }
+
+ if (info->result_class) {
+ CU_DEBUG("Check client's resultClass:
'%s'",
+ info->result_class);
+
+ rc = match_class(ctx->brkr,
+ NAMESPACE(ref),
+ info->result_class,
+ ptr->target_class);
+
+ if (!rc) {
+ CU_DEBUG("ResultClass not valid.");
+ goto out;
+ }
+ CU_DEBUG("ResultClass valid.");
+ }
+
+ if (info->role) {
+ CU_DEBUG("Check client's role: '%s'",
+ info->role);
+
+ if (!STREQC(info->role, ptr->source_prop)) {
+ CU_DEBUG("Role not valid.");
+ goto out;
+ }
+ CU_DEBUG("Role valid.");
+ }
+
+ if (info->result_role) {
+ CU_DEBUG("Check client's resultRole:
'%s'",
+ info->result_role);
+
+ if (!STREQC(info->result_role, ptr->target_prop)) {
+ CU_DEBUG("ResultRole not valid.");
+ goto out;
+ }
+ CU_DEBUG("ResultRole valid.");
+ }
+
+ return ptr;
+ }
+
+ out:
return NULL;
}
@@ -158,57 +228,21 @@ static CMPIStatus do_assoc(struct std_as
const CMPIObjectPath *ref,
bool names_only)
{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
struct inst_list list;
- CMPIStatus s;
struct std_assoc *handler;
- bool rc;
inst_list_init(&list);
CU_DEBUG("Getting handler...");
-
- handler = std_assoc_get_handler(ctx, ref);
+ handler = std_assoc_get_handler(ctx, info, ref);
if (handler == NULL) {
CU_DEBUG("No handler found.");
goto out;
}
-
- CU_DEBUG("OK.\n\tsource: '%s'\n\ttarget:
'%s'\n\tassoc_class: '%s'",
- handler->source_class,
- handler->target_class,
- handler->assoc_class);
- CU_DEBUG("Calling match_class: \n\tinfo->result_class:
'%s'",
- info->result_class);
-
- rc = match_class(ctx->brkr,
- NAMESPACE(ref),
- info->result_class,
- handler->target_class);
- if (!rc) {
- CU_DEBUG("Match_class failed.");
- cu_statusf(ctx->brkr, &s,
- CMPI_RC_ERR_FAILED,
- "Result class is not valid for this association");
- goto out;
- }
- CU_DEBUG("Match_class succeeded.");
-
- CU_DEBUG("Calling match_class: \n\tinfo->assoc_class:
'%s'",
- info->assoc_class);
- rc = match_class(ctx->brkr,
- NAMESPACE(ref),
- info->assoc_class,
- handler->assoc_class);
- if (!rc) {
- CU_DEBUG("Match_class failed.");
- cu_statusf(ctx->brkr, &s,
- CMPI_RC_ERR_FAILED,
- "Association class given is not valid for"
- "this association");
- goto out;
- }
- CU_DEBUG("Match_class succeeded, calling handler->handler...");
-
+ CU_DEBUG("Getting handler succeeded.");
+
+ CU_DEBUG("Calling handler->handler...");
s = handler->handler(ref, info, &list);
if (s.rc != CMPI_RC_OK) {
@@ -256,32 +290,22 @@ static CMPIStatus do_ref(struct std_asso
const CMPIObjectPath *ref,
bool names_only)
{
+ CMPIStatus s = {CMPI_RC_OK, NULL};
struct inst_list list;
- CMPIStatus s;
- int i;
struct std_assoc *handler;
- bool rc;
+ int i;
inst_list_init(&list);
- handler = std_assoc_get_handler(ctx, ref);
+ CU_DEBUG("Getting handler...");
+ handler = std_assoc_get_handler(ctx, info, ref);
if (handler == NULL) {
CU_DEBUG("No handler found.");
goto out;
}
-
- rc = match_class(ctx->brkr,
- NAMESPACE(ref),
- info->result_class,
- handler->assoc_class);
- if (!rc) {
- cu_statusf(ctx->brkr, &s,
- CMPI_RC_ERR_FAILED,
- "Result class is not valid for this association");
- goto out;
- }
-
-
+ CU_DEBUG("Getting handler succeeded.");
+
+ CU_DEBUG("Calling handler->handler...");
s = handler->handler(ref, info, &list);
if ((s.rc != CMPI_RC_OK) || (list.list == NULL))
goto out;
9:36 a.m.
HE> std_assoc_get_handler(const struct std_assoc_ctx *ctx,
HE> + struct std_assoc_info *info,
HE> const CMPIObjectPath *ref)
HE> {
HE> struct std_assoc *ptr = NULL;
HE> int i;
HE> + bool rc;
HE> +
HE> + CU_DEBUG("Calling Provider: '%s'",
HE> + info->provider_name);
HE> for (i = 0; ctx->handlers[i]; i++) {
HE> ptr = ctx->handlers[i];
HE> if (match_source_class(ctx->brkr, ref, ptr))
HE> - return ptr;
HE> - }
HE> -
HE> + break;
HE> + }
HE> +
HE> + if (ptr) {
This starts a large block of indented code that doesn't need to be.
Can we replace this with:
if (ptr == NULL)
goto out;
so that the rest of the body can just continue?
Otherwise, this seems fine, but I think it would be a good idea to get
a sign-off from Kaitlin as she is familiar with this stuff too.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
12:49 p.m.
Heidi Eckhart wrote:
for (i = 0; comp_class_list[i]; i++) {
comp_class = comp_class_list[i];
- if ((test_class == NULL) ||
- (comp_class == NULL) ||
- match_op(broker, rop, comp_class))
+ if (!CMClassPathIsA(broker, rop, comp_class, NULL))
return true;
}
I like the idea of this approach - I think it will help with scalability
in the future if we decide to add support for new VMs. I have a few
comments though. =)
I'm not sure about this change here. If you return true here, then
you're accepting just about anything as a possible result class. ECTP
for example, the following query accepts Xen_Processor as a valid result
class:
wbemcli ain -ac Xen_ElementConformsToProfile -arc Xen_Process
'http://root:elm3b41@localhost/root/interop:Xen_RegisteredProfile.InstanceID="CIM:DSP1042-SystemVirtualization-1.0.0"'
I tried modifying this line so that it is
+ if (CMClassPathIsA(broker, rop, comp_class, NULL))
However, the problem with this is that it only allows result class to be
one of the items in the list supplied by the handler. For example, ETCP
has:
+char* managed_element[] = {
+ "Xen_HostSystem",
+ "Xen_ComputerSystem",
+ "KVM_HostSystem",
+ "KVM_ComputerSystem",
+ NULL
+};
The result class in the following query would not be valid because it
doesn't appear in the list.
wbemcli ain -ac Xen_ElementConformsToProfile -arc CIM_ManagedElement
'http://root:elm3b41@localhost/root/interop:Xen_RegisteredProfile.InstanceID="CIM:DSP1042-SystemVirtualization-1.0.0"'
We can't just add CIM_ManagedElement to the list because this same list
is being used for the source class. The source class needs to be
specific. Thoughts? I'm not sure if I'm correct in this line of
thinking. =)
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
3:53 p.m.
Kaitlin Rupert wrote:
Heidi Eckhart wrote:
> for (i = 0; comp_class_list[i]; i++) {
> comp_class = comp_class_list[i];
>
> - if ((test_class == NULL) ||
> - (comp_class == NULL) ||
> - match_op(broker, rop, comp_class))
> + if (!CMClassPathIsA(broker, rop, comp_class, NULL))
> return true;
> }
>
I like the idea of this approach - I think it will help with
scalability in the future if we decide to add support for new VMs. I
have a few comments though. =)
I'm not sure about this change here. If you return true here, then
you're accepting just about anything as a possible result class. ECTP
for example, the following query accepts Xen_Processor as a valid
result class:
wbemcli ain -ac Xen_ElementConformsToProfile -arc Xen_Process
'http://root:elm3b41@localhost/root/interop:Xen_RegisteredProfile.InstanceID="CIM:DSP1042-SystemVirtualization-1.0.0"'
I tried modifying this line so that it is
+ if (CMClassPathIsA(broker, rop, comp_class, NULL))
A very good catch :) ! Fixed this in the new version. Thanks :) !
However, the problem with this is that it only allows result class to
be one of the items in the list supplied by the handler. For example,
ETCP has:
+char* managed_element[] = {
+ "Xen_HostSystem",
+ "Xen_ComputerSystem",
+ "KVM_HostSystem",
+ "KVM_ComputerSystem",
+ NULL
+};
The result class in the following query would not be valid because it
doesn't appear in the list.
wbemcli ain -ac Xen_ElementConformsToProfile -arc
CIM_ManagedElement
'http://root:elm3b41@localhost/root/interop:Xen_RegisteredProfile.InstanceID="CIM:DSP1042-SystemVirtualization-1.0.0"'
We can't just add CIM_ManagedElement to the list because this same
list is being used for the source class. The source class needs to be
specific. Thoughts? I'm not sure if I'm correct in this line of
thinking. =)
The problem was, that I swapped the classes - the next patch will
show
it correct:
+ rop = CMNewObjectPath(broker, ns, comp_class, NULL);
+
+ if (CMClassPathIsA(broker, rop, test_class, NULL))
return true;
The CMPI Spec tells the following about CMClassPathIsA:
Function to determine whether a CIM class is of "type" or any of
"type"
subclasses.
CMPIBoolean CMClassPathIsA (const CMPIBroker* mb, const CMPIObjectPath*
op, const char* type, CMPIStatus* rc)
So with the fixes your mentioned wbemain request works now. One thing
that also caused a problem was, that the classes Xen/KVM_ComputerSystem
and Xen/KVM_HostSystem haven't been registered to the interop namespace
(sure, why should they ... ;) ). This is now also fixed with the patch
for ECTP. But only the classes, not the provider are registered, because
the DMTF Profile Spec tells us, that no instances of them are available
in the interop namespace. It was my fault to choose the one
cross-namespace association we have for implementing this
functionality... ;).
--
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
8:45 p.m.
Heidi Eckhart wrote:
The problem was, that I swapped the classes - the next patch will
show
it correct:
+ rop = CMNewObjectPath(broker, ns, comp_class, NULL);
+
+ if (CMClassPathIsA(broker, rop, test_class, NULL))
return true;
The CMPI Spec tells the following about CMClassPathIsA:
Function to determine whether a CIM class is of "type" or any of
"type" subclasses.
CMPIBoolean CMClassPathIsA (const CMPIBroker* mb, const
CMPIObjectPath* op, const char* type, CMPIStatus* rc)
Thank for the clarification =) I've tripped myself up on the
CMClassPathIsA() a few times. The updated changes look great. And the
changes passed my quick unit tests. ;)
So with the fixes your mentioned wbemain request works now. One thing
that also caused a problem was, that the classes
Xen/KVM_ComputerSystem and Xen/KVM_HostSystem haven't been registered
to the interop namespace (sure, why should they ... ;) ). This is now
also fixed with the patch for ECTP. But only the classes, not the
provider are registered, because the DMTF Profile Spec tells us, that
no instances of them are available in the interop namespace. It was my
fault to choose the one cross-namespace association we have for
implementing this functionality... ;).
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
6233
days inactive
6235
days old
4 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert