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

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.