On Thu, May 15, 2014 at 01:20:39PM +0200, Jiri Denemark wrote:

Coverity complains about event being leaked in qemuDomainCheckRemoveOptionalDisk. The best fix for it is to avoid searching for the disk (using its srouce path even) we got by walking

s/srouce/source/ , but the whole sentence is hard to read, just "The best fix for it is to remove it by its index.", and you can add ", which is also safer" if you want ;) ACK with at lease the spelling fixed, it fixes an issue with multiple disks using the same non-existing file path also. Martin

through vm->def->disks array.

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_domain.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 7a0be12..78cfdc6 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2176,11 +2176,11 @@ qemuDomainSetFakeReboot(virQEMUDriverPtr driver, static int qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk) + size_t diskIndex) { char uuid[VIR_UUID_STRING_BUFLEN]; virObjectEventPtr event = NULL; - virDomainDiskDefPtr del_disk = NULL; + virDomainDiskDefPtr disk = vm->def->disks[diskIndex]; const char *src = virDomainDiskGetSource(disk);

virUUIDFormat(vm->def->uuid, uuid); @@ -2200,13 +2200,8 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, event = virDomainEventDiskChangeNewFromObj(vm, src, NULL, disk->info.alias, VIR_DOMAIN_EVENT_DISK_DROP_MISSING_ON_START); - - if (!(del_disk = virDomainDiskRemoveByName(vm->def, src))) { - virReportError(VIR_ERR_INVALID_ARG, - _("no source device %s"), src); - return -1; - } - virDomainDiskDefFree(del_disk); + virDomainDiskRemove(vm->def, diskIndex); + virDomainDiskDefFree(disk); }

if (event) @@ -2218,11 +2213,11 @@ qemuDomainCheckRemoveOptionalDisk(virQEMUDriverPtr driver, static int qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, virDomainObjPtr vm, - virDomainDiskDefPtr disk, + size_t diskIndex, bool cold_boot) { char uuid[VIR_UUID_STRING_BUFLEN]; - int startupPolicy = disk->startupPolicy; + int startupPolicy = vm->def->disks[diskIndex]->startupPolicy;

virUUIDFormat(vm->def->uuid, uuid);

@@ -2244,7 +2239,7 @@ qemuDomainCheckDiskStartupPolicy(virQEMUDriverPtr driver, break; }

- if (qemuDomainCheckRemoveOptionalDisk(driver, vm, disk) < 0) + if (qemuDomainCheckRemoveOptionalDisk(driver, vm, diskIndex) < 0) goto error;

return 0; @@ -2282,11 +2277,11 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, { int ret = -1; size_t i; - virDomainDiskDefPtr disk;

VIR_DEBUG("Checking for disk presence"); for (i = vm->def->ndisks; i > 0; i--) { - disk = vm->def->disks[i - 1]; + size_t idx = i - 1; + virDomainDiskDefPtr disk = vm->def->disks[idx]; const char *path = virDomainDiskGetSource(disk); virStorageFileFormat format = virDomainDiskGetFormat(disk); virStorageType type = virStorageSourceGetActualType(&disk->src); @@ -2308,7 +2303,7 @@ qemuDomainCheckDiskPresence(virQEMUDriverPtr driver, continue;

if (disk->startupPolicy && - qemuDomainCheckDiskStartupPolicy(driver, vm, disk, + qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) >= 0) { virResetLastError(); continue; -- 1.9.3

-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list