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

On 01/26/2012 12:58 PM, Chip Vincent wrote: > From: Chip Vincent<cvincent(a)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.