Re: [Libvirt-cim] [PATCH] Fix AllocationCapabilities to make EnumInstances and EnumInstanceNames work

JG> + for (i = 0; device_pool_names[i]; i++) { JG> + s = get_pool_by_type(broker, JG> + conn, JG> + device_pool_names[i], JG> + NAMESPACE(ref), JG> + &device_pool_list); JG> + if (s.rc != CMPI_RC_OK) { JG> + cu_statusf(broker, &s, JG> + CMPI_RC_ERR_FAILED, JG> + "Error fetching device pools"); JG> + goto out; JG> + } JG> + } We do this elsewhere too. Perhaps we need a get_all_pools() function in Virt_DevicePool? JG> + JG> + for (i = 0; i < device_pool_list.cur; i++) { JG> + inst = get_typed_instance(broker, JG> + "AllocationCapabilities", JG> + NAMESPACE(ref)); JG> + if (inst == NULL) { JG> + cu_statusf(broker, &s, JG> + CMPI_RC_ERR_FAILED, JG> + "Could not get alloc_cap instance"); JG> + goto out; JG> + } JG> + JG> + s = cu_copy_prop(broker, device_pool_list.list[i], inst, JG> + "InstanceID", NULL); This function looks great, but I think we're missing the patch that adds it to libcmpiutil :) JG> + if (s.rc != CMPI_RC_OK) { JG> + cu_statusf(broker, &s, JG> + CMPI_RC_ERR_FAILED, JG> + "Error copying InstanceID"); JG> + goto out; JG> + } JG> + JG> + s = cu_copy_prop(broker, device_pool_list.list[i], inst, JG> + "ResourceType", NULL); JG> + if (s.rc != CMPI_RC_OK) { JG> + cu_statusf(broker, &s, JG> + CMPI_RC_ERR_FAILED, JG> + "Error copying InstanceID"); JG> + goto out; JG> + } JG> + JG> + inst_list_add(&alloc_cap_list, inst); JG> + } To reduce the size of this loop, and to be in line with our other instance providers, I think it would be good to define a function that looks something like this: CMPIStatus get_ac_instance(CMPIInstance *pool, CMPIInstance **ac); This would encapsulate things a bit and also would provide a clean hook for ElementCapabilities, which will need to, given a pool, get an instance of AllocationCapabilities. -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms(a)us.ibm.com