[Libvirt-cim] [PATCH] ProcessorRASD, the class that won't go away

Kaitlin Rupert kaitlin at linux.vnet.ibm.com
Wed May 14 16:47:19 UTC 2008


> +static bool vcpu_instances(const CMPIBroker *broker,
> +                           const virDomainPtr dom,
> +                           const char *ns,
> +                           uint64_t proc_count,
> +                           struct inst_list *list)
> +{
> +        int i;
> +        char *dev_id;
> +        CMPIInstance *inst;
> +        virConnectPtr conn;
> +
> +        for (i = 0; i < proc_count; i++) {
> +                conn = virDomainGetConnect(dom);
> +                inst = get_typed_instance(broker,
> +                                          pfx_from_conn(conn),
> +                                          "Processor",
> +                                          ns);
> +                if (inst == NULL)
> +                        return false;
> +
> +                dev_id = get_vcpu_inst_id(dom, i);
> +                CMSetProperty(inst, "DeviceID",
> +                              (CMPIValue *)dev_id, CMPI_chars);
> +                free(dev_id);
> +                
> +                device_set_systemname(inst, dom);
> +                inst_list_add(list, inst);
> +        }

Need to close the virConnectPtr here.

> +
> +        return true;
> +}
> +
> +static bool device_instances(const CMPIBroker *broker,
> +                             struct virt_device *devs,
> +                             int count,
> +                             const virDomainPtr dom,
> +                             const char *ns,
> +                             struct inst_list *list)
> +{
> +        int i;
> +        bool ret;
> +        uint64_t proc_count = 0;
> +        bool proc_found = false;
> +        CMPIInstance *instance = NULL;
> +
> +        for (i = 0; i < count; i++) {
> +                struct virt_device *dev = &devs[i];
> +
> +                if (dev->type == CIM_RES_TYPE_NET)
> +                        instance = net_instance(broker,
> +                                                &dev->dev.net,
> +                                                dom,
> +                                                ns);
> +                else if (dev->type == CIM_RES_TYPE_DISK)
> +                        instance = disk_instance(broker,
> +                                                 &dev->dev.disk,
> +                                                 dom,
> +                                                 ns);
> +                else if (dev->type == CIM_RES_TYPE_MEM)
> +                        instance = mem_instance(broker,
> +                                                &dev->dev.mem,
> +                                                dom,
> +                                                ns);
> +                else if (dev->type == CIM_RES_TYPE_PROC) {
> +                        proc_found = true;


proc_found isn't used anywhere else

> +                        proc_count = dev->dev.vcpu.quantity;
> +                        continue;
> +                } else
> +                        return false;
> +
> +                if (!instance)
> +                        return false;
> +                
> +                device_set_devid(instance, dev, dom);
> +                device_set_systemname(instance, dom);
> +                inst_list_add(list, instance);
> +        }
> +
> +        if (proc_count) {
> +                ret = vcpu_instances(broker,
> +                                     dom,
> +                                     ns,
> +                                     proc_count,
> +                                     list);
> +        }
> +
> +        return true;
>  }
> 
>  uint16_t res_type_from_device_classname(const char *classname)
> @@ -290,25 +344,27 @@ static CMPIStatus _get_devices(const CMP
>  {
>          CMPIStatus s = {CMPI_RC_OK, NULL};
>          int count;
> -        int i;
> +        bool rc;
>          struct virt_device *devs = NULL;
> 
>          count = get_devices(dom, &devs, type);
>          if (count <= 0)
>                  goto out;
> 
> -        for (i = 0; i < count; i++) {
> -                CMPIInstance *dev = NULL;
> -
> -                dev = device_instance(broker,
> -                                      &devs[i],
> -                                      dom,
> -                                      NAMESPACE(reference));
> -                if (dev)
> -                        inst_list_add(list, dev);
> -
> -                cleanup_virt_device(&devs[i]);
> -        }
> +        rc = device_instances(broker, 
> +                              devs, 
> +                              count,
> +                              dom, 
> +                              NAMESPACE(reference),
> +                              list);
> +
> +        if (!rc) {
> +                CU_DEBUG("Problem getting device instances");
> +                cu_statusf(broker, &s,
> +                           CMPI_RC_ERR_FAILED,
> +                           "Couldn't get device instances");

This block is missing a closing brace.


> @@ -462,10 +552,13 @@ CMPIStatus get_device_by_name(const CMPI
>          CMPIStatus s = {CMPI_RC_OK, NULL};
>          char *domain = NULL;
>          char *device = NULL;
> -        CMPIInstance *instance = NULL;
>          virConnectPtr conn = NULL;
>          virDomainPtr dom = NULL;
>          struct virt_device *dev = NULL;
> +        struct inst_list tmp_list;
> +        bool rc;
> +
> +        inst_list_init(&tmp_list);
> 
>          conn = connect_by_classname(broker, CLASSNAME(reference), &s);
>          if (conn == NULL) {
> @@ -501,13 +594,27 @@ CMPIStatus get_device_by_name(const CMPI
>                  goto err;
>          }
> 
> -        instance = device_instance(broker, 
> -                                   dev, 
> -                                   dom, 
> -                                   NAMESPACE(reference));
> +        if (type == CIM_RES_TYPE_PROC) {
> +                CU_DEBUG("PROC");

This statement should be removed or should be more descriptive.

> +                rc = vcpu_instances(broker,
> +                                    dom,
> +                                    NAMESPACE(reference),
> +                                    1,
> +                                    &tmp_list);

I think you need a different function here.  If you have a guest with 
more than one proc, a query with the following with fail:

  wbemcli gi 
'http://localhost/root/virt:KVM_Processor.CreationClassName="KVM_Processor",SystemName="demo2",DeviceID="demo2/1",SystemCreationClassName="KVM_ComputerSystem"'

This is because you are telling vcpu_instances() there is only 1 device, 
and so vcpu_instances() assigns the instance a DeviceID of 
<guest_name>/0 instead of <guest_name>/1.

> +        } else {
> +                
> +                rc = device_instances(broker, 
> +                                      dev, 
> +                                      1,
> +                                      dom, 
> +                                      NAMESPACE(reference),
> +                                      &tmp_list);
> +        }
> +
>          cleanup_virt_device(dev);
> 
> -        *_inst = instance;
> +        *_inst = tmp_list.list[0];
> +        CU_DEBUG("cleaning up");

Same here - this statement can be removed.

Also, the following queries fail for me - not sure why:

wbemcli ain -ac KVM_SettingsDefineState 
'http://localhost/root/virt:KVM_Processor.CreationClassName="KVM_Processor",SystemName="demo2",DeviceID="demo2/0",SystemCreationClassName="KVM_ComputerSystem"'

wbemcli ain -ac KVM_SettingsDefineState 
'http://localhost/root/virt:KVM_ProcResourceAllocationSettingData.InstanceID="demo2/proc"'

Here's the values I get back from the enumerate calls:

wbemcli ein http://localhost/root/virt:KVM_ProcResourceAllocationSettingData
localhost:5988/root/virt:KVM_ProcResourceAllocationSettingData.InstanceID="demo2/proc"

  wbemcli ein http://localhost/root/virt:KVM_Processor
localhost:5988/root/virt:KVM_Processor.CreationClassName="KVM_Processor",DeviceID="demo2/0",SystemCreationClassName="KVM_ComputerSystem",SystemName="demo2"
localhost:5988/root/virt:KVM_Processor.CreationClassName="KVM_Processor",DeviceID="demo2/1",SystemCreationClassName="KVM_ComputerSystem",SystemName="demo2"
localhost:5988/root/virt:KVM_Processor.CreationClassName="KVM_Processor",DeviceID="demo2/2",SystemCreationClassName="KVM_ComputerSystem",SystemName="demo2"
localhost:5988/root/virt:KVM_Processor.CreationClassName="KVM_Processor",DeviceID="demo2/3",SystemCreationClassName="KVM_ComputerSystem",SystemName="demo2"



Don't forget the xml_parse.c related change.
-- 
Kaitlin Rupert
IBM Linux Technology Center
kaitlin at linux.vnet.ibm.com




More information about the Libvirt-cim mailing list