Re: [PATCH v3 2/9] remote: implement virDomainGetGuestInfo

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

 



On 8/23/19 6:31 PM, Jonathon Jongsma wrote:
Add daemon and client code to serialize/deserialize
virDomainGetGuestInfo().

Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
---
  src/remote/remote_daemon_dispatch.c | 41 ++++++++++++++++++++++
  src/remote/remote_driver.c          | 53 +++++++++++++++++++++++++++++
  src/remote/remote_protocol.x        | 21 +++++++++++-
  src/remote_protocol-structs         | 12 +++++++
  4 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index 1bd281dd6d..665d938a99 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -7650,3 +7650,44 @@ remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors,
      }
      return -1;
  }
+
+static int
+remoteDispatchDomainGetGuestInfo(virNetServerPtr server ATTRIBUTE_UNUSED,
+                                 virNetServerClientPtr client,
+                                 virNetMessagePtr msg ATTRIBUTE_UNUSED,
+                                 virNetMessageErrorPtr rerr,
+                                 remote_domain_get_guest_info_args *args,
+                                 remote_domain_get_guest_info_ret *ret)
+{
+    int rv = -1;
+    struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);

We prefer remoteGetHypervisorConn() ...


+    virDomainPtr dom = NULL;
+    virTypedParameterPtr params = NULL;
+    int nparams = 0;
+
+    if (!priv->conn) {
+        virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));

.. which also reports an error. So no need to invent a new one.

+        goto cleanup;
+    }
+
+    if (!(dom = get_nonnull_domain(priv->conn, args->dom)))
+        goto cleanup;
+
+    if (virDomainGetGuestInfo(dom, args->types, &params, &nparams, args->flags) < 0)
+        goto cleanup;


1: here

+
+    if (virTypedParamsSerialize(params, nparams,
+                                (virTypedParameterRemotePtr *) &ret->params.params_val,
+                                &ret->params.params_len,
+                                VIR_TYPED_PARAM_STRING_OKAY) < 0)
+        goto cleanup;
+
+    rv = 0;
+
+ cleanup:
+    if (rv < 0)
+        virNetMessageSaveError(rerr);
+    virObjectUnref(dom);

You need to free @params here.

+
+    return rv;
+}
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index daac506672..5ba144648a 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -8306,6 +8306,58 @@ remoteNetworkPortGetParameters(virNetworkPortPtr port,
      return rv;
  }
+static int
+remoteDomainGetGuestInfo(virDomainPtr dom,
+                         unsigned int types,
+                         virTypedParameterPtr *params,
+                         int *nparams,
+                         unsigned int flags)
+{
+    int rv = -1;
+    struct private_data *priv = dom->conn->privateData;
+    remote_domain_get_guest_info_args args;
+    remote_domain_get_guest_info_ret ret;
+
+    remoteDriverLock(priv);
+
+    make_nonnull_domain(&args.dom, dom);
+
+    args.types = types;
+    args.flags = flags;
+
+    memset(&ret, 0, sizeof(ret));
+
+    if (call(dom->conn, priv, 0, REMOTE_PROC_DOMAIN_GET_GUEST_INFO,
+             (xdrproc_t)xdr_remote_domain_get_guest_info_args, (char *)&args,
+             (xdrproc_t)xdr_remote_domain_get_guest_info_ret, (char *)&ret) == -1)
+        goto done;
+
+    if (ret.params.params_len > REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Too many params in guestinfo: %d for limit %d"),
+                       ret.params.params_len, REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX);
+        goto cleanup;
+    }

No need to check for this here. It's checked for in virTypedParamsDeserialize(). That's why you need to pass _MAX argument. In fact, the check should go to [1].

+
+    if (params) {

The @params can't be NULL here, can they? I mean, in the public API you check for:

  virCheckNonNullArgGoto(params, error);

But I agree that our RPC infrastructure is complicated and it's not easy to understand it at the first glance. Basically, it works like this: at client side on virConnectOpen() the conn->driver is initialized to remote driver (@hypervisor_driver from remote_driver.c). So calling any public API ends up calling the callback from remote_driver.c (remoteDomainGetGuestInfo() in this case). That is the reason why these callbacks in remote driver need to look exactly like the public APIs (and hence you need to create typedef for every public API). In the end, these callbacks do mere arg serialization and reply deserialization.

On server, things are a bit different. We have a code which deserializes incoming RPC packet and then we have this huge array of pairs PROC_NR + dispatch callback (look at the end of src/remote/remote_daemon_dispatch_stubs.h you'll find the array there). Yeah, yeah, it's not list of pairs, rather than tuples, but for our case we can safely ignore the other members of the tuple.

So, after server has decoded the incoming RPC it learned what procedure the client wants to call and thus it calls the associated callback (remoteDispatchDomainGetGuestInfoHelper() in this case). And this callback will then call the public API again, but this time conn->driver points to actual driver (qemu driver from instance). It is only here where all the interesting work is done.

It's more or less documented here:

https://libvirt.org/api.html
https://libvirt.org/internals/rpc.html
https://libvirt.org/api_extension.html (looks like you've followed this one)

Please let me know if you want me to expand on something.

+        if (virTypedParamsDeserialize((virTypedParameterRemotePtr) ret.params.params_val,
+                                      ret.params.params_len,
+                                      REMOTE_DOMAIN_GUEST_INFO_PARAMS_MAX,
+                                      params,
+                                      nparams) < 0)
+            goto cleanup;
+    }
+
+    rv = 0;
+
+ cleanup:
+    xdr_free((xdrproc_t)xdr_remote_domain_get_guest_info_ret,
+             (char *) &ret);
+
+ done:
+    remoteDriverUnlock(priv);
+    return rv;
+}
/* get_nonnull_domain and get_nonnull_network turn an on-wire
   * (name, uuid) pair into virDomainPtr or virNetworkPtr object.
@@ -8733,6 +8785,7 @@ static virHypervisorDriver hypervisor_driver = {
      .domainCheckpointLookupByName = remoteDomainCheckpointLookupByName, /* 5.6.0 */
      .domainCheckpointGetParent = remoteDomainCheckpointGetParent, /* 5.6.0 */
      .domainCheckpointDelete = remoteDomainCheckpointDelete, /* 5.6.0 */
+    .domainGetGuestInfo = remoteDomainGetGuestInfo, /* 5.6.0 */

Ooops, s/6/7/ :D

Michal

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux