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

[Eduardo Lima (Etrunko)]

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?

Another question, is it necessary to free the _reference variable
somehow? Looking at Pegasus headers, I can see a CMRelease declaration
near CMClone.

-- 
Eduardo de Barros Lima
Software Engineer, Open Virtualization
Linux Technology Center - IBM/Brazil
eblima at br.ibm.com