5:04 a.m.
# HG changeset patch
# User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
# Date 1195819371 -3600
# Node ID db20c6206fb6decb484035bec81d7c7f2be75eae
# Parent bf54de6af2e210bef57d74cf12e4872f6ba2da4f
[RFC] 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 bf54de6af2e2 -r db20c6206fb6 std_association.c
--- a/std_association.c Tue Nov 20 16:24:27 2007 -0800
+++ b/std_association.c Fri Nov 23 13:02:51 2007 +0100
@@ -61,18 +61,24 @@ 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 CMPIStatus filter_results(struct inst_list *list,
@@ -113,15 +119,30 @@ 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;
- int i;
+ struct std_assoc *hdl = NULL;
+ char *source_class;
+ int i, j;
for (i = 0; ctx->handlers[i]; i++) {
- ptr = ctx->handlers[i];
-
- if (CMClassPathIsA(ctx->brkr, ref, ptr->source_class, NULL))
- return ptr;
- }
+ hdl = ctx->handlers[i];
+
+ for (j = 0; hdl->source_class[j]; j++) {
+ source_class = hdl->source_class[j];
+
+ if (CMClassPathIsA(ctx->brkr,
+ ref,
+ source_class,
+ NULL))
+ break;
+
+ source_class = NULL;
+ }
+ if (source_class)
+ break;
+ }
+
+ if (hdl)
+ return hdl;
return NULL;
}
@@ -144,9 +165,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;
}
@@ -243,9 +261,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;
}
diff -r bf54de6af2e2 -r db20c6206fb6 std_association.h
--- a/std_association.h Tue Nov 20 16:24:27 2007 -0800
+++ b/std_association.h Fri Nov 23 13:02:51 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;
10:29 a.m.
HE> # HG changeset patch
HE> # User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
HE> # Date 1195819371 -3600
HE> # Node ID db20c6206fb6decb484035bec81d7c7f2be75eae
HE> # Parent bf54de6af2e210bef57d74cf12e4872f6ba2da4f
HE> [RFC] Enhance handling of association's references
HE> The source and target classnames of std_assoc are now lists,
HE> containing all supported classnames. This approach frees the
HE> provider from listing all possible combinations as instances of
HE> std_assoc.
I'm tentatively okay with this approach. We need to make a decision
and go with it so that we can freeze (at least to some extent) the
libcmpiutil API and make an official code release.
HE> @@ -113,15 +119,30 @@ std_assoc_get_handler(const struct std_a
HE> std_assoc_get_handler(const struct std_assoc_ctx *ctx,
HE> const CMPIObjectPath *ref)
HE> {
HE> - struct std_assoc *ptr;
HE> - int i;
HE> + struct std_assoc *hdl = NULL;
HE> + char *source_class;
HE> + int i, j;
Please don't arbitrarily rename and reorder variables like this. It
makes it harder to read the diff and is really unnecessary.
HE> for (i = 0; ctx->handlers[i]; i++) {
HE> - ptr = ctx->handlers[i];
HE> -
HE> - if (CMClassPathIsA(ctx->brkr, ref, ptr->source_class,
NULL))
HE> - return ptr;
HE> - }
HE> + hdl = ctx->handlers[i];
HE> +
HE> + for (j = 0; hdl->source_class[j]; j++) {
HE> + source_class = hdl->source_class[j];
HE> +
HE> + if (CMClassPathIsA(ctx->brkr,
HE> + ref,
HE> + source_class,
HE> + NULL))
HE> + break;
HE> +
HE> + source_class = NULL;
HE> + }
HE> + if (source_class)
HE> + break;
HE> + }
HE> +
HE> + if (hdl)
HE> + return hdl;
This double-nested loop really needs to have the inner loop broken out
as another function (like match_class()).
HE> return NULL;
HE> }
HE> @@ -144,9 +165,6 @@ static CMPIStatus do_assoc(struct std_as
HE> handler = std_assoc_get_handler(ctx, ref);
HE> if (handler == NULL) {
HE> CU_DEBUG("No handler found.");
HE> - cu_statusf(ctx->brkr, &s,
HE> - CMPI_RC_ERR_FAILED,
HE> - "Unable to handle this association");
HE> goto out;
HE> }
HE> @@ -243,9 +261,7 @@ static CMPIStatus do_ref(struct std_asso
HE> handler = std_assoc_get_handler(ctx, ref);
HE> if (handler == NULL) {
HE> - cu_statusf(ctx->brkr, &s,
HE> - CMPI_RC_ERR_FAILED,
HE> - "Unable to handle this association");
HE> + CU_DEBUG("No handler found.");
HE> goto out;
HE> }
Why should we not return error in these cases now? Currently, these
signal the case of someone trying to do something like:
wbemcli ain -ac Xen_SystemDevice http://...:Xen_MemoryPool
(i.e. resolving a particular association with an invalid reference)
--
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: danms(a)us.ibm.com
4:27 a.m.
Dan Smith wrote:
HE> return NULL;
HE> }
HE> @@ -144,9 +165,6 @@ static CMPIStatus do_assoc(struct std_as
HE> handler = std_assoc_get_handler(ctx, ref);
HE> if (handler == NULL) {
HE> CU_DEBUG("No handler found.");
HE> - cu_statusf(ctx->brkr, &s,
HE> - CMPI_RC_ERR_FAILED,
HE> - "Unable to handle this association");
HE> goto out;
HE> }
HE> @@ -243,9 +261,7 @@ static CMPIStatus do_ref(struct std_asso
HE> handler = std_assoc_get_handler(ctx, ref);
HE> if (handler == NULL) {
HE> - cu_statusf(ctx->brkr, &s,
HE> - CMPI_RC_ERR_FAILED,
HE> - "Unable to handle this association");
HE> + CU_DEBUG("No handler found.");
HE> goto out;
HE> }
Why should we not return error in these cases now? Currently, these
signal the case of someone trying to do something like:
wbemcli ain -ac Xen_SystemDevice http://...:Xen_MemoryPool
(i.e. resolving a particular association with an invalid reference)
Because this is
only a request the provider is not responsible for, but
not an error case. There exists the possibility that another provider is
called by the same request and this on is responsible for this request,
returning valid instances that are revoked, because our provider only
wants to tell "I'm not responsible for this request". The behavior of an
association provider in the case he figures out that he is not
responsible for a certain request is to be "silent". On the other hand -
avoiding such invalid requests is the responsibility of the client.
--
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
6:46 a.m.
Dan Smith wrote:
HE> # HG changeset patch
HE> # User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
HE> # Date 1195819371 -3600
HE> # Node ID db20c6206fb6decb484035bec81d7c7f2be75eae
HE> # Parent bf54de6af2e210bef57d74cf12e4872f6ba2da4f
HE> [RFC] Enhance handling of association's references
HE> The source and target classnames of std_assoc are now lists,
HE> containing all supported classnames. This approach frees the
HE> provider from listing all possible combinations as instances of
HE> std_assoc.
I'm tentatively okay with this approach. We need to make a decision
and go with it so that we can freeze (at least to some extent) the
libcmpiutil API and make an official code release.
If this approach is ok for all,
then I can cook up a patch for each
association.
--
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:28 a.m.
Heidi Eckhart wrote:
Dan Smith wrote:
> HE> # HG changeset patch
> HE> # User Heidi Eckhart <heidieck(a)linux.vnet.ibm.com>
> HE> # Date 1195819371 -3600
> HE> # Node ID db20c6206fb6decb484035bec81d7c7f2be75eae
> HE> # Parent bf54de6af2e210bef57d74cf12e4872f6ba2da4f
> HE> [RFC] Enhance handling of association's references
>
> HE> The source and target classnames of std_assoc are now lists,
> HE> containing all supported classnames. This approach frees the
> HE> provider from listing all possible combinations as instances of
> HE> std_assoc.
>
> I'm tentatively okay with this approach. We need to make a decision
> and go with it so that we can freeze (at least to some extent) the
> libcmpiutil API and make an official code release.
If this approach is ok for all, then I can cook up a patch for each
association.
I'm also on the tentatively okay side. It looks sound, and should work,
but I always get a little tentative when we deal with the "who handles
what" type questions, as I'm a bit over my head on those most of the
time. I'd say that if nobody sees anything that looks outright wrong we
should go with it, as you seem to have a better understanding of how
that sort of thing should go than we do.
--
-Jay
6234
days inactive
6238
days old
4 comments
3 participants
participants (3)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon