[Libvirt-cim] [PATCH v2] Fix AppliedFilterList creation and deletion

[Chip Vincent]

On 01/26/2012 11:08 AM, Eduardo Lima (Etrunko) wrote:
> On 01/26/2012 12:58 PM, Chip Vincent wrote:
>> From: Chip Vincent<cvincent at us.ibm.com>
>>
>> Fixes many small issues with the current AppliedFilterList provider.
>>
>> 1) Fix Create to properly return a complete object path and fix Delete to
>>     properly parse that path.
>>
>> 2) Persist applied filer rules. Since it's not possible to dyanmically update
>>     a single device, I've changed the code to modify and re-define the VM to
>>     essentially add/remove ACL filter associations.
>>
>>     I also updated the code to minimize domain/device parsing overhead. For
>>     some strange reason, our internal APIs sometimes take a virDomainPtr and
>>     other times a struct domain * forcing providers who work with domains
>>     *and* devices to parse everything twice. Until the internal APIs are
>>     cleaned up, I simply parse everything once and then fetch the device
>>     manually from the struct domain *.
>>
>> 3) Add VIR_DOMAIN_XML_INACTIVE to virDomainGetXML(). By default, libvirt only
>>     returns the XML of the running domain. We need to fetch the *stored* XML
>>     that will be used for the next boot so that all changes made while the VM
>>     is running are preserved.
>>
>> Changes from v1:
>>   - Fix leak and other comments
>>   - Fix all cases virDomainGetXML()
>>   - Fix NestedFilterList Create/Delete instance
> Almost there, see below:
>
> [snip]
>
>> diff --git a/src/Virt_NestedFilterList.c b/src/Virt_NestedFilterList.c
>> index 894cd7c..b72c582 100644
>> --- a/src/Virt_NestedFilterList.c
>> +++ b/src/Virt_NestedFilterList.c
>> @@ -98,7 +98,10 @@ static CMPIrc cu_get_ref_path(const CMPIObjectPath *reference,
>>           if ((s.rc != CMPI_RC_OK) || CMIsNullValue(value))
>>                   return CMPI_RC_ERR_NO_SUCH_PROPERTY;
>>
>> -        /* how to parse and object path? */
>> +        if ((value.type != CMPI_ref) ||  CMIsNullObject(value.value.ref))
>> +                return CMPI_RC_ERR_TYPE_MISMATCH;
>> +
>> +        *_reference = value.value.ref;
>>
>>           return CMPI_RC_OK;
>>   }
>> @@ -305,6 +308,7 @@ static CMPIStatus CreateInstance(
>>           const char *child_name = NULL;
>>           struct acl_filter *child_filter = NULL;
>>           virConnectPtr conn = NULL;
>> +        CMPIObjectPath *_reference = NULL;
>>
>>           CU_DEBUG("Reference = %s", REF2STR(reference));
>>
>> @@ -383,6 +387,11 @@ static CMPIStatus CreateInstance(
>>                   goto out;
>>           }
>>
>> +        /* create new object path */
>> +        _reference = CMClone(reference, NULL);
>> +        CMAddKey(_reference, "Antecedent", (CMPIValue *)&antecedent, CMPI_ref);
>> +        CMAddKey(_reference, "Dependent", (CMPIValue *)&dependent, CMPI_ref);
>> +
>>           CMReturnObjectPath(results, reference);
> Should be returning _reference right?
Good catch! Will fix
> Another question, is it necessary to free the _reference variable
> somehow? Looking at Pegasus headers, I can see a CMRelease declaration
> near CMClone.
>
No. All CMPI objects are released once the request thread is completed. 
CMRelease() is exposed for symmetry and also for long running threads, like jobs 
or indication providers.

-- 
Chip Vincent
Open Virtualization
IBM Linux Technology Center
cvincent at linux.vnet.ibm.com