[PATCH] [RFC] Enhance handling of association's references
# HG changeset patch # User Heidi Eckhart <heidieck@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@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;
HE> # HG changeset patch HE> # User Heidi Eckhart <heidieck@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@us.ibm.com
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@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
Dan Smith wrote:
HE> # HG changeset patch HE> # User Heidi Eckhart <heidieck@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@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
Heidi Eckhart wrote:
Dan Smith wrote:
HE> # HG changeset patch HE> # User Heidi Eckhart <heidieck@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
participants (3)
-
Dan Smith -
Heidi Eckhart -
Jay Gagnon