[Libvirt-cim] [PATCH] [RFC] Added HostedAccessPoint association (ComputerSystem <-> KVMRedirectionSAP)

[Kaitlin Rupert]

> Running the following query:
> wbemcli ain -ac KVM_HostedAccessPoint 'http://root:1mud2ar3@localhost:5988/root/virt:
> KVM_HostSystem.CreationClassName="KVM_HostSystem",Name="F10"'
> 
> Expecting this query to return the list KVMRedirectionSAPs associated with the host
> 
> Right now the query returns the following message:
>  wbemcli: Cim: (4) CIM_ERR_INVALID_PARAMETER: KVM_HostSystem.CreationClassName=
> "KVM_HostSystem",Name="F10"
> 

I'm not getting this error.. are you reconfiguring your tree to make 
sure the makefiles are updated?

> +static const CMPIBroker *_BROKER;
> +
> +static CMPIStatus rsap_to_host(const CMPIObjectPath *ref,
> +                               struct std_assoc_info *info,
> +                               struct inst_list *list)
> +{
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        CMPIInstance *instance = NULL;
> +
> +        if (!match_hypervisor_prefix(ref, info))
> +                goto out;
> +
> +        s = get_console_sap_by_ref(_BROKER, ref, &instance);
> +        if (s.rc != CMPI_RC_OK)
> +                goto out;
> +
> +        s = get_host(_BROKER, info->context, ref, &instance, false);
> +        if (s.rc == CMPI_RC_OK)
> +                inst_list_add(list, instance);
> +
> +out:

The style is to put a space before out.

> +        return s;
> +}
> +
> +
> +static CMPIStatus host_to_rsap(const CMPIObjectPath *ref,
> +                               struct std_assoc_info *info,
> +                               struct inst_list *list)
> +{
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        CMPIInstance *instance = NULL;
> +        CMPIObjectPath *vref = NULL;
> +
> +        if (!match_hypervisor_prefix(ref, info))
> +                goto out;
> +
> +        s = get_host(_BROKER, info->context, ref, &instance, true);
> +        if (s.rc != CMPI_RC_OK)
> +                goto out; 
> +
> +        vref = convert_sblim_hostsystem(_BROKER, ref, info);
> +        if (vref == NULL)
> +                goto out;
> +
> +        s = return_console_sap(vref, list);
> +
> +        //s = get_console_sap_by_ref(_BROKER, ref, &instance);
> +        if (s.rc != CMPI_RC_OK)
> +                goto out;
> +        //if (!CMIIsNullObject(instance))
> +        inst_list_add(list, instance);

You're adding the host instance to the list.  You don't want to return 
the host instance.. just the KVMRedirectionSAP instances.

> +         
> +out:
> +        return s;
> +}
> +
> +LIBVIRT_CIM_DEFAULT_MAKEREF()
> +
> +static char* antecedent[] = {
> +        "Xen_HostSystem",
> +        "KVM_HostSystem",
> +        "LXC_HostSystem",

You'll need to include Linux_ComputerSystem in this list.

> +        NULL
> +};
> +
> +static char* dependent[] = {
> +        "Xen_KVMRedirectionSAP",
> +        "KVM_KVMRedirectionSAP",
> +        "LXC_KVMRedirectionSAP",
> +        NULL
> +};
> +
> +static char* assoc_classname[] = {
> +        "Xen_HostedAccessPoint",
> +        "KVM_HostedAccessPoint",
> +        "LXC_HostedAccessPoint",
> +        NULL
> +};
> +
> +static struct std_assoc _rsap_to_host = {
> +        .source_class = (char **)&antecedent,
> +        .source_prop = "Antecedent", 
> +
> +        .target_class = (char **)&dependent,
> +        .target_prop = "Dependent", 
> +
> +        .assoc_class = (char **)&assoc_classname,
> +
> +        .handler = rsap_to_host,

This needs to be host_to_rsap  - your source class is antecedent, which 
is populated with <>_HostSystem classnames.

> +        .make_ref = make_ref
> +};
> +
> +static struct std_assoc _host_to_rsap = {
> +        .source_class = (char **)&dependent,
> +        .source_prop = "Dependent",
> +
> +        .target_class = (char **)&antecedent,
> +        .target_prop = "Antecedent",
> +        
> +        .assoc_class = (char **)&assoc_classname,
> +
> +        .handler = host_to_rsap,

This needs to be rsap_to_host.

> diff -r 5d8d418eef37 -r 6197321a3c5e src/Virt_KVMRedirectionSAP.c

This is a very long patch.  I'd break this up into smaller patches and 
send it as a patchset.

> --- a/src/Virt_KVMRedirectionSAP.c	Wed Nov 19 16:08:21 2008 -0200
> +++ b/src/Virt_KVMRedirectionSAP.c	Mon Nov 24 16:58:39 2008 -0200
> @@ -236,15 +236,36 @@
>          return true;
>  }
> 
> -static CMPIStatus return_console_sap(const CMPIObjectPath *ref,
> -                                     const CMPIResult *results,
> -                                     bool names_only)
> +static CMPIStatus enum_console_saps(const CMPIObjectPath *ref,
> +                                    const CMPIResult *results,
> +                                    bool names_only)
> +{
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        struct inst_list list;

You'll need to call inst_list_init() to initialize the list.

> +
> +        s = return_console_sap(ref, &list);
> +        if (s.rc != CMPI_RC_OK)
> +                goto out;
> +
> +        if (names_only)
> +                cu_return_instance_names(results, &list);
> +        else
> +                cu_return_instances(results, &list);
> +
> +out:

You'll need to call inst_list_free() here to free the list.

> +        return s;
> +}
> +
> +CMPIStatus return_console_sap(const CMPIObjectPath *ref,
> +                              struct inst_list *list)
> +                                     //const CMPIResult *results,
> +                                     //bool names_only)

Since you'll be calling this function from other providers, it needs to 
take a CMPIBroker param.  Instead of using the provider's _BROKER, it 
should be using the broker that is passed in.

>  {
>          CMPIStatus s = {CMPI_RC_OK, NULL};
>          virConnectPtr conn;
>          virDomainPtr *domain_list;
>          struct domain *dominfo = NULL;
> -        struct inst_list list;
> +        //struct inst_list list;
>          struct vnc_ports port_list;
>          int count;
>          int lport;
> @@ -255,7 +276,7 @@
>          if (conn == NULL)
>                  return s;
> 
> -        inst_list_init(&list);
> +        inst_list_init(list);

The list should already be initialized by the calling function.  No need 
to initialize it here.

-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com