[Libvirt-cim] [PATCH] [RFC] Enhance handling of association's references

[Dan Smith]

HE> # HG changeset patch
HE> # User Heidi Eckhart <heidieck at 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 at us.ibm.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 188 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20071126/f7d76ff7/attachment.sig>