[libvirt] [PATCH 00/19] More Coverity patches
I almost didn't want to do this due to the sheer volume, but figured at the very least the bulk of these are resource leaks found by the much pickier new coverity scanner. After this there are "only" 70 issues found... John Ferlan (19): libxl_migration: Resolve Coverity NULL_RETURNS daemon: Resolve Coverity NEGATIVE_RETURNS domain_conf: Resolve Coverity RESOURCE_LEAK cpu_x86: Resolve Coverity RESOURCE_LEAK qemu_command: Resolve Coverity RESOURCE_LEAK qemu_agent: Resolve Coverity RESOURCE_LEAK libxl_domain: Resolve Coverity RESOURCE_LEAK qemu_capabilities: Resolve Coverity RESOURCE_LEAK network_conf: Resolve Coverity RESOURCE_LEAK virsh-network: Resolve Coverity RESOURCE_LEAK bridge_driver: Resolve Coverity RESOURCE_LEAK libxl_migration: Resolve Coverity RESOURCE_LEAK phyp_driver: Resolve Coverity RESOURCE_LEAK qemu_driver: Resolve Coverity RESOURCE_LEAK storage_conf: Resolve Coverity RESOURCE_LEAK qemu_monitor: Resolve Coverity NESTING_INDENT_MISMATCH domain_conf: Resolve Coverity DEADCODE qemu_driver: Resolve Coverity DEADCODE qemu_command: Resolve Coverity DEADCODE daemon/remote.c | 24 ++++++++++++------------ src/conf/domain_conf.c | 28 ++++++++++++++++++++++++---- src/conf/network_conf.c | 2 ++ src/conf/storage_conf.c | 2 ++ src/cpu/cpu_x86.c | 15 ++++++++++----- src/libxl/libxl_domain.c | 4 +++- src/libxl/libxl_migration.c | 11 +++++++++-- src/network/bridge_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_agent.c | 6 ++++-- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 9 +++++---- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_monitor.c | 3 ++- tools/virsh-network.c | 2 +- 15 files changed, 85 insertions(+), 33 deletions(-) -- 1.9.3
Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that prior to generating an event for the dom == NULL condition, the resume/suspend event should be queue'd after the virDomainSaveStatus() call which will goto cleanup and queue the saved event anyway. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..53ae63a 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -512,6 +512,11 @@ libxlDomainMigrationFinish(virConnectPtr dconn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup; + if (event) { + libxlDomainEventQueue(driver, event); + event = NULL; + } + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid); if (dom == NULL) { @@ -519,7 +524,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); - libxlDomainEventQueue(driver, event); } cleanup: -- 1.9.3
John Ferlan wrote:
Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that prior to generating an event for the dom == NULL condition, the resume/suspend event should be queue'd after the virDomainSaveStatus() call which will goto cleanup and queue the saved event anyway.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..53ae63a 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -512,6 +512,11 @@ libxlDomainMigrationFinish(virConnectPtr dconn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup;
+ if (event) { + libxlDomainEventQueue(driver, event); + event = NULL; + } + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
if (dom == NULL) { @@ -519,7 +524,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); - libxlDomainEventQueue(driver, event); }
See my question in your first series about whether the dom == NULL check is even needed. If not, I can send a patch to remove the check, in which case this patch wouldn't be needed. https://www.redhat.com/archives/libvir-list/2014-August/msg01399.html Regards, Jim
On 08/27/2014 05:29 PM, Jim Fehlig wrote:
John Ferlan wrote:
Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that prior to generating an event for the dom == NULL condition, the resume/suspend event should be queue'd after the virDomainSaveStatus() call which will goto cleanup and queue the saved event anyway.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..53ae63a 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -512,6 +512,11 @@ libxlDomainMigrationFinish(virConnectPtr dconn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup;
+ if (event) { + libxlDomainEventQueue(driver, event); + event = NULL; + } + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
if (dom == NULL) { @@ -519,7 +524,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); - libxlDomainEventQueue(driver, event); }
See my question in your first series about whether the dom == NULL check is even needed. If not, I can send a patch to remove the check, in which case this patch wouldn't be needed.
https://www.redhat.com/archives/libvir-list/2014-August/msg01399.html
I am going to remove this one from my series and let you handle it. I would seem perhaps that the code was there to ensure that if either of the calls to virDomainObjSetState() ended up resulting in 'dom' not returning something from the virGetDomain(), then like perhaps the qemu migration code, the "best choice" was to remove it. In particular I looked at the second VIR_DOMAIN_SHUTOFF_FAILED in qemuMigrationFinish() for a "comparison". I'm by far no libvirt migration processing expert though... Tks - John
John Ferlan wrote:
On 08/27/2014 05:29 PM, Jim Fehlig wrote:
John Ferlan wrote:
Coverity noted that all callers to libxlDomainEventQueue() could ensure the second parameter (event) was true before calling except this case. As I look at the code and how events are used - it seems that prior to generating an event for the dom == NULL condition, the resume/suspend event should be queue'd after the virDomainSaveStatus() call which will goto cleanup and queue the saved event anyway.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index dbb5a8f..53ae63a 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -512,6 +512,11 @@ libxlDomainMigrationFinish(virConnectPtr dconn, if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) goto cleanup;
+ if (event) { + libxlDomainEventQueue(driver, event); + event = NULL; + } + dom = virGetDomain(dconn, vm->def->name, vm->def->uuid);
if (dom == NULL) { @@ -519,7 +524,6 @@ libxlDomainMigrationFinish(virConnectPtr dconn, libxlDomainCleanup(driver, vm, VIR_DOMAIN_SHUTOFF_FAILED); event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_STOPPED, VIR_DOMAIN_EVENT_STOPPED_FAILED); - libxlDomainEventQueue(driver, event); }
See my question in your first series about whether the dom == NULL check is even needed. If not, I can send a patch to remove the check, in which case this patch wouldn't be needed.
https://www.redhat.com/archives/libvir-list/2014-August/msg01399.html
I am going to remove this one from my series and let you handle it.
After looking at the code again, I think it is safest to go with your patch.
I would seem perhaps that the code was there to ensure that if either of the calls to virDomainObjSetState() ended up resulting in 'dom' not returning something from the virGetDomain(), then like perhaps the qemu migration code, the "best choice" was to remove it.
Yep, agreed. I haven't convinced myself that it is impossible for virGetDomain() to return a NULL domainPtr, but in the event it does I agree it is best to remove the domain. Regards, Jim
On 08/28/2014 11:20 AM, Jim Fehlig wrote:
John Ferlan wrote:
I am going to remove this one from my series and let you handle it.
After looking at the code again, I think it is safest to go with your patch.
I would seem perhaps that the code was there to ensure that if either of the calls to virDomainObjSetState() ended up resulting in 'dom' not returning something from the virGetDomain(), then like perhaps the qemu migration code, the "best choice" was to remove it.
Yep, agreed. I haven't convinced myself that it is impossible for virGetDomain() to return a NULL domainPtr, but in the event it does I agree it is best to remove the domain.
Regards, Jim
Close enough to an ACK for me ;-) I've pushed the change Thanks John
In each of these cases, Coverity complains that the result count returned on error paths would be -1 disregarding that the count and the corresponding are "linked" together (it doesn't know that). Simple enough to check and remove the warning Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 9251576..3efe12b 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1535,7 +1535,7 @@ remoteDispatchConnectListAllDomains(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (doms) { + if (doms && ndomains > 0) { for (i = 0; i < ndomains; i++) virDomainFree(doms[i]); VIR_FREE(doms); @@ -4632,7 +4632,7 @@ remoteDispatchDomainGetDiskErrors(virNetServerPtr server ATTRIBUTE_UNUSED, virNetMessageSaveError(rerr); if (dom) virDomainFree(dom); - if (errors) { + if (errors && len > 0) { size_t i; for (i = 0; i < len; i++) VIR_FREE(errors[i].disk); @@ -4698,7 +4698,7 @@ remoteDispatchDomainListAllSnapshots(virNetServerPtr server ATTRIBUTE_UNUSED, virNetMessageSaveError(rerr); if (dom) virDomainFree(dom); - if (snaps) { + if (snaps && nsnaps > 0) { for (i = 0; i < nsnaps; i++) virDomainSnapshotFree(snaps[i]); VIR_FREE(snaps); @@ -4769,7 +4769,7 @@ remoteDispatchDomainSnapshotListAllChildren(virNetServerPtr server ATTRIBUTE_UNU virDomainSnapshotFree(snapshot); if (dom) virDomainFree(dom); - if (snaps) { + if (snaps && nsnaps > 0) { for (i = 0; i < nsnaps; i++) virDomainSnapshotFree(snaps[i]); VIR_FREE(snaps); @@ -4828,7 +4828,7 @@ remoteDispatchConnectListAllStoragePools(virNetServerPtr server ATTRIBUTE_UNUSED cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (pools) { + if (pools && npools > 0) { for (i = 0; i < npools; i++) virStoragePoolFree(pools[i]); VIR_FREE(pools); @@ -4891,7 +4891,7 @@ remoteDispatchStoragePoolListAllVolumes(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (vols) { + if (vols && nvols > 0) { for (i = 0; i < nvols; i++) virStorageVolFree(vols[i]); VIR_FREE(vols); @@ -4952,7 +4952,7 @@ remoteDispatchConnectListAllNetworks(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (nets) { + if (nets && nnets > 0) { for (i = 0; i < nnets; i++) virNetworkFree(nets[i]); VIR_FREE(nets); @@ -5011,7 +5011,7 @@ remoteDispatchConnectListAllInterfaces(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (ifaces) { + if (ifaces && nifaces > 0) { for (i = 0; i < nifaces; i++) virInterfaceFree(ifaces[i]); VIR_FREE(ifaces); @@ -5070,7 +5070,7 @@ remoteDispatchConnectListAllNodeDevices(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (devices) { + if (devices && ndevices > 0) { for (i = 0; i < ndevices; i++) virNodeDeviceFree(devices[i]); VIR_FREE(devices); @@ -5129,7 +5129,7 @@ remoteDispatchConnectListAllNWFilters(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (filters) { + if (filters && nfilters > 0) { for (i = 0; i < nfilters; i++) virNWFilterFree(filters[i]); VIR_FREE(filters); @@ -5188,7 +5188,7 @@ remoteDispatchConnectListAllSecrets(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (secrets) { + if (secrets && nsecrets > 0) { for (i = 0; i < nsecrets; i++) virSecretFree(secrets[i]); VIR_FREE(secrets); @@ -6373,7 +6373,7 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED, cleanup: if (rv < 0) virNetMessageSaveError(rerr); - if (leases) { + if (leases && nleases > 0) { for (i = 0; i < nleases; i++) virNetworkDHCPLeaseFree(leases[i]); VIR_FREE(leases); -- 1.9.3
Resolve a few RESOURCE_LEAK's identified by Coverity Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 48afb8c..c7dbc73 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5654,7 +5654,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } VIR_FREE(ready); } - } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) { + } else if (!authdef && + xmlStrEqual(cur->name, BAD_CAST "auth")) { if (!(authdef = virStorageAuthDefParse(node->doc, cur))) goto error; if ((auth_secret_usage = @@ -12069,15 +12070,17 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; } - if (vcpupin->vcpuid >= def->vcpus) + if (vcpupin->vcpuid >= def->vcpus) { /* To avoid the regression when daemon loading * domain confs, we can't simply error out if * <vcpupin> nodes greater than current vcpus, * ignoring them instead. */ VIR_WARN("Ignore vcpupin for not onlined vcpus"); - else + VIR_FREE(vcpupin); + } else { def->cputune.vcpupin[def->cputune.nvcpupin++] = vcpupin; + } } VIR_FREE(nodes); @@ -13127,6 +13130,7 @@ virDomainDefParseXML(xmlDocPtr xml, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't add host USB device: " "USB is disabled in this host")); + virDomainHostdevDefFree(hostdev); goto error; } @@ -13266,6 +13270,7 @@ virDomainDefParseXML(xmlDocPtr xml, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Can't add redirected USB device: " "USB is disabled for this domain")); + virDomainRedirdevDefFree(redirdev); goto error; } @@ -15127,8 +15132,10 @@ virDomainEmulatorPinAdd(virDomainDefPtr def, emulatorpin->vcpuid = -1; emulatorpin->cpumask = virBitmapNewData(cpumap, maplen); - if (!emulatorpin->cpumask) + if (!emulatorpin->cpumask) { + virDomainVcpuPinDefFree(emulatorpin); return -1; + } def->cputune.emulatorpin = emulatorpin; } else { -- 1.9.3
On 08/27/2014 10:54 PM, John Ferlan wrote:
Resolve a few RESOURCE_LEAK's identified by Coverity
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 48afb8c..c7dbc73 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5654,7 +5654,8 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, } VIR_FREE(ready); } - } else if (xmlStrEqual(cur->name, BAD_CAST "auth")) { + } else if (!authdef && + xmlStrEqual(cur->name, BAD_CAST "auth")) { if (!(authdef = virStorageAuthDefParse(node->doc, cur))) goto error; if ((auth_secret_usage = @@ -12069,15 +12070,17 @@ virDomainDefParseXML(xmlDocPtr xml, goto error; }
- if (vcpupin->vcpuid >= def->vcpus) + if (vcpupin->vcpuid >= def->vcpus) { /* To avoid the regression when daemon loading * domain confs, we can't simply error out if * <vcpupin> nodes greater than current vcpus, * ignoring them instead. */ VIR_WARN("Ignore vcpupin for not onlined vcpus"); - else + VIR_FREE(vcpupin);
virDomainVcpuPinDefFree should be used here as well as in the 'duplicate vcpupin' case a few lines above. ACK with that fixed. Jan
Coverity determined that the copied 'oldguest' would be leaked for both error and success paths. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/cpu/cpu_x86.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c index fb89086..b460e8d 100644 --- a/src/cpu/cpu_x86.c +++ b/src/cpu/cpu_x86.c @@ -2024,8 +2024,9 @@ static int x86UpdateHostModel(virCPUDefPtr guest, const virCPUDef *host) { - virCPUDefPtr oldguest; + virCPUDefPtr oldguest = NULL; size_t i; + int ret = -1; guest->match = VIR_CPU_MATCH_EXACT; @@ -2037,20 +2038,24 @@ x86UpdateHostModel(virCPUDefPtr guest, /* update the host model according to the desired configuration */ if (!(oldguest = virCPUDefCopy(guest))) - return -1; + goto cleanup; virCPUDefFreeModel(guest); if (virCPUDefCopyModel(guest, host, true) < 0) - return -1; + goto cleanup; for (i = 0; i < oldguest->nfeatures; i++) { if (virCPUDefUpdateFeature(guest, oldguest->features[i].name, oldguest->features[i].policy) < 0) - return -1; + goto cleanup; } - return 0; + ret = 0; + + cleanup: + virCPUDefFree(oldguest); + return ret; } -- 1.9.3
In qemuParseISCSIString() if an error was returned, then the call to qemuParseDriveURIString() where the uri is free'd wouldn't be run Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 8fb81a4..528f947 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2795,6 +2795,7 @@ qemuParseISCSIString(virDomainDiskDefPtr def) virReportError(VIR_ERR_INTERNAL_ERROR, _("invalid name '%s' for iSCSI disk"), def->src->path); + virURIFree(uri); return -1; } } -- 1.9.3
Coverity found that on error paths, the 'arg' value wasn't be cleaned up. Followed the example in qemuAgentSetVCPUs() where upon successful call to qemuAgentCommand() the 'cpus' is set to NULL; otherwise, when cleanup occurs the free the memory for 'arg' Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_agent.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index a10954a..fe38f6d 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1328,7 +1328,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, unsigned int nmountpoints) { int ret = -1; - virJSONValuePtr cmd, arg; + virJSONValuePtr cmd, arg = NULL; virJSONValuePtr reply = NULL; if (mountpoints && nmountpoints) { @@ -1343,7 +1343,8 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, } if (!cmd) - return -1; + goto cleanup; + arg = NULL; if (qemuAgentCommand(mon, cmd, &reply, true, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) @@ -1355,6 +1356,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, } cleanup: + virJSONValueFree(arg); virJSONValueFree(cmd); virJSONValueFree(reply); return ret; -- 1.9.3
On 2014/8/28 4:54, John Ferlan wrote:
Coverity found that on error paths, the 'arg' value wasn't be cleaned up. Followed the example in qemuAgentSetVCPUs() where upon successful call to qemuAgentCommand() the 'cpus' is set to NULL; otherwise, when cleanup occurs the free the memory for 'arg'
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_agent.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index a10954a..fe38f6d 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1328,7 +1328,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, unsigned int nmountpoints) { int ret = -1; - virJSONValuePtr cmd, arg; + virJSONValuePtr cmd, arg = NULL; virJSONValuePtr reply = NULL;
if (mountpoints && nmountpoints) { @@ -1343,7 +1343,8 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, }
if (!cmd) - return -1; + goto cleanup; + arg = NULL;
Setting arg to NULL can also lead to memory leak. It makes virJSONValueFree(arg) below invalid.
if (qemuAgentCommand(mon, cmd, &reply, true, VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) @@ -1355,6 +1356,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, }
cleanup: + virJSONValueFree(arg); virJSONValueFree(cmd); virJSONValueFree(reply); return ret;
On 08/28/2014 04:40 AM, Wang Rui wrote:
On 2014/8/28 4:54, John Ferlan wrote:
Coverity found that on error paths, the 'arg' value wasn't be cleaned up. Followed the example in qemuAgentSetVCPUs() where upon successful call to qemuAgentCommand() the 'cpus' is set to NULL; otherwise, when cleanup occurs the free the memory for 'arg'
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_agent.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index a10954a..fe38f6d 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1328,7 +1328,7 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, unsigned int nmountpoints) { int ret = -1; - virJSONValuePtr cmd, arg; + virJSONValuePtr cmd, arg = NULL; virJSONValuePtr reply = NULL;
if (mountpoints && nmountpoints) { @@ -1343,7 +1343,8 @@ int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, }
if (!cmd) - return -1; + goto cleanup; + arg = NULL;
Setting arg to NULL can also lead to memory leak. It makes virJSONValueFree(arg) below invalid.
If qemuAgentMakeCommand succeeds, the 'arg' array is now owned by 'cmd' and we need to set it to NULL here to prevent double free. ACK to the patch as-is. Jan
On 2014/8/28 17:03, Ján Tomko wrote:
On 08/28/2014 04:40 AM, Wang Rui wrote:
On 2014/8/28 4:54, John Ferlan wrote:
Coverity found that on error paths, the 'arg' value wasn't be cleaned up. Followed the example in qemuAgentSetVCPUs() where upon successful call to qemuAgentCommand() the 'cpus' is set to NULL; otherwise, when cleanup occurs the free the memory for 'arg'
Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_agent.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
[...] Setting arg to NULL can also lead to memory leak. It makes virJSONValueFree(arg) below invalid.
If qemuAgentMakeCommand succeeds, the 'arg' array is now owned by 'cmd' and we need to set it to NULL here to prevent double free.
Oh, I got it. Thanks.
On the error path need to free the chrdef Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_domain.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index cdac82c..557fc20 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -532,8 +532,10 @@ libxlDomainDefPostParse(virDomainDefPtr def, chrdef->target.port = 0; chrdef->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; - if (VIR_ALLOC_N(def->consoles, 1) < 0) + if (VIR_ALLOC_N(def->consoles, 1) < 0) { + virDomainChrDefFree(chrdef); return -1; + } def->nconsoles = 1; def->consoles[0] = chrdef; -- 1.9.3
Coverity determined that on error path that 'mach' wouldn't be free'd Since virCapabilitiesFreeGuestMachine() isn't globally available, we'll insert first and then if the VIR_STRDUP's fail they it will eventually cause the 'mach' to be freed in the error path Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ce899f2..fcd7b19 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2106,6 +2106,7 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, virCapsGuestMachinePtr mach; if (VIR_ALLOC(mach) < 0) goto error; + (*machines)[i] = mach; if (qemuCaps->machineAliases[i]) { if (VIR_STRDUP(mach->name, qemuCaps->machineAliases[i]) < 0 || VIR_STRDUP(mach->canonical, qemuCaps->machineTypes[i]) < 0) @@ -2115,7 +2116,6 @@ int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, goto error; } mach->maxCpus = qemuCaps->machineMaxCpus[i]; - (*machines)[i] = mach; } return 0; -- 1.9.3
Need to VIR_FREE the startip/endip we allocated for the error message Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/network_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index dc25c6e..9571ee1 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -3627,6 +3627,8 @@ virNetworkDefUpdateIPDHCPRange(virNetworkDefPtr def, def->name, startip ? startip : "unknown", endip ? endip : "unknown"); + VIR_FREE(startip); + VIR_FREE(endip); goto cleanup; } -- 1.9.3
Need to free 'xmlFromFile' on/for the error path when current was returning false only Signed-off-by: John Ferlan <jferlan@redhat.com> --- tools/virsh-network.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 578abe0..9497472 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -974,7 +974,7 @@ cmdNetworkUpdate(vshControl *ctl, const vshCmd *cmd) if (current) { if (live || config) { vshError(ctl, "%s", _("--current must be specified exclusively")); - return false; + goto cleanup; } flags |= VIR_NETWORK_UPDATE_AFFECT_CURRENT; } else { -- 1.9.3
In the error path the 'ipaddr' wasn't VIR_FREE'd before jumping to cleanup Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/network/bridge_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9491c51..4b3f07f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -993,6 +993,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network, "(as described in RFC1918/RFC3484/RFC4193)."), ipaddr, (int)version / 1000000, (int)(version % 1000000) / 1000); + VIR_FREE(ipaddr); goto cleanup; } virBufferAsprintf(&configbuf, "listen-address=%s\n", ipaddr); -- 1.9.3
In libxlDomainMigrationPrepare() if the uri_in is false, then 'hostname' is allocated and used "generically" in the routine, but not freed. Conversely, if uri_in is true, then a uri is allocated and hostname is set to the uri->hostname value and likewise generically used. At function exit, hostname wasn't free'd in the !uri_in path, so that was added. To just make it clearer on usage the else path became the call to virURIFree() although I suppose technically it didn't have to since it would be a call using (NULL) Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libxl/libxl_migration.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c index 53ae63a..0b562f7 100644 --- a/src/libxl/libxl_migration.c +++ b/src/libxl/libxl_migration.c @@ -412,7 +412,10 @@ libxlDomainMigrationPrepare(virConnectPtr dconn, } done: - virURIFree(uri); + if (!uri_in) + VIR_FREE(hostname); + else + virURIFree(uri); if (vm) virObjectUnlock(vm); return ret; -- 1.9.3
Coverity determines that when jumping to the connected: label, the addressinfo (ai) is not free'd. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/phyp/phyp_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index aa1e105..ea1981a 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -946,6 +946,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth, sock = socket(cur->ai_family, cur->ai_socktype, cur->ai_protocol); if (sock >= 0) { if (connect(sock, cur->ai_addr, cur->ai_addrlen) == 0) { + freeaddrinfo(ai); goto connected; } VIR_FORCE_CLOSE(sock); -- 1.9.3
Coverity found that the 'buf' wasn't VIR_FREE'd at exit. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 73959da..323957e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10516,6 +10516,7 @@ qemuDomainGetBlockInfo(virDomainPtr dom, } cleanup: + VIR_FREE(buf); VIR_FREE(alias); virStorageSourceFree(meta); VIR_FORCE_CLOSE(fd); -- 1.9.3
If there was a failure processing 'authdef' and the code went to cleanup before the setting to source->auth, then it'd be leaked. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/storage_conf.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 5a16767..e72a869 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -661,6 +661,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, } source->auth = authdef; + authdef = NULL; } source->vendor = virXPathString("string(./vendor/@name)", ctxt); @@ -673,6 +674,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt, VIR_FREE(port); VIR_FREE(nodeset); VIR_FREE(adapter_type); + virStorageAuthDefFree(authdef); return ret; } -- 1.9.3
The PROBE macro can expand to more than one line/statement - put curly braces around the if statement to be safe Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index d5ba08d..5b2952a 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -506,10 +506,11 @@ qemuMonitorIOWrite(qemuMonitorPtr mon) mon->msg->txLength - mon->msg->txOffset, done, errno); - if (mon->msg->txFD != -1) + if (mon->msg->txFD != -1) { PROBE(QEMU_MONITOR_IO_SEND_FD, "mon=%p fd=%d ret=%d errno=%d", mon, mon->msg->txFD, done, errno); + } if (done < 0) { if (errno == EAGAIN) -- 1.9.3
A bunch of a useless warnings brought on by our own doing. Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/conf/domain_conf.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index c7dbc73..ce5aad6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2804,6 +2804,8 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, return -1; } + /* Coverity is not very happy with this - all dead_error_condition */ +#if !STATIC_ANALYSIS /* This switch statement is here to trigger compiler warning when adding * a new device type. When you are adding a new field to the switch you * also have to add an iteration statement above. Otherwise the switch @@ -2833,6 +2835,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def, case VIR_DOMAIN_DEVICE_RNG: break; } +#endif return 0; } @@ -12263,6 +12266,7 @@ virDomainDefParseXML(xmlDocPtr xml, ctxt->node = node; break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_FEATURE_LAST: break; } @@ -12346,6 +12350,7 @@ virDomainDefParseXML(xmlDocPtr xml, def->hyperv_features[feature] = value; break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; } @@ -12394,6 +12399,7 @@ virDomainDefParseXML(xmlDocPtr xml, def->kvm_features[feature] = value; break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break; } @@ -14406,6 +14412,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, } break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; } @@ -14429,6 +14436,7 @@ virDomainDefFeaturesCheckABIStability(virDomainDefPtr src, break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break; } @@ -18253,6 +18261,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "/>\n"); break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; } @@ -18277,6 +18286,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, def->kvm_features[j])); break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_KVM_LAST: break; } @@ -18304,6 +18314,7 @@ virDomainDefFormatInternal(virDomainDefPtr def, virBufferAddLit(buf, "</capabilities>\n"); break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_FEATURE_LAST: break; } @@ -20239,6 +20250,7 @@ virDomainObjGetMetadata(virDomainObjPtr vm, goto cleanup; break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_METADATA_LAST: break; } @@ -20324,6 +20336,7 @@ virDomainDefSetMetadata(virDomainDefPtr def, } break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_METADATA_LAST: break; } -- 1.9.3
A bunch of false positives brought on by our own doings Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_driver.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 323957e..2c27cd6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -8167,6 +8167,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, goto cleanup; break; + /* coverity[dead_error_begin] */ default: break; /* should not hit here */ @@ -8349,6 +8350,7 @@ qemuDomainGetBlkioParameters(virDomainPtr dom, break; + /* coverity[dead_error_begin] */ default: break; /* should not hit here */ @@ -8601,6 +8603,7 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; break; + /* coverity[dead_error_begin] */ default: break; /* should not hit here */ @@ -8645,6 +8648,7 @@ qemuDomainGetMemoryParameters(virDomainPtr dom, goto cleanup; break; + /* coverity[dead_error_begin] */ default: break; /* should not hit here */ @@ -8942,6 +8946,7 @@ qemuDomainGetNumaParameters(virDomainPtr dom, break; + /* coverity[dead_error_begin] */ default: break; /* should not hit here */ @@ -10118,6 +10123,7 @@ qemuDomainGetInterfaceParameters(virDomainPtr dom, if (net->bandwidth && net->bandwidth->out) params[i].value.ui = net->bandwidth->out->burst; break; + /* coverity[dead_error_begin] */ default: break; /* should not hit here */ @@ -16188,6 +16194,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, reply.write_iops_sec) < 0) goto endjob; break; + /* coverity[dead_error_begin] */ default: break; } -- 1.9.3
One useless warning, but the other one rather pertinent. On entry the 'trans' variable is initialized to VIR_DOMAIN_DISK_TRANS_DEFAULT. When the "trans" was found in the parsing loop it def->geometry.trans was assigned to the return from virDomainDiskGeometryTransTypeFromString and then 'trans' was used to do the comparison to see if it was valid. So remove 'trans' and use def->geometry.trans properly Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 528f947..bf6285f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6237,6 +6237,7 @@ qemuBuildCpuArgStr(virQEMUDriverPtr driver, def->hyperv_spinlocks); break; + /* coverity[dead_error_begin] */ case VIR_DOMAIN_HYPERV_LAST: break; } @@ -9835,7 +9836,6 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, int idx = -1; int busid = -1; int unitid = -1; - int trans = VIR_DOMAIN_DISK_TRANS_DEFAULT; if (qemuParseKeywords(val, &keywords, @@ -10054,12 +10054,12 @@ qemuParseCommandLineDisk(virDomainXMLOptionPtr xmlopt, } else if (STREQ(keywords[i], "trans")) { def->geometry.trans = virDomainDiskGeometryTransTypeFromString(values[i]); - if ((trans < VIR_DOMAIN_DISK_TRANS_DEFAULT) || - (trans >= VIR_DOMAIN_DISK_TRANS_LAST)) { + if ((def->geometry.trans < VIR_DOMAIN_DISK_TRANS_DEFAULT) || + (def->geometry.trans >= VIR_DOMAIN_DISK_TRANS_LAST)) { virDomainDiskDefFree(def); def = NULL; virReportError(VIR_ERR_INTERNAL_ERROR, - _("cannot parse translation value'%s'"), + _("cannot parse translation value '%s'"), values[i]); goto error; } -- 1.9.3
On 2014/8/28 4:54, John Ferlan wrote:
I almost didn't want to do this due to the sheer volume, but figured at the very least the bulk of these are resource leaks found by the much pickier new coverity scanner.
After this there are "only" 70 issues found...
Nice. I did coverity scan yesterday by coincidence. There are 1400+ total errors on my scan environment. I began to analyze 300+ RESOURCE LEAK errors. You said there were more than 140 on your side. So when you have finished fixing errors on your side, I'm glad to check if some other RESOURCE LEAK errors left and fix them
On 08/27/2014 09:38 PM, Wang Rui wrote:
On 2014/8/28 4:54, John Ferlan wrote:
I almost didn't want to do this due to the sheer volume, but figured at the very least the bulk of these are resource leaks found by the much pickier new coverity scanner.
After this there are "only" 70 issues found...
Nice. I did coverity scan yesterday by coincidence. There are 1400+ total errors on my scan environment. I began to analyze 300+ RESOURCE LEAK errors. You said there were more than 140 on your side. So when you have finished fixing errors on your side, I'm glad to check if some other RESOURCE LEAK errors left and fix them
I used "Coverity Static Analysis version 7.5.0 on Linux 3.15.8-200.fc20.x86_64 x86_64" that we have as a shared license of sorts inside Red Hat. I generally just take the defaults for the analysis "strength" (eg, the --aggressiveness-level is the default of low rather than medium or high, which I know from experience are much more intolerant). My 141 count covered the gamut of issues and will be reduced to 72 with these patches applied. Some of the remaining are sourced from the same problem - some are false positives. It just takes time to go through and figure that out and what the "best solution" may be. Perhaps you are running with a different version or a different aggressiveness especially to have 300+ resource leaks. That's fine - the more found the better off we are. John
On 08/27/2014 10:54 PM, John Ferlan wrote:
I almost didn't want to do this due to the sheer volume, but figured at the very least the bulk of these are resource leaks found by the much pickier new coverity scanner.
After this there are "only" 70 issues found...
John Ferlan (19): libxl_migration: Resolve Coverity NULL_RETURNS
I still don't know how to answer this one. ACK to the rest, see my reply to 3/19 for a minor tweak. Jan
daemon: Resolve Coverity NEGATIVE_RETURNS domain_conf: Resolve Coverity RESOURCE_LEAK cpu_x86: Resolve Coverity RESOURCE_LEAK qemu_command: Resolve Coverity RESOURCE_LEAK qemu_agent: Resolve Coverity RESOURCE_LEAK libxl_domain: Resolve Coverity RESOURCE_LEAK qemu_capabilities: Resolve Coverity RESOURCE_LEAK network_conf: Resolve Coverity RESOURCE_LEAK virsh-network: Resolve Coverity RESOURCE_LEAK bridge_driver: Resolve Coverity RESOURCE_LEAK libxl_migration: Resolve Coverity RESOURCE_LEAK phyp_driver: Resolve Coverity RESOURCE_LEAK qemu_driver: Resolve Coverity RESOURCE_LEAK storage_conf: Resolve Coverity RESOURCE_LEAK qemu_monitor: Resolve Coverity NESTING_INDENT_MISMATCH domain_conf: Resolve Coverity DEADCODE qemu_driver: Resolve Coverity DEADCODE qemu_command: Resolve Coverity DEADCODE
On 08/27/2014 04:54 PM, John Ferlan wrote:
I almost didn't want to do this due to the sheer volume, but figured at the very least the bulk of these are resource leaks found by the much pickier new coverity scanner.
After this there are "only" 70 issues found...
John Ferlan (19): libxl_migration: Resolve Coverity NULL_RETURNS daemon: Resolve Coverity NEGATIVE_RETURNS domain_conf: Resolve Coverity RESOURCE_LEAK cpu_x86: Resolve Coverity RESOURCE_LEAK qemu_command: Resolve Coverity RESOURCE_LEAK qemu_agent: Resolve Coverity RESOURCE_LEAK libxl_domain: Resolve Coverity RESOURCE_LEAK qemu_capabilities: Resolve Coverity RESOURCE_LEAK network_conf: Resolve Coverity RESOURCE_LEAK virsh-network: Resolve Coverity RESOURCE_LEAK bridge_driver: Resolve Coverity RESOURCE_LEAK libxl_migration: Resolve Coverity RESOURCE_LEAK phyp_driver: Resolve Coverity RESOURCE_LEAK qemu_driver: Resolve Coverity RESOURCE_LEAK storage_conf: Resolve Coverity RESOURCE_LEAK qemu_monitor: Resolve Coverity NESTING_INDENT_MISMATCH domain_conf: Resolve Coverity DEADCODE qemu_driver: Resolve Coverity DEADCODE qemu_command: Resolve Coverity DEADCODE
daemon/remote.c | 24 ++++++++++++------------ src/conf/domain_conf.c | 28 ++++++++++++++++++++++++---- src/conf/network_conf.c | 2 ++ src/conf/storage_conf.c | 2 ++ src/cpu/cpu_x86.c | 15 ++++++++++----- src/libxl/libxl_domain.c | 4 +++- src/libxl/libxl_migration.c | 11 +++++++++-- src/network/bridge_driver.c | 1 + src/phyp/phyp_driver.c | 1 + src/qemu/qemu_agent.c | 6 ++++-- src/qemu/qemu_capabilities.c | 2 +- src/qemu/qemu_command.c | 9 +++++---- src/qemu/qemu_driver.c | 8 ++++++++ src/qemu/qemu_monitor.c | 3 ++- tools/virsh-network.c | 2 +- 15 files changed, 85 insertions(+), 33 deletions(-)
I removed patch 1 (letting Jim handle it) I modified patch 3 (or now 2) to use virDomainVcpuPinDefFree() I have pushed the series. Thanks for the quick review - John
participants (4)
-
Jim Fehlig -
John Ferlan -
Ján Tomko -
Wang Rui