5:13 a.m.
Please don't ask me what happened to the patch set before and why patch 3 was not
submitted to the list. So resending the complete patch set once again ... and hope.
This patch set fixes:
- enhance the std_assoc struct to support lists for source, target and association
classnames
- fixed a wrong error return for a NULL handler
- reworked check for input parameter: assocClass, resultClass, role, resultRole
5:13 a.m.
New subject: [PATCH 1 of 3] Enhance handling of association's references
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1196283440 -3600
# Node ID 85f916da818a0d078d21efbb09df724e9cbf31dc
# Parent 074c037c19ce46559f36a3fd06b943f5fe3c6009
Enhance handling of association's references
The source and target classnames of std_assoc are now lists,
containing all supported classnames. This approach frees
the provider from listing all possible combinations as
instances of std_assoc.
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 074c037c19ce -r 85f916da818a std_association.c
--- a/std_association.c Tue Nov 27 16:29:13 2007 -0500
+++ b/std_association.c Wed Nov 28 21:57:20 2007 +0100
@@ -61,18 +61,44 @@ static bool match_class(const CMPIBroker
static bool match_class(const CMPIBroker *broker,
const char *ns,
const char *test_class,
- const char *comp_class)
+ char **comp_class_list)
{
CMPIObjectPath *rop;
+ char *comp_class;
+ int i;
rop = CMNewObjectPath(broker, ns, test_class, NULL);
- if ((test_class == NULL) ||
- (comp_class == NULL) ||
- match_op(broker, rop, comp_class))
- return true;
- else
- 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))
+ return true;
+ }
+
+ return false;
+}
+
+static bool match_source_class(const CMPIBroker *broker,
+ const CMPIObjectPath *ref,
+ struct std_assoc *ptr)
+{
+ char *source_class;
+ int i;
+
+ for (i = 0; ptr->source_class[i]; i++) {
+ source_class = ptr->source_class[i];
+
+ if (CMClassPathIsA(broker,
+ ref,
+ source_class,
+ NULL))
+ return true;
+ }
+
+ return false;
}
static CMPIStatus filter_results(struct inst_list *list,
@@ -113,13 +139,13 @@ std_assoc_get_handler(const struct std_a
std_assoc_get_handler(const struct std_assoc_ctx *ctx,
const CMPIObjectPath *ref)
{
- struct std_assoc *ptr;
+ struct std_assoc *ptr = NULL;
int i;
for (i = 0; ctx->handlers[i]; i++) {
ptr = ctx->handlers[i];
- if (CMClassPathIsA(ctx->brkr, ref, ptr->source_class, NULL))
+ if (match_source_class(ctx->brkr, ref, ptr))
return ptr;
}
diff -r 074c037c19ce -r 85f916da818a std_association.h
--- a/std_association.h Tue Nov 27 16:29:13 2007 -0500
+++ b/std_association.h Wed Nov 28 21:57:20 2007 +0100
@@ -37,13 +37,13 @@ typedef CMPIInstance *(*make_ref_t)(cons
struct std_assoc *);
struct std_assoc {
- char *source_class;
+ char **source_class;
char *source_prop;
- char *target_class;
+ char **target_class;
char *target_prop;
- char *assoc_class;
+ char **assoc_class;
assoc_handler_t handler;
make_ref_t make_ref;
5:13 a.m.
New subject: [PATCH 2 of 3] Removed error messages that cause wrong error returns
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1196283441 -3600
# Node ID 53d512fbf05747b7bca5c189b0d6ab5e7e63f90b
# Parent 85f916da818a0d078d21efbb09df724e9cbf31dc
Removed error messages that cause wrong error returns
The case that no handler was found for a request caused an
error as return. This is wrong as the not-found handler
tells only, that the provider is not responsible for the
given request. In scenarios with a general request causing
the call of several association providers that return
valid instances, the former behavior causes a wrong
result, as the CIMOM would return the error and revoke all
valid results.
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 85f916da818a -r 53d512fbf057 std_association.c
--- a/std_association.c Wed Nov 28 21:57:20 2007 +0100
+++ b/std_association.c Wed Nov 28 21:57:21 2007 +0100
@@ -170,9 +170,6 @@ static CMPIStatus do_assoc(struct std_as
handler = std_assoc_get_handler(ctx, ref);
if (handler == NULL) {
CU_DEBUG("No handler found.");
- cu_statusf(ctx->brkr, &s,
- CMPI_RC_ERR_FAILED,
- "Unable to handle this association");
goto out;
}
@@ -269,9 +266,7 @@ static CMPIStatus do_ref(struct std_asso
handler = std_assoc_get_handler(ctx, ref);
if (handler == NULL) {
- cu_statusf(ctx->brkr, &s,
- CMPI_RC_ERR_FAILED,
- "Unable to handle this association");
+ CU_DEBUG("No handler found.");
goto out;
}
5:13 a.m.
New subject: [PATCH 3 of 3] Enhance handling of input parameter of std_association logic
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1196287649 -3600
# Node ID ae5641cb6977307772a38c2585be008fc72b59a1
# Parent 53d512fbf05747b7bca5c189b0d6ab5e7e63f90b
Enhance handling of input parameter of std_association logic
Signed-off-by: Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
diff -r 53d512fbf057 -r ae5641cb6977 std_association.c
--- a/std_association.c Wed Nov 28 21:57:21 2007 +0100
+++ b/std_association.c Wed Nov 28 23:07:29 2007 +0100
@@ -67,14 +67,17 @@ static bool match_class(const CMPIBroker
char *comp_class;
int i;
- rop = CMNewObjectPath(broker, ns, test_class, NULL);
-
+ if (test_class == NULL)
+ return true;
+
+ if (comp_class_list == NULL)
+ return true;
+
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))
+ rop = CMNewObjectPath(broker, ns, comp_class, NULL);
+
+ if (CMClassPathIsA(broker, rop, test_class, NULL))
return true;
}
@@ -137,18 +140,83 @@ 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)
+ goto out;
+
+ 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 +226,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 +288,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;
1:50 a.m.
New subject: [PATCH 3 of 3] Enhance handling of input parameter of std_association logic
Heidi Eckhart wrote:
Sorry for revisiting this patch. I failed to test references yesterday,
so I didn't catch this problem then.
static struct std_assoc *
std_assoc_get_handler(const struct std_assoc_ctx *ctx,
+ struct std_assoc_info *info,
const CMPIObjectPath *ref)
{
<snip>
+
+ 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.");
+ }
This works for associations. But for references, the result class is
the association class name. So, when you call std_assoc_get_handler()
from do_ref(), you end up checking the client's result class (which is
the association name) against the result class list instead of the
association list. Here's some sample debug:
std_association.c(152): Calling Provider:
'associationVirt_HostedDependencyProvider'
std_association.c(182): Check client's resultClass: 'Xen_HostedDependency'
std_association.c(78): provider accepts Xen_ComputerSystem resultclass
std_association.c(78): provider accepts KVM_ComputerSystem resultclass
std_association.c(190): ResultClass not valid.
std_association.c(302): No handler found.
You could add flag argument to std_assoc_get_handler() that indicates
whether the call is asking for references or an associators. Although,
I'm not sure that's the most elegant way.
Also, adding the debug messages that print which values the provider is
checking the result class / assoc class / etc might be useful - that is,
print out what classes the provider accepts. Someone else can chime in
as to whether this would be useful.
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
6:54 p.m.
New subject: [PATCH 3 of 3] Enhance handling of input parameter of std_association logic
Kaitlin Rupert wrote:
This works for associations. But for references, the result class is
the association class name. So, when you call std_assoc_get_handler()
from do_ref(), you end up checking the client's result class (which is
the association name) against the result class list instead of the
association list. Here's some sample debug:
std_association.c(152): Calling Provider:
'associationVirt_HostedDependencyProvider'
std_association.c(182): Check client's resultClass:
'Xen_HostedDependency'
std_association.c(78): provider accepts Xen_ComputerSystem resultclass
std_association.c(78): provider accepts KVM_ComputerSystem resultclass
std_association.c(190): ResultClass not valid.
std_association.c(302): No handler found.
You could add flag argument to std_assoc_get_handler() that indicates
whether the call is asking for references or an associators.
Although, I'm not sure that's the most elegant way.
Also, adding the debug messages that print which values the provider
is checking the result class / assoc class / etc might be useful -
that is, print out what classes the provider accepts. Someone else
can chime in as to whether this would be useful.
That's also a very good catch
:). You are great in indicating bugs in
the association logic :) !
Fortunately this can be fixed very easy - the assocClass and resultClass
values have only been swapped while setting std_assoc_info values in
stda_ReferenceNames() and stda_References(). In the case of reference
calls the client's resultClass value needs to become the value for
assocClass internally.
@@ -373,8 +397,8 @@ CMPIStatus stda_ReferenceNames(CMPIAssoc
const char *role)
{
struct std_assoc_info info = {
- NULL,
- resultClass,
+ resultClass ,
+ NULL,
role,
NULL,
NULL,
@@ -394,8 +418,8 @@ CMPIStatus stda_References(CMPIAssociati
const char **properties)
{
struct std_assoc_info info = {
- NULL,
resultClass,
+ NULL,
role,
NULL,
properties,
--
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
10:55 a.m.
Heidi Eckhart wrote:
Please don't ask me what happened to the patch set before and why
patch 3 was not submitted to the list. So resending the complete patch set once again ...
and hope.
This patch set fixes:
- enhance the std_assoc struct to support lists for source, target and association
classnames
- fixed a wrong error return for a NULL handler
- reworked check for input parameter: assocClass, resultClass, role, resultRole
The set looks good. I liked the added role, resultRole support. Also,
I liked how you moved the checking of assocClass, resultClass, role,
resultRole to std_assoc_get_handler() instead of do_assoc(). It seems
like a more applicable place since std_assoc_get_handler() is supposed
to determine whether the provider can handle the association, and if so,
which function will be handling the call. Nice work =) +1
One odd thing I noticed about this patch set is that I could only get
the ECTP association to build properly. It seems like the other
associations need to be updated like ECTP before they will build
properly. This is strange because I was able to build all of the
associations with the first iteration of the patchset without problems.
I didn't see anything obvious that would cause the change. It looks
like all the providers will need to be updated at the same time the
libcmpiutil pieces are checked in, which probably makes sense anyway. =)
--
Kaitlin Rupert
IBM Linux Technology Center
karupert(a)us.ibm.com
11:55 a.m.
KR> One odd thing I noticed about this patch set is that I could only get
KR> the ECTP association to build properly. It seems like the other
KR> associations need to be updated like ECTP before they will build
KR> properly.
Yeah, they'll all need a change for sure.
KR> This is strange because I was able to build all of the
KR> associations with the first iteration of the patchset without
KR> problems.
Maybe you didn't do a clean first? The libvirt-cim build system won't
notice changes in the libcmpiutil stuff in order to force a clean
rebuild.
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
6183
days inactive
6185
days old
7 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Kaitlin Rupert