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

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.