Re: [libvirt] [PATCH v4 2/4] qemu: guestinfo: handle unsupported agent commands

When we're collecting guest information, older agents may not support all agent commands. In the case where the user requested all info types (i.e. types == 0), ignore unsupported command errors and gather as much information as possible. If the agent command failed for some other reason, or if the user explciitly requested a specific info type (i.e. types != 0), abort on the first error. Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com&gt; --- src/qemu/qemu_agent.c | 70 ++++++++++++++++++++++++++++++++++++++---- src/qemu/qemu_driver.c | 33 ++++++++++---------- tests/qemuagenttest.c | 2 +- 3 files changed, 82 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index f2a8bb6263..c63db968c6 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -995,6 +995,26 @@ 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 = 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 * @@ -1708,8 +1728,11 @@ qemuAgentGetHostname(qemuAgentPtr mon, return ret; if (qemuAgentCommand(mon, cmd, &reply, true, - VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) { + if (qemuAgentErrorCommandUnsupported(reply)) + ret = -2; goto cleanup; + } if (!(data = virJSONValueObjectGet(reply, "return"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", @@ -2005,6 +2028,10 @@ qemuAgentGetFSInfoInternalDisk(virJSONValuePtr jsondisks, return 0; } +/* returns 0 on success + * -2 when agent command is not supported by the agent + * -1 otherwise + */

static int qemuAgentGetFSInfoInternal(qemuAgentPtr mon, qemuAgentFSInfoPtr **info,

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d0e67863ea..908460bc79 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -23220,6 +23220,7 @@ qemuDomainGetGuestInfo(virDomainPtr dom, int maxparams = 0; VIR_AUTOFREE(char *) hostname = NULL; unsigned int supportedTypes = types; + int status;

virCheckFlags(0, -1); qemuDomainGetGuestInfoCheckSupport(&supportedTypes); @@ -23240,30 +23241,30 @@ qemuDomainGetGuestInfo(virDomainPtr dom, agent = qemuDomainObjEnterAgent(vm); - /* Although the libvirt qemu driver supports all of these guest info types, - * some guest agents might be too old to support these commands. If these - * info categories were explicitly requested (i.e. 'types' is non-zero), - * abort and report an error on any failures, otherwise continue and return - * as much info as is supported by the guest agent. */ + /* The agent info commands will return -2 for any commands that are not + * supported by the agent, or -1 for all other errors. In the case where no + * categories were explicitly requrested (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) { - if (qemuAgentGetUsers(agent, params, nparams, &maxparams) < 0 && - types != 0) + status = qemuAgentGetUsers(agent, params, nparams, &maxparams); + if (status == -1 || (status == -2 && types != 0))

goto exitagent; } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) { - if (qemuAgentGetOSInfo(agent, params, nparams, &maxparams) < 0 - && types != 0) + status = qemuAgentGetOSInfo(agent, params, nparams, &maxparams); + if (status == -1 || (status == -2 && types != 0)) goto exitagent; } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) { - if (qemuAgentGetTimezone(agent, params, nparams, &maxparams) < 0 - && types != 0) + status = qemuAgentGetTimezone(agent, params, nparams, &maxparams); + if (status == -1 || (status == -2 && types != 0)) goto exitagent; } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) { - if (qemuAgentGetHostname(agent, &hostname) < 0) { - if (types != 0) - goto exitagent; + status = qemuAgentGetHostname(agent, &hostname); + if (status == -1 || (status == -2 && types != 0)) { + goto exitagent; } else { if (virTypedParamsAddString(params, nparams, &maxparams, "hostname", hostname) < 0) @@ -23271,8 +23272,8 @@ qemuDomainGetGuestInfo(virDomainPtr dom, } } if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) { - if (qemuAgentGetFSInfoParams(agent, params, nparams, &maxparams, vm->def) < 0 && - types != 0) + status = qemuAgentGetFSInfoParams(agent, params, nparams, &maxparams, vm->def); + if (status == -1 || (status == -2 && types != 0)) goto exitagent; } diff --git a/tests/qemuagenttest.c b/tests/qemuagenttest.c index ae57da5989..91f19644d5 100644 --- a/tests/qemuagenttest.c +++ b/tests/qemuagenttest.c @@ -462,7 +462,7 @@ testQemuAgentGetFSInfoParams(const void *data) goto cleanup; if (qemuAgentGetFSInfoParams(qemuMonitorTestGetAgent(test), &params, - &nparams, &maxparams, def) != -1) { + &nparams, &maxparams, def) != -2) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", "agent get-fsinfo command should have failed"); goto cleanup;