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.
...
> Subject: [PATCH] qemuDomainSnapshotCreateXML: avoid NULL dereference
>
> * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): When setting
> "vm" to NULL, jump over vm-dereferencing code to "cleanup".
> (qemuDomainRevertToSnapshot): Likewise.
Confirmed that the patch addresses the clang-reported problems
and (of course) passes make check and syntax-check.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list