[Libvirt-cim] [PATCH V3 09/11] CSI Discard libvirt event by default

[Wenchao Xia]

于 2013-3-12 1:22, John Ferlan 写道:
>
> Since I don't have the original email to reply-to, here is the link:
>
> https://www.redhat.com/archives/libvirt-cim/2012-December/msg00032.html
>
> Seems this was "pulled back" from some other commit - might be nice to
> reference the commit id where it was removed and now replaced - at least
> for historical purposes.
>
>
>   src/Virt_ComputerSystem.c
>
> In trigger_mod_indication()
>   * The "goto out" if/when "(ind == NULL)" will perhaps trigger a false
> positive this succeeded since "s" is initialized to CMPI_RC_OK.
>
   This is a problem, sorry haven't check it out since the code is
copied from old version.

>
>
>   src/Virt_ComputerSystemIndication.c
>
> In doms_to_xml(),
>
>   * the calloc() has no check for successful return of memory leading to
> possible NULL pointer deref of *dom_xml_list.
>
>
> In async_ind_native():
>   * The CU_DEBUG() message "...indication diliv..." should be
> "...indication delivery..."
>   * The CU_DEBUG("Unrecognized indication type"); should report what
> 'ind_type' is (btw: it's unrecognised)
>
> In lifecycle_thread_native():
>   * Seems inefficient to traverse lists so many times.  In the for loop
> running through prev_xml, it would seem that if 'dom_in_list()' returns
> 1, then we could call 'dom_changed()' and possibly 'async_ind_native()'
> if it has changed.
   The logic is from old version;s code, I must admit there are space to
optimize for performance.

>   * Since this routine keeps lifecycle_mutex for the duration - I believe
> it makes it even more important to separate it from 'dom_list' in your
> previous change...
>
   This is not needed, the CSI from libvirt-cim/libvirt can be activated
only one at one time. Actually this patch disabled previous libvirt CSI
event feature, with a macro it can be activated again when we found
the real problem below later.

> NOTE: the get_domain_list() call could be made better by getting all
> "Active and Defined" domains in one virConnectListAllDomains() call, but
> that's mostly unrelated to this code.  Getting the list of All domains
> at one time is "preferred" since you won't run into odd timing issues by
> making two calls.
>
> In raise_indication()
>   * The returns for the strdup()'s are not checked.
>
> John
>
   Please form a patch based on mine if you wish,  to fix these problems,
I am a bit too busy to rewrite the code....


> _______________________________________________
> Libvirt-cim mailing list
> Libvirt-cim at redhat.com
> https://www.redhat.com/mailman/listinfo/libvirt-cim
>


-- 
Best Regards

Wenchao Xia