[libvirt] [PATCH] virBitmapFree: Change the function to a macro

[Liuji (Jeremy)]

> -----Original Message-----
> From: Daniel P. Berrange [mailto:berrange at redhat.com]
> Sent: Tuesday, September 10, 2013 3:57 PM
> To: Liuji (Jeremy)
> Cc: libvir-list at redhat.com; Jinbo (Justin); Luohao (brian); Haofeng
> Subject: Re: [libvirt] [PATCH] virBitmapFree: Change the function to a macro
> 
> On Tue, Sep 10, 2013 at 01:29:40AM +0000, Liuji (Jeremy) wrote:
> > > -----Original Message-----
> > > From: Daniel P. Berrange [mailto:berrange at redhat.com]
> > > Sent: Monday, September 09, 2013 7:26 PM
> > > To: Liuji (Jeremy)
> > > Cc: libvir-list at redhat.com; Jinbo (Justin); Luohao (brian); Haofeng
> > > Subject: Re: [libvirt] [PATCH] virBitmapFree: Change the function to a macro
> > >
> > > On Mon, Sep 09, 2013 at 01:39:42AM +0000, Liuji (Jeremy) wrote:
> > > > > -----Original Message-----
> > > > > From: Daniel P. Berrange [mailto:berrange at redhat.com]
> > > > > Sent: Friday, September 06, 2013 6:37 PM
> > > > > To: Liuji (Jeremy)
> > > > > Cc: libvir-list at redhat.com; Jinbo (Justin); Luohao (brian); Haofeng
> > > > > Subject: Re: [libvirt] [PATCH] virBitmapFree: Change the function to a
> macro
> > > > >
> > > > > On Fri, Sep 06, 2013 at 10:30:56AM +0000, Liuji (Jeremy) wrote:
> > > > > > The parameter of virBitmapFree function is just a pointer, not a
> pointer of
> > > > > pointer.
> > > > > > The second VIR_FREE on virBitmapFree only assign NULL to the formal
> > > > > parameter.
> > > > > > After calling the virBitmapFree function, the actual parameter are still
> not
> > > > > NULL.
> > > > > > There are many code segment don't assign NULL to the formal
> parameter
> > > > > after calling
> > > > > > the virBitmapFree function. This will bring potential risks.
> > > > > >
> > > > > > A problem scenario:
> > > > > > 1) The XML of VM contain the below segment:
> > > > > >     <numatune>
> > > > > >         <memory mode='preferred' placement='auto' nodeset='0'/>
> > > > > >     </numatune>
> > > > > > 2)virsh create the VM
> > > > > > 3)In the virDomainDefParseXML funtion:
> > > > > >                     /* Ignore 'nodeset' if 'placement' is 'auto'
> finally
> > > */
> > > > > >                     if (placement_mode ==
> > > > > VIR_NUMA_TUNE_MEM_PLACEMENT_MODE_AUTO) {
> > > > > >
> > > > > virBitmapFree(def->numatune.memory.nodemask);
> > > > > >                         def->numatune.memory.nodemask =
> NULL;
> > > > > >                     }
> > > > > > 4)Then, virsh destroy the VM. In the virDomainDefFree funtion, it also
> call
> > > the
> > > > > > virBitmapFree function to free the nodemask:
> > > > > >     virBitmapFree(def->numatune.memory.nodemask);
> > > > > > But after this call, the value of def->numatune.memory.nodemask is
> still
> > > not
> > > > > NULL.
> > > > > > This will generate an exception.
> > > > >
> > > > > Have you got an actual crash happening today, or is this just a
> theoretical
> > > > > problem you're trying to address ?
> > > >
> > > > Yes，it's an actual crash problem. I found the problem in the above
> problem
> > > scenario in my first mail.
> > >
> > > Then can you send a patch which explicitly fixes that problem on its
> > > own, without doing this major refactoring, which obscures what is being
> > > fixed.
> >
> > I had sent a patch in my first mail. You can find the mail at the following links:
> >
> https://www.redhat.com/archives/libvir-list/2013-September/msg00337.html
> 
> No, I want the root cause fixed. This change to the bitmap APIs is just
> papering over any root cause bug.
> 

This is a simple problem about an address is released twice in some scenario. 
I think that released twice is the root cause. I'm not sure what you mean about root cause.
Did you mean that why the address will be released two times?