[Libvirt-cim] [PATCH] [RFC]Added 2 end points for the ServiceAffectsElement association, both related to the LogicalDevice class

[Kaitlin Rupert]

> +
> +static CMPIStatus add_devices_to_list(const CMPIObjectPath *ref,
> +                                          const char *host,
> +                                          int type,
> +                                          struct inst_list *list)
> +{
> +        CMPIStatus s = {CMPI_RC_OK, NULL};
> +        struct inst_list temp_list;
> +        int i, iret;
> +
> +        inst_list_init(&temp_list);
> +        
> +        s = enum_devices(_BROKER, ref, host, type, &temp_list);
> +        
> +        if (s.rc != CMPI_RC_OK)
> +                goto out;
> +
> +        for (i = 0; i < temp_list.cur; i++) {
> +                iret = inst_list_add(list, temp_list.list[i]);
> +                if (!iret) {
> +                        cu_statusf(_BROKER, &s,
> +                                   CMPI_RC_ERR_FAILED,
> +                                   "Not enough space to add device to list");
> +                        goto out;
> +                }
> +        } 

Why use a temp list here?  enum_devices() calls inst_list_add(), so 
there's no need to duplicate the inst_list_add() code here.  Instead, 
pass the original inst_list to enum_devices() directly.

> +
> + out:
> +        inst_list_free(&temp_list);
> +        
> +        return s;
> +}
> 
>  static CMPIStatus service_to_cs(const CMPIObjectPath *ref,
>                                  struct std_assoc_info *info,
> @@ -41,15 +75,60 @@
>  {
>          CMPIStatus s = {CMPI_RC_OK, NULL};
>          CMPIInstance *instance = NULL;
> +        const char *host = NULL;
> +        int i, num_of_domains;

Most of the rest of the code has one variable declaration per line.
>
> +        
>          if (!match_hypervisor_prefix(ref, info))
> -                return s;
> +                goto out;
> 
>          s = get_console_rs(ref, &instance, _BROKER, info->context, true);
>          if (s.rc != CMPI_RC_OK)
> -                return s;
> +                goto out;
> 
>          s = enum_domains(_BROKER, ref, list);
> +        if (s.rc != CMPI_RC_OK)
> +                goto out;
> +       
> +        num_of_domains = list->cur;
> + 
> +        // For each domain, insert its video and pointer devices into
> +        // the list
> +        for (i = 0; i < num_of_domains; i++) {
> +                s.rc = cu_get_str_prop(list->list[i], "Name", &host);
> +                if (s.rc != CMPI_RC_OK) 
> +                        goto out;
> +
> +                s = add_devices_to_list(ref, host, CIM_RES_TYPE_INPUT, list);
> +                if (s.rc != CMPI_RC_OK)
> +                        goto out;
> +
> +                s = add_devices_to_list(ref, host, CIM_RES_TYPE_GRAPHICS, list);
> +                if (s.rc != CMPI_RC_OK)
> +                        goto out;
> +        }

You can call enum_domains() with a NULL domain param - it will handle 
the looping for you.

The way you have it hear means the code will execute faster.  I usually 
try to avoid duplicating code when possible, but in this case, the 
reduction of execution time is a benefit.

> +
> + out:
> +        return s;
> +}
> +
> +static CMPIStatus validate_cs_or_dev_ref(const CMPIContext *context,
> +                                         const CMPIObjectPath *ref)
> +{
> +         CMPIStatus s = {CMPI_RC_OK, NULL};

There's a spacing issue here - might be a tab instead of 8 spaces.

> +        CMPIInstance *inst = NULL;
> +        char* classname;
> +                                  
> +        classname = class_base_name(CLASSNAME(ref));
> +
> +        if (STREQC(classname, "ComputerSystem")) {
> +                s = get_domain_by_ref(_BROKER, ref, &inst);
> +        } else if ( (STREQC(classname, "PointingDevice"))  || 
> +                    (STREQC(classname, "DisplayController")) ) {

Most of the other code uses this style:

else if ((STREQC()...

No spaces between the two sets of parens.


Everything tested out fine - so looks good otherwise.

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