[libvirt] [PATCH 1/2] qemu: Save qemuDomainGetStats error
Ján Tomko
jtomko at redhat.com
Thu Dec 6 13:49:59 UTC 2018
On Tue, Nov 27, 2018 at 11:23:22AM -0500, John Ferlan wrote:
>During qemuConnectGetAllDomainStats if qemuDomainGetStats causes
>a failure, then when collecting more than one domain's worth of
>statistics the loop in virDomainStatsRecordListFree would call
>virDomainFree which would call virResetLastError effectively wiping
>out the reason we failed leaving the caller with no idea why the
>collection failed.
>
>To fix this, let's save the error and restore it prior to return
>so that a caller such as 'virsh domstats' doesn't get the generic
>"error: An error occurred, but the cause is unknown".
>
>Signed-off-by: John Ferlan <jferlan at redhat.com>
>---
> src/qemu/qemu_driver.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index 7d9e17e72c..2fb8eef609 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -21092,6 +21092,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
> unsigned int flags)
> {
> virQEMUDriverPtr driver = conn->privateData;
>+ virErrorPtr save_err = NULL;
git grep virError src/qemu shows most files use orig_err as the variable
name
> virDomainObjPtr *vms = NULL;
> virDomainObjPtr vm;
> size_t nvms;
>@@ -21160,6 +21161,7 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
> if (flags & VIR_CONNECT_GET_ALL_DOMAINS_STATS_BACKING)
> domflags |= QEMU_DOMAIN_STATS_BACKING;
> if (qemuDomainGetStats(conn, vm, stats, &tmp, domflags) < 0) {
>+ save_err = virSaveLastError();
Since virDomainStatsRecordListFree is the one resetting the error,
I think we should only save it right before that call.
This would cause a memory leak if qemuDomainGetStats would fail for more
than one domain.
> if (HAVE_JOB(domflags) && vm)
> qemuDomainObjEndJob(driver, vm);
>
>@@ -21184,6 +21186,10 @@ qemuConnectGetAllDomainStats(virConnectPtr conn,
> cleanup:
Here. And we have a specific helper for that:
virErrorPreserveLast(&orig_err);
> virDomainStatsRecordListFree(tmpstats);
> virObjectListFreeCount(vms, nvms);
>+ if (save_err) {
>+ virSetError(save_err);
>+ virFreeError(save_err);
>+ }
virErrorRestore(&orig_err);
With that addressed:
Reviewed-by: Ján Tomko <jtomko at redhat.com>
Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20181206/a7d39ab5/attachment-0001.sig>
More information about the libvir-list
mailing list