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

[Eduardo Lima (Etrunko)]

On 01/26/2012 06:01 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 v3:
>  - Fix NestedFilterList DeleteInstance
>  - Fix remove_filter_ref() in acl_parsing.c (intend to refactor in
>    future patch)
> 
> Changes from v2:
>  - Return the correct reference in NestedFilterList
> 
> Changes from v1:
>  - Fix leak and other comments
>  - Fix all cases virDomainGetXML()
>  - Fix NestedFilterList Create/Delete instance
> 
> Signed-off-by: Chip Vincent <cvincent at us.ibm.com>
> ---
>  libxkutil/acl_parsing.c             |    1 +
>  libxkutil/device_parsing.c          |    6 ++-
>  src/Virt_AppliedFilterList.c        |   97 +++++++++++++++++++----------------
>  src/Virt_ComputerSystem.c           |    3 +-
>  src/Virt_ComputerSystemIndication.c |    3 +-
>  src/Virt_FilterList.c               |    5 ++-
>  src/Virt_NestedFilterList.c         |   19 +++++--
>  src/Virt_VSMigrationService.c       |    3 +-
>  8 files changed, 82 insertions(+), 55 deletions(-)
> 
> diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c
> index 5b6d7bb..9c4b4b2 100644
> --- a/libxkutil/acl_parsing.c
> +++ b/libxkutil/acl_parsing.c
> @@ -652,6 +652,7 @@ int remove_filter_ref(struct acl_filter *filter, const char *name)
> 
>          /* TODO: called infrequently, but needs optimization */
>          old_refs = filter->refs;
> +        filter->ref_ct = 0;

I know it is a bit too late, since the code has already been pushed.
Taking a closer look now, I find this line avoids the crash we were
having in the deletion of the NestedFilterList instance, but in the end
it will cause filter->refs to leak.

I tried to understand the code and it looks too much complex for the
purpose of only removing a string of an array. Taking this into account,
I took the next step and ported the ACL code to use the linked list
implementation.

I have just updated the linked list patch series and sent v4 for review.
Please take a look. We should either use this new patch or fix this code
properly.

Best regards,

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