The only caller doesn't check the value and also there are no real errors to report anyways. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cd761f87b5..02ea582767 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22709,24 +22709,20 @@ qemuDomainGetGuestInfoCheckSupport(unsigned int *types) *types = *types & supportedGuestInfoTypes; } -/* Returns: 0 on success - * -1 otherwise - */ -static int +static void qemuAgentFSInfoFormatParams(qemuAgentFSInfoPtr *fsinfo, int nfs, virDomainDefPtr vmdef, virTypedParameterPtr *params, int *nparams, int *maxparams) { - int ret = -1; size_t i, j; /* FIXME: get disk target */ if (virTypedParamsAddUInt(params, nparams, maxparams, "fs.count", nfs) < 0) - goto cleanup; + return; for (i = 0; i < nfs; i++) { char param_name[VIR_TYPED_PARAM_FIELD_LENGTH]; @@ -22734,17 +22730,17 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfoPtr *fsinfo, "fs.%zu.name", i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, fsinfo[i]->name) < 0) - goto cleanup; + return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "fs.%zu.mountpoint", i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, fsinfo[i]->mountpoint) < 0) - goto cleanup; + return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "fs.%zu.fstype", i); if (virTypedParamsAddString(params, nparams, maxparams, param_name, fsinfo[i]->fstype) < 0) - goto cleanup; + return; /* disk usage values are not returned by older guest agents, so * only add the params if the value is set */ @@ -22753,20 +22749,20 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfoPtr *fsinfo, if (fsinfo[i]->total_bytes != -1 && virTypedParamsAddULLong(params, nparams, maxparams, param_name, fsinfo[i]->total_bytes) < 0) - goto cleanup; + return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "fs.%zu.used-bytes", i); if (fsinfo[i]->used_bytes != -1 && virTypedParamsAddULLong(params, nparams, maxparams, param_name, fsinfo[i]->used_bytes) < 0) - goto cleanup; + return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "fs.%zu.disk.count", i); if (virTypedParamsAddUInt(params, nparams, maxparams, param_name, fsinfo[i]->ndisks) < 0) - goto cleanup; + return; for (j = 0; j < fsinfo[i]->ndisks; j++) { virDomainDiskDefPtr diskdef = NULL; qemuAgentDiskInfoPtr d = fsinfo[i]->disks[j]; @@ -22782,7 +22778,7 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfoPtr *fsinfo, if (diskdef->dst && virTypedParamsAddString(params, nparams, maxparams, param_name, diskdef->dst) < 0) - goto cleanup; + return; } g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, @@ -22790,22 +22786,19 @@ qemuAgentFSInfoFormatParams(qemuAgentFSInfoPtr *fsinfo, if (d->serial && virTypedParamsAddString(params, nparams, maxparams, param_name, d->serial) < 0) - goto cleanup; + return; g_snprintf(param_name, VIR_TYPED_PARAM_FIELD_LENGTH, "fs.%zu.disk.%zu.device", i, j); if (d->devnode && virTypedParamsAddString(params, nparams, maxparams, param_name, d->devnode) < 0) - goto cleanup; + return; } } - ret = nfs; - - cleanup: - return ret; } + static int qemuDomainGetGuestInfo(virDomainPtr dom, unsigned int types, -- 2.24.1

'nfs' variable was set to -1 or -2 on agent failure. Cleanup then tried to free 'nfs' elements of the array which resulted into a crash. Make 'nfs' size_t and assign it only on successful agent call. https://bugzilla.redhat.com/show_bug.cgi?id=1812965 Broken by commit 599ae372d8cf092 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 2 +- src/qemu/qemu_driver.c | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 9f3fb9732f..dff327e8d5 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1914,7 +1914,7 @@ qemuAgentGetFSInfoFillDisks(virJSONValuePtr jsondisks, return 0; } -/* Returns: 0 on success +/* Returns: number of entries in '@info' on success * -2 when agent command is not supported by the agent * -1 otherwise */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 02ea582767..e285e9373c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22814,7 +22814,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, g_autofree char *hostname = NULL; unsigned int supportedTypes = types; int rc; - int nfs = 0; + size_t nfs = 0; qemuAgentFSInfoPtr *agentfsinfo = NULL; size_t i; @@ -22867,9 +22867,13 @@ qemuDomainGetGuestInfo(virDomainPtr dom, } } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) { - rc = nfs = qemuAgentGetFSInfo(agent, &agentfsinfo); - if (rc < 0 && !(rc == -2 && types == 0)) - goto exitagent; + rc = qemuAgentGetFSInfo(agent, &agentfsinfo); + if (rc < 0) { + if (!(rc == -2 && types == 0)) + goto exitagent; + } else { + nfs = rc; + } } ret = 0; -- 2.24.1

On Fri, Mar 13, 2020 at 07:39:24 +0100, Ján Tomko wrote: [...] I'll fix this. Yes that's also weird. I have a patch. Actually qemuAgentCommand calls qemuAgentCheckError all the time even if we don't want to report the error. I'm not sure if we care.