Re: [Libvirt-cim] [PATCH V3 09/11] CSI Discard libvirt event by default
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. 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. * 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... 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
δΊ 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@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-cim
-- Best Regards Wenchao Xia
participants (2)
-
John Ferlan -
Wenchao Xia