[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] new NULL-dereference in qemu_driver.c



Chris Lalancette wrote:
> On 04/28/2010 11:43 AM, Jim Meyering wrote:
>> Chris Lalancette wrote:
>>> On 04/27/2010 04:40 PM, Jim Meyering wrote:
>>>> Daniel P. Berrange wrote:
>>>>
>>>>> On Tue, Apr 27, 2010 at 06:45:16PM +0200, Jim Meyering wrote:
>>>>>> I ran clang on the very latest and it spotted this problem:
>>>>>> >From qemu_driver.c, around line 11100,
>>>>>>
>>>>>>     else {
>>>>>>         /* qemu is a little funny with running guests and the restoration
>>>>>>          * of snapshots.  If the snapshot was taken online,
>>>>>>          * then after a "loadvm" monitor command, the VM is set running
>>>>>>          * again.  If the snapshot was taken offline, then after a "loadvm"
>>>>>>          * monitor command the VM is left paused.  Unpausing it leads to
>>>>>>          * the memory state *before* the loadvm with the disk *after* the
>>>>>>          * loadvm, which obviously is bound to corrupt something.
>>>>>>          * Therefore we destroy the domain and set it to "off" in this case.
>>>>>>          */
>>>>>>
>>>>>>         if (virDomainObjIsActive(vm)) {
>>>>>>             qemudShutdownVMDaemon(driver, vm);
>>>>>>             event = virDomainEventNewFromObj(vm,
>>>>>>                                              VIR_DOMAIN_EVENT_STOPPED,
>>>>>>                                              VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT);
>>>>>>             if (!vm->persistent) {
>>>>>>                 if (qemuDomainObjEndJob(vm) > 0)
>>>>>>                     virDomainRemoveInactive(&driver->domains, vm);
>>>>>>                 vm = NULL;
>>>>>
>>>>> This needs to add 'goto endjob' or possibly 'goto cleanup'
>>>>
>>>> No point in endjob, since it does nothing when vm == NULL.
>>>>
>>>> Here's a tentative patch for that and another, similar problem
>>>> (haven't even compiled it or run it through clang, but have to run).
>>>> Will follow up tomorrow.
>>>
>>> Yeah, this looks reasonable and is what I was going to submit.  It
>>> would be good to give a test first, though.
>>
>> Can any of you easily test it?
>> I can't right now.
>
> Yep, this works fine with transient domains and snapshotting.

Thanks for testing that, Chris.
I'll rebase and push in 8 or 10 hours.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]