> -----Original Message----- > From: Daniel P. Berrange [mailto:berrange@xxxxxxxxxx] > Sent: Tuesday, September 10, 2013 3:57 PM > To: Liuji (Jeremy) > Cc: libvir-list@xxxxxxxxxx; Jinbo (Justin); Luohao (brian); Haofeng > Subject: Re: [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@xxxxxxxxxx] > > > Sent: Monday, September 09, 2013 7:26 PM > > > To: Liuji (Jeremy) > > > Cc: libvir-list@xxxxxxxxxx; Jinbo (Justin); Luohao (brian); Haofeng > > > Subject: Re: [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@xxxxxxxxxx] > > > > > Sent: Friday, September 06, 2013 6:37 PM > > > > > To: Liuji (Jeremy) > > > > > Cc: libvir-list@xxxxxxxxxx; Jinbo (Justin); Luohao (brian); Haofeng > > > > > Subject: Re: [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? -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list