[Libvirt-cim] [PATCH] Workaround to fix race condition around libvirt init

[Sharad Mishra]

Wayne Xia <xiawenc at linux.vnet.ibm.com> wrote on 08/29/2011 02:47:34 AM:

> Wayne Xia <xiawenc at linux.vnet.ibm.com>
> 08/29/11 02:47 AM
>
> To
>
> libvirt-cim at redhat.com, Sharad Mishra/Beaverton/IBM at IBMUS
>
> cc
>
> Subject
>
> Re: [Libvirt-cim] [PATCH] Workaround to fix race condition around libvirt
init


Thanks for the comment, Wayne.

>
> Is it possible that libvirt would invoke one VM twice?

that will be a bug. I have never seen that happen.

>
> also some comments below.
>
> 于 2011-8-27 1:32, Sharad Mishra 写道:
> > # HG changeset patch
> > # User Sharad Mishra<snmishra at us.ibm.com>
> > # Date 1314379933 25200
> > # Node ID a42b68361ed9cb4f7d6f15de5b58ccc68e88ef38
> > # Parent  a346baf140d64177a9dc1066677c307ee6518236
> > Workaround to fix race condition around libvirt init.
> >
> > This patch fixes the race condition caused when mutiple
> > threads try to start a VM. This patch also fixes the issue
> > of incorrect mem allocation for VSSD property - emulator.
> >
> > 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 (0 == libvirt_initialized) {
> > +                pthread_mutex_lock(&libvirt_mutex);
> > +                if (0 == libvirt_initialized) {
> > +                        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,20 @@
> >           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;
> > +
> maybe some lines should be added here like this:
>
> if (*list!=NULL) {
>       memcpy(tmp, *list, sizeof(*tmp) * cur);
>       free(*list);
>       *list = tmp;
> }
>

Eduardo already submitted a patch for this. I will redo this patch to
include Eduardo's fix.

> > +        memcpy(tmp, *list, sizeof(*tmp) * cur);
> > +        *list = tmp;
> > +
> > +        return true;
> > +}
> > +
> >   static bool fv_set_emulator(struct domain *domain,
> >                               const char *emu)
> >   {
> > @@ -198,6 +212,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,20 +1388,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;
> > -
> > -        memcpy(tmp, *list, sizeof(*tmp) * cur);
> > -        *list = tmp;
> > -
> > -        return true;
> > -}
> > -
> >   static char *add_device_nodup(struct virt_device *dev,
> >                                 struct virt_device *list,
> >                                 int max,
> >
> > _______________________________________________
> > Libvirt-cim mailing list
> > Libvirt-cim at redhat.com
> > https://www.redhat.com/mailman/listinfo/libvirt-cim
>
>
> --
> Best Regards
>
> Wayne Xia
> mail:xiawenc at linux.vnet.ibm.com
> tel:86-010-82450803
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvirt-cim/attachments/20110829/03ade6f5/attachment.htm>