[Libvirt-cim] [PATCH 2/2] ComputerSystemIndication: Support libvirt domain events

[Eduardo Lima (Etrunko)]

On 12/31/2011 04:42 PM, Chip Vincent wrote:
> Follow up from my previous comments...
> 
> I think the problem is related to the use of
> virEventRegisterDefaultImpl() in libvirt_cim_init(), which is called by
> all providers, not just indication providers. Given the number of times
> we call that function, perhaps we're overloading libvirt and/or
> triggering an underlying bug. When I either comment out that function, I
> see no crashes. I then moved it to be called once in
> ComputerSystemIndication->ActivateFilter() (via a static init variable),
> and I was able to avoid the crashes. Unfortunately, I'm not able to
> verify the domain events function via cimtest. FWIW, I see the same
> message w/o the patch so perhaps I need to do something else to make it
> work.
> 

This sounds weird, if you notice the virEventRegisterDefaultImpl() call
is protected both by a static variable in libxkutil/misc_util.c and a
mutex lock. I could also protect the call with a static variable to
avoid calling it everytime.

> ComputerSystemIndication - 01_created_indication.py: FAIL
> ERROR     - Waited too long for define indication
> ERROR     - Exception: Poll for indication Failed
> ERROR     - Waited too long for start indication
> ERROR     - Exception: Poll for indication Failed
> ERROR     - Waited too long for destroy indication
> ERROR     - Exception: Poll for indication Failed
> 
> Here's a quick peek at the changes I made for reference:
> 
> Remove virEventRegisterDefaultImpl() from init functon
> 
> diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c
> index 556fe1f..61893c3 100644
> --- a/libxkutil/misc_util.c
> +++ b/libxkutil/misc_util.c
> @@ -607,8 +607,6 @@ bool libvirt_cim_init(void)
>                          ret = virInitialize();
>                          if (ret == 0)
>                                  libvirt_initialized = 1;
> -
> -                        virEventRegisterDefaultImpl();
>                  }
>                  pthread_mutex_unlock(&libvirt_mutex);
>          }
> 
> Initialize entire csi_thread_data_t structures. Not sure if a single
> '{0}' sets *all* members to 0 and
> I wanted to be sure the active_filters member was set to 0 before
> incrementing/decrementing.

Yes, it does initialize everything. In fact, it is actually unnecessary
to initialize any of the global static variables, as it is defined in
the C language specification that they should be initialized
automatically with zero (if not explicitly initialized).

> Also added one time call to virEventRegisterDefaultImpl() to
> ActiveFilter().
> 
> diff --git a/src/Virt_ComputerSystemIndication.c
> b/src/Virt_ComputerSystemIndication.c
> index 0df73f3..1ea8520 100644
> --- a/src/Virt_ComputerSystemIndication.c
> +++ b/src/Virt_ComputerSystemIndication.c
> @@ -80,7 +80,10 @@ struct _csi_thread_data_t {
>  static const CMPIBroker *_BROKER;
>  static pthread_mutex_t lifecycle_mutex = PTHREAD_MUTEX_INITIALIZER;
>  static bool lifecycle_enabled = false;
> -static csi_thread_data_t csi_thread_data[CSI_NUM_PLATFORMS] = {{0},
> {0}, {0}};
> +static csi_thread_data_t csi_thread_data[CSI_NUM_PLATFORMS] =
> +        {{0, 0, 0, 0, 0},
> +         {0, 0, 0, 0, 0},
> +         {0, 0, 0, 0, 0}};
> 
>  /*
>   * Domain list manipulation
> @@ -698,6 +703,13 @@ static CMPIStatus ActivateFilter(CMPIIndicationMI* mi,
>          bool error = false;
>          csi_thread_data_t *thread = NULL;
> 
> +        static int _init = 0;
> +
> +        if(_init == 0) {
> +                _init = 1;
> +                virEventRegisterDefaultImpl();
> +        }
> +
>          CU_DEBUG("ActivateFilter for %s", CLASSNAME(op));
> 
>          pthread_mutex_lock(&lifecycle_mutex);
> 
> 

I will try this. Thanks

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