[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 8/9] qemu: Implement virDomainGetGuestInfo()



On 8/23/19 6:31 PM, Jonathon Jongsma wrote:
Iimplements the new guest information API by querying requested
information via the guest agent.

Signed-off-by: Jonathon Jongsma <jjongsma redhat com>
---
  src/qemu/qemu_driver.c | 110 +++++++++++++++++++++++++++++++++++++++++
  1 file changed, 110 insertions(+)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1b051a9424..446266e66b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -23190,6 +23190,115 @@ qemuDomainGetLaunchSecurityInfo(virDomainPtr domain,
      return ret;
  }
+static unsigned int supportedGuestInfoTypes =

This can be const since it's never modified.

+    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)
+{
+    if (*types == 0)
+        *types = supportedGuestInfoTypes;
+
+    *types = *types & supportedGuestInfoTypes;
+}
+
+static int
+qemuDomainGetGuestInfo(virDomainPtr dom,
+                       unsigned int types,
+                       virTypedParameterPtr *params,
+                       int *nparams,
+                       unsigned int flags)
+{
+    virQEMUDriverPtr driver = dom->conn->privateData;
+    virDomainObjPtr vm = NULL;
+    qemuAgentPtr agent;
+    int ret = -1;
+    int rv = -1;

Huh, this @rv and @ret are used a bit misleadinly :-D

+    int maxparams = 0;
+    char *hostname = NULL;

VIR_AUTOFREE()

+    virDomainDefPtr def = NULL;
+    virCapsPtr caps = NULL;
+    unsigned int supportedTypes = types;
+
+    virCheckFlags(0, ret);
+    qemuDomainGetGuestInfoCheckSupport(&supportedTypes);
+
+    if (!(vm = qemuDomObjFromDomain(dom)))
+        goto cleanup;
+
+    if (virDomainGetGuestInfoEnsureACL(dom->conn, vm->def) < 0)
+        goto cleanup;
+
+    if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_QUERY) < 0)
+        goto cleanup;
+
+    if (!qemuDomainAgentAvailable(vm, true))
+        goto endjob;
+
+    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. */
+    if (supportedTypes & VIR_DOMAIN_GUEST_INFO_USERS) {
+        if (qemuAgentGetUsers(agent, params, nparams, &maxparams) < 0 &&

This might not do what you think it does. qemuAgentGetUsers() (and the rest of qemuAgentXXX() for that matter) can fail because of variety of reasons. We want to continue if and only if command was not found and halt in all other cases. But I'll leave this as is for now and probably fix this in a follow up patch. Unless you want to work on that. If you do, then you might want to take a look how we do that in qemu monitor code:

  qemuMonitorJSONHasError(reply, "CommandNotFound"))

+            types != 0)
+            goto exitagent;
+    }
+    if (supportedTypes & VIR_DOMAIN_GUEST_INFO_OS) {
+        if (qemuAgentGetOSInfo(agent, params, nparams, &maxparams) < 0
+            && types != 0)
+            goto exitagent;
+    }
+    if (supportedTypes & VIR_DOMAIN_GUEST_INFO_TIMEZONE) {
+        if (qemuAgentGetTimezone(agent, params, nparams, &maxparams) < 0
+            && types != 0)
+            goto exitagent;
+    }
+    if (supportedTypes & VIR_DOMAIN_GUEST_INFO_HOSTNAME) {
+        if (qemuAgentGetHostname(agent, &hostname) < 0) {
+            if (types != 0)
+                goto exitagent;
+        } else {
+            if (virTypedParamsAddString(params, nparams, &maxparams, "hostname",
+                                        hostname) < 0)
+                goto exitagent;
+        }
+    }
+    if (supportedTypes & VIR_DOMAIN_GUEST_INFO_FILESYSTEM) {
+        if (!(caps = virQEMUDriverGetCapabilities(driver, false)))
+            goto exitagent;
+
+        if (!(def = virDomainDefCopy(vm->def, caps, driver->xmlopt, NULL, false)))
+            goto exitagent;
+

No need to create the copy of the domain def (which is really expensive).

+        if (qemuAgentGetFSInfoParams(agent, params, nparams, &maxparams, def) < 0 &&
+            types != 0)
+            goto exitagent;
+    }
+
+    rv = 0;
+
+ exitagent:
+    qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+    qemuDomainObjEndAgentJob(vm);
+
+ cleanup:
+    virDomainObjEndAPI(&vm);
+    virDomainDefFree(def);
+    virObjectUnref(caps);
+    VIR_FREE(hostname);
+    return rv;
+}
+
  static virHypervisorDriver qemuHypervisorDriver = {
      .name = QEMU_DRIVER_NAME,
      .connectURIProbe = qemuConnectURIProbe,
@@ -23425,6 +23534,7 @@ static virHypervisorDriver qemuHypervisorDriver = {
      .domainCheckpointLookupByName = qemuDomainCheckpointLookupByName, /* 5.6.0 */
      .domainCheckpointGetParent = qemuDomainCheckpointGetParent, /* 5.6.0 */
      .domainCheckpointDelete = qemuDomainCheckpointDelete, /* 5.6.0 */
+    .domainGetGuestInfo = qemuDomainGetGuestInfo, /* 5.6.0 */

s/6/7/

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]