[PATCH 00/12] qemu: More fixes for qemuDomainGetGuestInfo
This fixes other issues pointed out in the review of my previous fix for crash in qemuDomainGetGuestInfo. Peter Krempa (12): qemuDomainGetGuestInfo: don't assign NULL hostname qemuDomainGetGuestInfo: Validate supported information types qemuAgentCheckError: use g_autofree qemuAgentCommand: Wire up suppressing of error reporting for unsupported commands qemuAgentGetUsers: Fix return value on success qemuAgentGetHostname: Refactor to remove cleanup section qemuAgentGetHostname: expose 'report_unsupported' argument qemuAgentGetUsers: expose 'report_unsupported' argument qemuAgentGetOSInfo: expose 'report_unsupported' argument qemuAgentGetTimezone: expose 'report_unsupported' argument qemuAgentGetFSInfo: expose 'report_unsupported' argument qemuDomainGetGuestInfo: Suppress non-fatal errors src/qemu/qemu_agent.c | 194 ++++++++++++++++++++--------------------- src/qemu/qemu_agent.h | 16 ++-- src/qemu/qemu_driver.c | 89 ++++++++++--------- tests/qemuagenttest.c | 14 +-- 4 files changed, 162 insertions(+), 151 deletions(-) -- 2.24.1
Don't rely on error check and assign hostname only when non-NULL. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 641a90d595..241513705d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22909,14 +22909,14 @@ qemuDomainGetGuestInfo(virDomainPtr dom, } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) { rc = qemuAgentGetHostname(agent, &hostname); - if (rc < 0 && !(rc == -2 && types == 0)) { + if (rc < 0 && !(rc == -2 && types == 0)) goto exitagent; - } else { - if (virTypedParamsAddString(params, nparams, &maxparams, "hostname", - hostname) < 0) - goto exitagent; - } } + + if (hostname && + virTypedParamsAddString(params, nparams, &maxparams, "hostname", hostname) < 0) + goto exitagent; + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) { rc = qemuAgentGetFSInfo(agent, &agentfsinfo); if (rc < 0) { -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Don't rely on error check and assign hostname only when non-NULL.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
'qemuDomainGetGuestInfoCheckSupport' despite it's name was not checking whether the info types are supported. Convert the function to return integers and include the check. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 241513705d..f9a4cc2758 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22744,20 +22744,32 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain, return ret; } -static const unsigned int supportedGuestInfoTypes = +static const unsigned int qemuDomainGetGuestInfoSupportedTypes = VIR_DOMAIN_GUEST_INFO_USERS | VIR_DOMAIN_GUEST_INFO_OS | VIR_DOMAIN_GUEST_INFO_TIMEZONE | VIR_DOMAIN_GUEST_INFO_HOSTNAME | VIR_DOMAIN_GUEST_INFO_FILESYSTEM; -static void -qemuDomainGetGuestInfoCheckSupport(unsigned int *types) +static int +qemuDomainGetGuestInfoCheckSupport(unsigned int types, + unsigned int *supportedTypes) { - if (*types == 0) - *types = supportedGuestInfoTypes; + if (types == 0) { + *supportedTypes = qemuDomainGetGuestInfoSupportedTypes; + return 0; + } + + *supportedTypes = types & qemuDomainGetGuestInfoSupportedTypes; + + if (types != *supportedTypes) { + virReportError(VIR_ERR_INVALID_ARG, + _("unsupported guest information types '0x%x'"), + types & ~qemuDomainGetGuestInfoSupportedTypes); + return -1; + } - *types = *types & supportedGuestInfoTypes; + return 0; } static void @@ -22863,14 +22875,16 @@ qemuDomainGetGuestInfo(virDomainPtr dom, int ret = -1; int maxparams = 0; g_autofree char *hostname = NULL; - unsigned int supportedTypes = types; + unsigned int supportedTypes; int rc; size_t nfs = 0; qemuAgentFSInfoPtr *agentfsinfo = NULL; size_t i; virCheckFlags(0, -1); - qemuDomainGetGuestInfoCheckSupport(&supportedTypes); + + if (qemuDomainGetGuestInfoCheckSupport(types, &supportedTypes) < 0) + return -1; if (!(vm = qemuDomainObjFromDomain(dom))) goto cleanup; -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
'qemuDomainGetGuestInfoCheckSupport' despite it's name was not checking
its
whether the info types are supported. Convert the function to return integers and include the check.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 9ea2c59563..8790c47b87 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1055,8 +1055,8 @@ qemuAgentCheckError(virJSONValuePtr cmd, { if (virJSONValueObjectHasKey(reply, "error")) { virJSONValuePtr error = virJSONValueObjectGet(reply, "error"); - char *cmdstr = virJSONValueToString(cmd, false); - char *replystr = virJSONValueToString(reply, false); + g_autofree char *cmdstr = virJSONValueToString(cmd, false); + g_autofree char *replystr = virJSONValueToString(reply, false); /* Log the full JSON formatted command & error */ VIR_DEBUG("unable to execute QEMU agent command %s: %s", @@ -1073,20 +1073,16 @@ qemuAgentCheckError(virJSONValuePtr cmd, qemuAgentCommandName(cmd), qemuAgentStringifyError(error)); - VIR_FREE(cmdstr); - VIR_FREE(replystr); return -1; } else if (!virJSONValueObjectHasKey(reply, "return")) { - char *cmdstr = virJSONValueToString(cmd, false); - char *replystr = virJSONValueToString(reply, false); + g_autofree char *cmdstr = virJSONValueToString(cmd, false); + g_autofree char *replystr = virJSONValueToString(reply, false); VIR_DEBUG("Neither 'return' nor 'error' is set in the JSON reply %s: %s", NULLSTR(cmdstr), NULLSTR(replystr)); virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to execute QEMU agent command '%s'"), qemuAgentCommandName(cmd)); - VIR_FREE(cmdstr); - VIR_FREE(replystr); return -1; } return 0; -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
In some cases we don't want to log errors if an agent command is unsupported. Wire it up into qemuAgentCheckError via qemuAgentCommandFull and provide a thin wrapper (qemuAgentCommand) to prevent having to fix all callers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 45 +++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 8790c47b87..bc5d57ab94 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1051,7 +1051,8 @@ qemuAgentCommandName(virJSONValuePtr cmd) static int qemuAgentCheckError(virJSONValuePtr cmd, - virJSONValuePtr reply) + virJSONValuePtr reply, + bool report_unsupported) { if (virJSONValueObjectHasKey(reply, "error")) { virJSONValuePtr error = virJSONValueObjectGet(reply, "error"); @@ -1063,15 +1064,25 @@ qemuAgentCheckError(virJSONValuePtr cmd, NULLSTR(cmdstr), NULLSTR(replystr)); /* Only send the user the command name + friendly error */ - if (!error) + if (!error) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unable to execute QEMU agent command '%s'"), qemuAgentCommandName(cmd)); - else - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unable to execute QEMU agent command '%s': %s"), - qemuAgentCommandName(cmd), - qemuAgentStringifyError(error)); + return -1; + } + + if (!report_unsupported) { + const char *klass = virJSONValueObjectGetString(error, "class"); + + if (STREQ_NULLABLE(klass, "CommandNotFound") || + STREQ_NULLABLE(klass, "CommandDisabled")) + return -2; + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unable to execute QEMU agent command '%s': %s"), + qemuAgentCommandName(cmd), + qemuAgentStringifyError(error)); return -1; } else if (!virJSONValueObjectHasKey(reply, "return")) { @@ -1089,10 +1100,11 @@ qemuAgentCheckError(virJSONValuePtr cmd, } static int -qemuAgentCommand(qemuAgentPtr agent, - virJSONValuePtr cmd, - virJSONValuePtr *reply, - int seconds) +qemuAgentCommandFull(qemuAgentPtr agent, + virJSONValuePtr cmd, + virJSONValuePtr *reply, + int seconds, + bool report_unsupported) { int ret = -1; qemuAgentMessage msg; @@ -1143,7 +1155,7 @@ qemuAgentCommand(qemuAgentPtr agent, } *reply = msg.rxObject; - ret = qemuAgentCheckError(cmd, *reply); + ret = qemuAgentCheckError(cmd, *reply, report_unsupported); cleanup: VIR_FREE(cmdstr); @@ -1153,6 +1165,15 @@ qemuAgentCommand(qemuAgentPtr agent, return ret; } +static int +qemuAgentCommand(qemuAgentPtr agent, + virJSONValuePtr cmd, + virJSONValuePtr *reply, + int seconds) +{ + return qemuAgentCommandFull(agent, cmd, reply, seconds, true); +} + static virJSONValuePtr G_GNUC_NULL_TERMINATED qemuAgentMakeCommand(const char *cmdname, ...) -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
In some cases we don't want to log errors if an agent command is unsupported. Wire it up into qemuAgentCheckError via qemuAgentCommandFull and provide a thin wrapper (qemuAgentCommand) to prevent having to fix all callers.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 45 +++++++++++++++++++++++++++++++------------ 1 file changed, 33 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Return 0 on success as per documentation and due to the fact that callers don't check the value. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bc5d57ab94..c55b989467 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2406,7 +2406,7 @@ qemuAgentGetUsers(qemuAgentPtr agent, return -1; } - return ndata; + return 0; } /* Returns: 0 on success -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Return 0 on success as per documentation and due to the fact that callers don't check the value.
-EPARSE Return 0 on success to match the documentation. The callers only check for negative values.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index bc5d57ab94..c55b989467 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2406,7 +2406,7 @@ qemuAgentGetUsers(qemuAgentPtr agent, return -1; }
- return ndata; + return 0; }
/* Returns: 0 on success
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use g_autoptr instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index c55b989467..07aba2b45d 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1720,44 +1720,36 @@ int qemuAgentGetHostname(qemuAgentPtr agent, char **hostname) { - int ret = -1; - virJSONValuePtr cmd; - virJSONValuePtr reply = NULL; + g_autoptr(virJSONValue) cmd = qemuAgentMakeCommand("guest-get-host-name", NULL); + g_autoptr(virJSONValue) reply = NULL; virJSONValuePtr data = NULL; const char *result = NULL; - cmd = qemuAgentMakeCommand("guest-get-host-name", - NULL); - if (!cmd) - return ret; + return -1; if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) { if (qemuAgentErrorCommandUnsupported(reply)) - ret = -2; - goto cleanup; + return -2; + + return -1; } if (!(data = virJSONValueObjectGet(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("malformed return value")); - goto cleanup; + return -1; } if (!(result = virJSONValueObjectGetString(data, "host-name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("'host-name' missing in guest-get-host-name reply")); - goto cleanup; + return -1; } *hostname = g_strdup(result); - ret = 0; - - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return 0; } -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Use g_autoptr instead.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use qemuAgentCommandFull in qemuAgentGetHostname so that we can suppress error reports if the caller will not require them. Callers for now always require error reporting but will be fixed later. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 22 +++++++++++++++------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 4 ++-- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 07aba2b45d..7de1fe496a 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1716,24 +1716,32 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, } +/** + * qemuAgentGetHostname: + * + * Gets the guest hostname using the guest agent. + * + * Returns 0 on success and fills @hostname. On error -1 is returned with an + * error reported and if '@report_unsupported' is false -2 is returned if the + * guest agent does not support the command without reporting an error + */ int qemuAgentGetHostname(qemuAgentPtr agent, - char **hostname) + char **hostname, + bool report_unsupported) { g_autoptr(virJSONValue) cmd = qemuAgentMakeCommand("guest-get-host-name", NULL); g_autoptr(virJSONValue) reply = NULL; virJSONValuePtr data = NULL; const char *result = NULL; + int rc; if (!cmd) return -1; - if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) { - if (qemuAgentErrorCommandUnsupported(reply)) - return -2; - - return -1; - } + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, + report_unsupported)) < 0) + return rc; if (!(data = virJSONValueObjectGet(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 67fe9fcde9..7cf38d7091 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -129,7 +129,8 @@ int qemuAgentUpdateCPUInfo(unsigned int nvcpus, int qemuAgentGetHostname(qemuAgentPtr mon, - char **hostname); + char **hostname, + bool report_unsupported); int qemuAgentGetTime(qemuAgentPtr mon, long long *seconds, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index f9a4cc2758..5e1f3efa7d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -20128,7 +20128,7 @@ qemuDomainGetHostnameAgent(virQEMUDriverPtr driver, goto endjob; agent = qemuDomainObjEnterAgent(vm); - ignore_value(qemuAgentGetHostname(agent, hostname)); + ignore_value(qemuAgentGetHostname(agent, hostname, true)); qemuDomainObjExitAgent(vm, agent); ret = 0; @@ -22922,7 +22922,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, goto exitagent; } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) { - rc = qemuAgentGetHostname(agent, &hostname); + rc = qemuAgentGetHostname(agent, &hostname, true); if (rc < 0 && !(rc == -2 && types == 0)) goto exitagent; } -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Use qemuAgentCommandFull in qemuAgentGetHostname so that we can suppress error reports if the caller will not require them. Callers for now always require error reporting but will be fixed later.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 22 +++++++++++++++------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 4 ++-- 3 files changed, 19 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use qemuAgentCommandFull so that callers of qemuAgentGetUsers can suppress error reports if the function is not supported by the guest agent. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 17 +++++++++-------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 7de1fe496a..261fc67021 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2319,29 +2319,30 @@ qemuAgentSetUserPassword(qemuAgentPtr agent, } /* Returns: 0 on success - * -2 when agent command is not supported by the agent - * -1 otherwise + * -2 when agent command is not supported by the agent and + * 'report_unsupported' is false (libvirt error is not reported) + * -1 otherwise (libvirt error is reported) */ int qemuAgentGetUsers(qemuAgentPtr agent, virTypedParameterPtr *params, int *nparams, - int *maxparams) + int *maxparams, + bool report_unsupported) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; virJSONValuePtr data = NULL; size_t ndata; size_t i; + int rc; if (!(cmd = qemuAgentMakeCommand("guest-get-users", NULL))) return -1; - if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) { - if (qemuAgentErrorCommandUnsupported(reply)) - return -2; - return -1; - } + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, + report_unsupported)) < 0) + return rc; if (!(data = virJSONValueObjectGetArray(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 7cf38d7091..3702abf593 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -151,7 +151,8 @@ int qemuAgentSetUserPassword(qemuAgentPtr mon, int qemuAgentGetUsers(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, - int *maxparams); + int *maxparams, + bool report_unsupported); int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5e1f3efa7d..1347514b6c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22907,7 +22907,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, * 'unsupported' errors and gather as much information as we can. In all * other cases, abort on error. */ if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) { - rc = qemuAgentGetUsers(agent, params, nparams, &maxparams); + rc = qemuAgentGetUsers(agent, params, nparams, &maxparams, true); if (rc < 0 && !(rc == -2 && types == 0)) goto exitagent; } diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 86fbfbaa4c..dee9068ce5 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1040,7 +1040,7 @@ testQemuAgentUsers(const void *data) /* get users */ if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams) < 0) + ¶ms, &nparams, &maxparams, true) < 0) goto cleanup; if (virTypedParamsGetUInt(params, nparams, "user.count", &count) < 0) @@ -1069,7 +1069,7 @@ testQemuAgentUsers(const void *data) /* get users with domain */ if (qemuAgentGetUsers(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams) < 0) + ¶ms, &nparams, &maxparams, true) < 0) goto cleanup; if (virTypedParamsGetUInt(params, nparams, "user.count", &count) < 0) -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Use qemuAgentCommandFull so that callers of qemuAgentGetUsers can suppress error reports if the function is not supported by the guest agent.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 17 +++++++++-------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use qemuAgentCommandFull so that callers of qemuAgentGetOSInfo can suppress error reports if the function is not supported by the guest agent. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 17 +++++++++-------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 261fc67021..77f3247602 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2411,27 +2411,28 @@ qemuAgentGetUsers(qemuAgentPtr agent, } /* Returns: 0 on success - * -2 when agent command is not supported by the agent - * -1 otherwise + * -2 when agent command is not supported by the agent and + * 'report_unsupported' is false (libvirt error is not reported) + * -1 otherwise (libvirt error is reported) */ int qemuAgentGetOSInfo(qemuAgentPtr agent, virTypedParameterPtr *params, int *nparams, - int *maxparams) + int *maxparams, + bool report_unsupported) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; virJSONValuePtr data = NULL; + int rc; if (!(cmd = qemuAgentMakeCommand("guest-get-osinfo", NULL))) return -1; - if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) { - if (qemuAgentErrorCommandUnsupported(reply)) - return -2; - return -1; - } + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, + report_unsupported)) < 0) + return rc; if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 3702abf593..9cade9ca4c 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -157,7 +157,8 @@ int qemuAgentGetUsers(qemuAgentPtr mon, int qemuAgentGetOSInfo(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, - int *maxparams); + int *maxparams, + bool report_unsupported); int qemuAgentGetTimezone(qemuAgentPtr mon, virTypedParameterPtr *params, diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1347514b6c..17d9bbe01c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22912,7 +22912,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, goto exitagent; } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) { - rc = qemuAgentGetOSInfo(agent, params, nparams, &maxparams); + rc = qemuAgentGetOSInfo(agent, params, nparams, &maxparams, true); if (rc < 0 && !(rc == -2 && types == 0)) goto exitagent; } diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index dee9068ce5..57d0f857cc 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1139,7 +1139,7 @@ testQemuAgentOSInfo(const void *data) /* get osinfo */ if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams) < 0) + ¶ms, &nparams, &maxparams, true) < 0) goto cleanup; if (nparams != 8) { @@ -1184,7 +1184,7 @@ testQemuAgentOSInfo(const void *data) /* get users with domain */ if (qemuAgentGetOSInfo(qemuMonitorTestGetAgent(test), - ¶ms, &nparams, &maxparams) < 0) + ¶ms, &nparams, &maxparams, true) < 0) goto cleanup; if (nparams != 10) { -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Use qemuAgentCommandFull so that callers of qemuAgentGetOSInfo can suppress error reports if the function is not supported by the guest agent.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 17 +++++++++-------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 4 ++-- 4 files changed, 14 insertions(+), 12 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use qemuAgentCommandFull so that callers of qemuAgentGetTimezone can suppress error reports if the function is not supported by the guest agent. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 17 +++++++++-------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 77f3247602..efb6c52f96 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -2465,29 +2465,30 @@ qemuAgentGetOSInfo(qemuAgentPtr agent, } /* Returns: 0 on success - * -2 when agent command is not supported by the agent - * -1 otherwise + * -2 when agent command is not supported by the agent and + * 'report_unsupported' is false (libvirt error is not reported) + * -1 otherwise (libvirt error is reported) */ int qemuAgentGetTimezone(qemuAgentPtr agent, virTypedParameterPtr *params, int *nparams, - int *maxparams) + int *maxparams, + bool report_unsupported) { g_autoptr(virJSONValue) cmd = NULL; g_autoptr(virJSONValue) reply = NULL; virJSONValuePtr data = NULL; const char *name; int offset; + int rc; if (!(cmd = qemuAgentMakeCommand("guest-get-timezone", NULL))) return -1; - if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) { - if (qemuAgentErrorCommandUnsupported(reply)) - return -2; - return -1; - } + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, + report_unsupported)) < 0) + return rc; if (!(data = virJSONValueObjectGetObject(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 9cade9ca4c..361a67aaee 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -163,7 +163,8 @@ int qemuAgentGetOSInfo(qemuAgentPtr mon, int qemuAgentGetTimezone(qemuAgentPtr mon, virTypedParameterPtr *params, int *nparams, - int *maxparams); + int *maxparams, + bool report_unsupported); void qemuAgentSetResponseTimeout(qemuAgentPtr mon, int timeout); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 17d9bbe01c..1b1fa5e209 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22917,7 +22917,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, goto exitagent; } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) { - rc = qemuAgentGetTimezone(agent, params, nparams, &maxparams); + rc = qemuAgentGetTimezone(agent, params, nparams, &maxparams, true); if (rc < 0 && !(rc == -2 && types == 0)) goto exitagent; } diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 57d0f857cc..7ea330892b 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -1244,7 +1244,7 @@ testQemuAgentTimezone(const void *data) response_) < 0) \ goto cleanup; \ if (qemuAgentGetTimezone(qemuMonitorTestGetAgent(test), \ - ¶ms_, &nparams_, &maxparams_) < 0) \ + ¶ms_, &nparams_, &maxparams_, true) < 0) \ goto cleanup; \ if (nparams_ != 2) { \ virReportError(VIR_ERR_INTERNAL_ERROR, \ -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Use qemuAgentCommandFull so that callers of qemuAgentGetTimezone can suppress error reports if the function is not supported by the guest agent.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 17 +++++++++-------- src/qemu/qemu_agent.h | 3 ++- src/qemu/qemu_driver.c | 2 +- tests/qemuagenttest.c | 2 +- 4 files changed, 13 insertions(+), 11 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Use qemuAgentCommandFull so that callers of qemuAgentGetFSInfo can suppress error reports if the function is not supported by the guest agent. Since this patch removes the last use of qemuAgentErrorCommandUnsupported the whole function is deleted as well. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 42 +++++++++--------------------------------- src/qemu/qemu_agent.h | 4 +++- src/qemu/qemu_driver.c | 4 ++-- tests/qemuagenttest.c | 4 ++-- 4 files changed, 16 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index efb6c52f96..358be30fa1 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -993,31 +993,6 @@ qemuAgentStringifyErrorClass(const char *klass) return "unknown QEMU command error"; } -/* Checks whether the agent reply msg is an error caused by an unsupported - * command. - * - * Returns true when reply is CommandNotFound or CommandDisabled - * false otherwise - */ -static bool -qemuAgentErrorCommandUnsupported(virJSONValuePtr reply) -{ - const char *klass; - virJSONValuePtr error; - - if (!reply) - return false; - - error = virJSONValueObjectGet(reply, "error"); - - if (!error) - return false; - - klass = virJSONValueObjectGetString(error, "class"); - return STREQ_NULLABLE(klass, "CommandNotFound") || - STREQ_NULLABLE(klass, "CommandDisabled"); -} - /* Ignoring OOM in this method, since we're already reporting * a more important error * @@ -1959,12 +1934,14 @@ qemuAgentGetFSInfoFillDisks(virJSONValuePtr jsondisks, } /* Returns: number of entries in '@info' on success - * -2 when agent command is not supported by the agent - * -1 otherwise + * -2 when agent command is not supported by the agent and + * 'report_unsupported' is false (libvirt error is not reported) + * -1 otherwise (libvirt error is reported) */ int qemuAgentGetFSInfo(qemuAgentPtr agent, - qemuAgentFSInfoPtr **info) + qemuAgentFSInfoPtr **info, + bool report_unsupported) { size_t i; int ret = -1; @@ -1973,16 +1950,15 @@ qemuAgentGetFSInfo(qemuAgentPtr agent, virJSONValuePtr data; size_t ndata = 0; qemuAgentFSInfoPtr *info_ret = NULL; + int rc; cmd = qemuAgentMakeCommand("guest-get-fsinfo", NULL); if (!cmd) return ret; - if (qemuAgentCommand(agent, cmd, &reply, agent->timeout) < 0) { - if (qemuAgentErrorCommandUnsupported(reply)) - ret = -2; - goto cleanup; - } + if ((rc = qemuAgentCommandFull(agent, cmd, &reply, agent->timeout, + report_unsupported)) < 0) + return rc; if (!(data = virJSONValueObjectGet(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 361a67aaee..2eeb376a68 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -98,7 +98,9 @@ int qemuAgentShutdown(qemuAgentPtr mon, int qemuAgentFSFreeze(qemuAgentPtr mon, const char **mountpoints, unsigned int nmountpoints); int qemuAgentFSThaw(qemuAgentPtr mon); -int qemuAgentGetFSInfo(qemuAgentPtr mon, qemuAgentFSInfoPtr **info); +int qemuAgentGetFSInfo(qemuAgentPtr mon, + qemuAgentFSInfoPtr **info, + bool report_unsupported); int qemuAgentSuspend(qemuAgentPtr mon, unsigned int target); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1b1fa5e209..2f043e3ad9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -21774,7 +21774,7 @@ qemuDomainGetFSInfoAgent(virQEMUDriverPtr driver, goto endjob; agent = qemuDomainObjEnterAgent(vm); - ret = qemuAgentGetFSInfo(agent, info); + ret = qemuAgentGetFSInfo(agent, info, true); qemuDomainObjExitAgent(vm, agent); endjob: @@ -22932,7 +22932,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) { - rc = qemuAgentGetFSInfo(agent, &agentfsinfo); + rc = qemuAgentGetFSInfo(agent, &agentfsinfo, true); if (rc < 0) { if (!(rc == -2 && types == 0)) goto exitagent; diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index 7ea330892b..42ef81ac9a 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -254,7 +254,7 @@ testQemuAgentGetFSInfo(const void *data) goto cleanup; if ((ninfo = qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test), - &info)) < 0) + &info, true)) < 0) goto cleanup; if (ninfo != 3) { @@ -326,7 +326,7 @@ testQemuAgentGetFSInfo(const void *data) "}") < 0) goto cleanup; - if (qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test), &info) >= 0) { + if (qemuAgentGetFSInfo(qemuMonitorTestGetAgent(test), &info, true) >= 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "agent get-fsinfo command should have failed"); goto cleanup; -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Use qemuAgentCommandFull so that callers of qemuAgentGetFSInfo can suppress error reports if the function is not supported by the guest agent.
Since this patch removes the last use of qemuAgentErrorCommandUnsupported the whole function is deleted as well.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_agent.c | 42 +++++++++--------------------------------- src/qemu/qemu_agent.h | 4 +++- src/qemu/qemu_driver.c | 4 ++-- tests/qemuagenttest.c | 4 ++-- 4 files changed, 16 insertions(+), 38 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Don't report cases when the guest information is not requested explicitly and not present either. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2f043e3ad9..946fe63ea6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -22876,6 +22876,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, int maxparams = 0; g_autofree char *hostname = NULL; unsigned int supportedTypes; + bool report_unsupported = types != 0; int rc; size_t nfs = 0; qemuAgentFSInfoPtr *agentfsinfo = NULL; @@ -22906,37 +22907,31 @@ qemuDomainGetGuestInfo(virDomainPtr dom, * categories were explicitly requested (i.e. 'types' is 0), ignore * 'unsupported' errors and gather as much information as we can. In all * other cases, abort on error. */ - if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) { - rc = qemuAgentGetUsers(agent, params, nparams, &maxparams, true); - if (rc < 0 && !(rc == -2 && types == 0)) - goto exitagent; - } - if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) { - rc = qemuAgentGetOSInfo(agent, params, nparams, &maxparams, true); - if (rc < 0 && !(rc == -2 && types == 0)) - goto exitagent; - } - if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) { - rc = qemuAgentGetTimezone(agent, params, nparams, &maxparams, true); - if (rc < 0 && !(rc == -2 && types == 0)) - goto exitagent; - } - if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) { - rc = qemuAgentGetHostname(agent, &hostname, true); - if (rc < 0 && !(rc == -2 && types == 0)) - goto exitagent; - } + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS && + qemuAgentGetUsers(agent, params, nparams, &maxparams, report_unsupported) == -1) + goto exitagent; + + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS && + qemuAgentGetOSInfo(agent, params, nparams, &maxparams, report_unsupported) == -1) + goto exitagent; + + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE && + qemuAgentGetTimezone(agent, params, nparams, &maxparams, report_unsupported) == -1) + goto exitagent; + + if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME && + qemuAgentGetHostname(agent, &hostname, report_unsupported) == -1) + goto exitagent; if (hostname && virTypedParamsAddString(params, nparams, &maxparams, "hostname", hostname) < 0) goto exitagent; if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) { - rc = qemuAgentGetFSInfo(agent, &agentfsinfo, true); - if (rc < 0) { - if (!(rc == -2 && types == 0)) - goto exitagent; - } else { + rc = qemuAgentGetFSInfo(agent, &agentfsinfo, report_unsupported); + if (rc == -1) { + goto exitagent; + } else if (rc >= 0) { nfs = rc; } } -- 2.24.1
On a Monday in 2020, Peter Krempa wrote:
Don't report cases when the guest information is not requested explicitly and not present either.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_driver.c | 45 +++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 25 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa