[Libvirt-cim] [PATCH 3/3] VSMS: Support for domains with controller devices

John Ferlan jferlan at redhat.com
Fri Mar 14 15:16:40 UTC 2014



On 03/14/2014 10:40 AM, Boris Fiuczynski wrote:
> On 03/14/2014 01:56 PM, John Ferlan wrote:
>> From: Xu Wang <gesaint at linux.vnet.ibm.com>
>>
>> Support being able to convert the Controller RASD into a virtual device
>> Signed-off-by: Xu Wang <gesaint at linux.vnet.ibm.com>
>>
>> Signed-off-by: John Ferlan <jferlan at redhat.com>
>> ---
>>   src/Virt_VirtualSystemManagementService.c | 76 +++++++++++++++++++++++++++++++
>>   1 file changed, 76 insertions(+)
>>
>> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
>> index 3e7785e..26de59d 100644
>> --- a/src/Virt_VirtualSystemManagementService.c
>> +++ b/src/Virt_VirtualSystemManagementService.c
>> @@ -1838,6 +1838,53 @@ static const char *input_rasd_to_vdev(CMPIInstance *inst,
>>           return NULL;
>>   }
>>
>> +static const char *controller_rasd_to_vdev(CMPIInstance *inst,
>> +                                           struct virt_device *dev)
>> +{
>> +        const char *val = NULL;
>> +        const char *msg = NULL;
>> +        int ret;
>> +
>> +        if (cu_get_str_prop(inst, "ResourceSubType", &val) != CMPI_RC_OK) {
>> +                msg = "ControllerRASD ResourceSubType field not valid";
>> +                goto out;
>> +        }
>> +        dev->dev.controller.type = strdup(val);
>> +
>> +        /* Required fields */
>> +        if (cu_get_u64_prop(inst, "Index",
>> +                            &dev->dev.controller.index) != CMPI_RC_OK) {
>> +                msg = "ControllerRASD Index field not valid";
>> +                goto out;
>> +        }
> As I wrote elsewhere this is going to prevent the libvirt-cim user from 
> using the libvirt auto assignment for the index.
> 

OK - I'll rework this..

>> +
>> +        /* Formulate our instance id from controller, controller type,
>> +         * and index value. This should be unique enough.
>> +         */
> 
> ...
> 
>> @@ -3029,6 +3102,7 @@ static CMPIStatus resource_del(struct domain *dominfo,
>>                   if (STREQ(dev->id, devid)) {
>>                           if ((type == CIM_RES_TYPE_GRAPHICS) ||
>>                               (type == CIM_RES_TYPE_CONSOLE)  ||
>> +                            (type == CIM_RES_TYPE_CONTROLLER) ||
> What is the reason for adding the controller here?
> 

That's original code that I haven't touched, thought about, or I guess
knew what to do with...  I guess I just hadn't got this far yet.  I'll
work through the remaining comments...

Xu if you have the time to provide some feedback it would be most welcome...

John

>>                               (type == CIM_RES_TYPE_INPUT))
>>                                   cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
>>                           else {
>> @@ -3111,6 +3185,7 @@ static CMPIStatus resource_add(struct domain *dominfo,
>>
> 
> Why is the controller instance not added to a running domain?
> Instead of restricting it in the if statement below all I think should 
> be done is this (before the if in the patch below.)
>          if (type == CIM_RES_TYPE_CONTROLLER &&
>              dev != NULL && dev->id == NULL) {
>                  cu_statusf(_BROKER, &s,
>                             CMPI_RC_ERR_FAILED,
>                             "Add resource failed: Index property is 
> required.");
>                  goto out;
>          }
> 
>>           if ((type == CIM_RES_TYPE_GRAPHICS) ||
>>               (type == CIM_RES_TYPE_INPUT) ||
>> +            (type == CIM_RES_TYPE_CONTROLLER) ||
>>               (type == CIM_RES_TYPE_CONSOLE)) {
>>                   (*count)++;
>>                   cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
>> @@ -3188,6 +3263,7 @@ static CMPIStatus resource_mod(struct domain *dominfo,
>>
>>                           if ((type == CIM_RES_TYPE_GRAPHICS) ||
>>                               (type == CIM_RES_TYPE_INPUT)    ||
>> +                            (type == CIM_RES_TYPE_CONTROLLER) ||
> What is the reason for adding the controller here?
> 
>>                               (type == CIM_RES_TYPE_CONSOLE))
>>                                   cu_statusf(_BROKER, &s, CMPI_RC_OK, "");
>>                           else {
>>
> 
> 




More information about the Libvirt-cim mailing list