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.
--
Chris Lalancette