[Libvirt-cim] [PATCH] (#2) Workaround to fix race condition around libvirt init

[Eduardo Lima (Etrunko)]

On 08/30/2011 02:14 PM, Sharad Mishra wrote:
> # HG changeset patch
> # User Sharad Mishra<snmishra at us.ibm.com>
> # Date 1314719316 25200
> # Node ID c54aafd0b2c6414f91b96bc30e2f148bb78d5c59
> # Parent  277b56b3863b5f81a3faa18aeb7b9951b963b489
> (#2) Workaround to fix race condition around libvirt init.
>
> This patch fixes the race condition caused when mutiple
> threads try to start VMs at the same time. This patch
> also fixes the issue of incorrect mem allocation for
> VSSD property - emulator.
>
> #2: included Edurdo's patch for make_space
>      took care of coding guidelines.
>
> Signed-off-by: Sharad Mishra<snmishra at us.ibm.com>
>
> diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c
> --- a/libxkutil/misc_util.c
> +++ b/libxkutil/misc_util.c
> @@ -28,6 +28,7 @@
>   #include<stdbool.h>
>   #include<stdarg.h>
>   #include<unistd.h>
> +#include<pthread.h>
>   #include<libvirt/libvirt.h>
>   #include<libvirt/virterror.h>
>
> @@ -45,6 +46,9 @@
>   #include "misc_util.h"
>   #include "cs_util.h"
>
> +static pthread_mutex_t libvirt_mutex = PTHREAD_MUTEX_INITIALIZER;
> +/* libvirt library not initialized */
> +static int libvirt_initialized = 0;
>
>   #define URI_ENV "HYPURI"
>
> @@ -114,11 +118,15 @@
>
>           CU_DEBUG("Connecting to libvirt with uri `%s'", uri);
>
> +        pthread_mutex_lock(&libvirt_mutex);
> +
>           if (is_read_only())
>                   conn = virConnectOpenReadOnly(uri);
>           else
>                   conn = virConnectOpen(uri);
>
> +        pthread_mutex_unlock(&libvirt_mutex);
> +
>           if (!conn) {
>                   CU_DEBUG("Unable to connect to `%s'", uri);
>                   return NULL;
> @@ -530,7 +538,19 @@
>
>   bool libvirt_cim_init(void)
>   {
> -        return virInitialize() == 0;
> +        int ret = 0;
> +
> +        /* double-check lock pattern used for performance reasons */
> +        if (libvirt_initialized == 0) {
> +                pthread_mutex_lock(&libvirt_mutex);
> +                if (libvirt_initialized == 0) {
> +                        ret = virInitialize();
> +                        if (ret == 0)
> +                                libvirt_initialized = 1;
> +                }
> +                pthread_mutex_unlock(&libvirt_mutex);
> +        }
> +        return (ret == 0);
>   }
>
>   bool check_refs_pfx_match(const CMPIObjectPath *refa,
> diff --git a/src/Virt_VirtualSystemManagementService.c b/src/Virt_VirtualSystemManagementService.c
> --- a/src/Virt_VirtualSystemManagementService.c
> +++ b/src/Virt_VirtualSystemManagementService.c
> @@ -188,6 +188,24 @@
>           return 1;
>   }
>
> +static bool make_space(struct virt_device **list, int cur, int new)
> +{
> +        struct virt_device *tmp;
> +
> +        tmp = calloc(cur + new, sizeof(*tmp));
> +        if (tmp == NULL)
> +                return false;
> +
> +        if (*list) {
> +                memcpy(tmp, *list, sizeof(*tmp) * cur);
> +                free(*list);
> +        }
> +
> +        *list = tmp;
> +
> +        return true;
> +}
> +
>   static bool fv_set_emulator(struct domain *domain,
>                               const char *emu)
>   {
> @@ -198,6 +216,11 @@
>           if (emu == NULL)
>                   return true;
>
> +        if (!make_space(&domain->dev_emu, 0, 1)) {
> +                CU_DEBUG("Failed to alloc disk list");
> +                return false;
> +        }
> +
>           cleanup_virt_device(domain->dev_emu);
>
>           domain->dev_emu->type = CIM_RES_TYPE_EMU;
> @@ -1369,24 +1392,6 @@
>           return msg;
>   }
>
> -static bool make_space(struct virt_device **list, int cur, int new)
> -{
> -        struct virt_device *tmp;
> -
> -        tmp = calloc(cur + new, sizeof(*tmp));
> -        if (tmp == NULL)
> -                return false;
> -
> -        if (*list) {
> -                memcpy(tmp, *list, sizeof(*tmp) * cur);
> -                free(*list);
> -        }
> -
> -        *list = tmp;
> -
> -        return true;
> -}
> -
>   static char *add_device_nodup(struct virt_device *dev,
>                                 struct virt_device *list,
>                                 int max,
>

+1. Tricky stuff. :)

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