diff mbox

[RFC,v4,04/18] virtagent: base RPC client definitions

Message ID 1289923320-5638-5-git-send-email-mdroth@linux.vnet.ibm.com
State New
Headers show

Commit Message

Michael Roth Nov. 16, 2010, 4:01 p.m. UTC
Base skeleton and helpers for executing RPC commands. Monitor commands
will result in a connect() being issued to the virtagent service socket,
which will then be transported to the listening RPC server in the guest
via the virtproxy layer, RPC requests are then sent/recieved via http
over the resulting connection.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 monitor.c   |    1 +
 qerror.c    |    4 +
 qerror.h    |    3 +
 virtagent.c |  237 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 virtagent.h |   27 +++++++
 5 files changed, 272 insertions(+), 0 deletions(-)
 create mode 100644 virtagent.c
 create mode 100644 virtagent.h

Comments

Jes Sorensen Nov. 18, 2010, 2:10 p.m. UTC | #1
On 11/16/10 17:01, Michael Roth wrote:
> diff --git a/monitor.c b/monitor.c
> index 8cee35d..cb81cd7 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -42,6 +42,7 @@
>  #include "audio/audio.h"
>  #include "disas.h"
>  #include "balloon.h"
> +#include "virtagent.h"
>  #include "qemu-timer.h"
>  #include "migration.h"
>  #include "kvm.h"

You are adding an include here without modifying any code to actually
use something from this file.

> +static int va_client_ready(void)
> +{
> +    if (client_state != NULL && client_state->vp != NULL
> +        && client_state->socket_path != NULL) {

Please put the && up on the previous line, makes it easier for a reader
to notice that the next line is part of the first portion.

> +        return 0;
> +    }
> +
> +    return -1;
> +}

-EAGAIN? -EBUSY?

> +static void va_set_capabilities(QList *qlist)
> +{
> +    TRACE("called");
> +
> +    if (client_state == NULL) {
> +        LOG("client is uninitialized, unable to set capabilities");
> +        return;
> +    }

Why no error value returned here?

> +int va_client_init(VPDriver *vp_drv, bool is_host)
> +{
> +    const char *service_id, *path;
> +    QemuOpts *opts;
> +    int fd, ret;
> +
> +    if (client_state) {
> +        LOG("virtagent client already initialized");
> +        return -1;
> +    }

-1 again :(

> +    /* setup listening socket to forward connections over */
> +    opts = qemu_opts_create(qemu_find_opts("net"), "va_client_opts", 0);
> +    qemu_opt_set(opts, "path", path);
> +    fd = unix_listen_opts(opts);
> +    qemu_opts_del(opts);
> +    if (fd < 0) {
> +        LOG("error setting up listening socket");
> +        goto out_bad;
> +    }
> +
> +    /* tell virtproxy to forward connections to this socket to
> +     * virtagent service on other end
> +     */
> +    ret = vp_set_oforward(vp_drv, fd, service_id);
> +    if (ret < 0) {
> +        LOG("error setting up virtproxy iforward");
> +        goto out_bad;
> +    }
> +
> +    return 0;
> +out_bad:
> +    qemu_free(client_state);
> +    client_state = NULL;
> +    return -1;
> +}

You know why you ended up here, please pass that information up the stack.

> +static int rpc_has_error(xmlrpc_env *env)
> +{
> +    if (env->fault_occurred) {
> +        LOG("An RPC error has occurred (%i): %s\n", env->fault_code, env->fault_string);
> +        //qerror_report(QERR_RPC_FAILED, env->fault_code, env->fault_string);
> +        return -1;
> +    }
> +    return 0;
> +}

-1 again

> +/*
> + * Get a connected socket that can be used to make an RPC call
> + * This interface will eventually return the connected virtproxy socket for the
> + * virt-agent channel
> + */
> +static int get_transport_fd(void)
> +{
> +    /* TODO: eventually this will need a path that is unique to other
> +     * instances of qemu-vp/qemu. for the integrated qemu-vp we should
> +     * explore the possiblity of not requiring a unix socket under the
> +     * covers, as well as having client init code set up the oforward
> +     * for the service rather than qemu-vp
> +     */
> +    int ret;
> +    int fd = unix_connect(client_state->socket_path);
> +    if (fd < 0) {
> +        LOG("failed to connect to virtagent service");
> +    }
> +    ret = fcntl(fd, F_GETFL);
> +    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
> +    return fd;
> +}

You are forgetting to check the return from fcntl() for errors.

> +static int rpc_execute(xmlrpc_env *const env, const char *function,
> +                       xmlrpc_value *params, VARPCData *rpc_data)
> +{
> +    xmlrpc_mem_block *call_xml;
> +    int fd, ret;
> +
> +    ret = va_client_ready();
> +    if (ret < 0) {
> +        LOG("client in uninitialized state, unable to execute RPC");
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    if (!va_has_capability(function)) {
> +        LOG("guest agent does not have required capability");
> +        ret = -1;
> +        goto out;
> +    }
> +
> +    fd = get_transport_fd();
> +    if (fd < 0) {
> +        LOG("invalid fd");
> +        ret = -1;
> +        goto out;
> +    }

You just got a proper error value back, why replace it with -1? It is
all over this function.

> diff --git a/virtagent.h b/virtagent.h
> new file mode 100644
> index 0000000..53efa29
> --- /dev/null
> +++ b/virtagent.h
> +#define GUEST_AGENT_PATH_CLIENT "/tmp/virtagent-guest-client.sock"
> +#define HOST_AGENT_PATH_CLIENT "/tmp/virtagent-host-client.sock"
> +#define VA_MAX_CHUNK_SIZE 4096 /* max bytes at a time for get/send file */

Config file please!

Jes
diff mbox

Patch

diff --git a/monitor.c b/monitor.c
index 8cee35d..cb81cd7 100644
--- a/monitor.c
+++ b/monitor.c
@@ -42,6 +42,7 @@ 
 #include "audio/audio.h"
 #include "disas.h"
 #include "balloon.h"
+#include "virtagent.h"
 #include "qemu-timer.h"
 #include "migration.h"
 #include "kvm.h"
diff --git a/qerror.c b/qerror.c
index ac2cdaf..2f111a9 100644
--- a/qerror.c
+++ b/qerror.c
@@ -200,6 +200,10 @@  static const QErrorStringTable qerror_table[] = {
         .error_fmt = QERR_VNC_SERVER_FAILED,
         .desc      = "Could not start VNC server on %(target)",
     },
+    {
+        .error_fmt = QERR_RPC_FAILED,
+        .desc      = "An RPC error has occurred",
+    },
     {}
 };
 
diff --git a/qerror.h b/qerror.h
index 943a24b..43cce4a 100644
--- a/qerror.h
+++ b/qerror.h
@@ -165,4 +165,7 @@  QError *qobject_to_qerror(const QObject *obj);
 #define QERR_VNC_SERVER_FAILED \
     "{ 'class': 'VNCServerFailed', 'data': { 'target': %s } }"
 
+#define QERR_RPC_FAILED \
+    "{ 'class': 'RPCFailed', 'data': { 'code': %i, 'message': %s } }"
+
 #endif /* QERROR_H */
diff --git a/virtagent.c b/virtagent.c
new file mode 100644
index 0000000..750c167
--- /dev/null
+++ b/virtagent.c
@@ -0,0 +1,237 @@ 
+/*
+ * virt-agent - host/guest RPC client functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Adam Litke        <aglitke@linux.vnet.ibm.com>
+ *  Michael Roth      <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu_socket.h"
+#include "virtagent-daemon.h"
+#include "virtagent-common.h"
+#include "virtagent.h"
+
+typedef struct VARPCClientState {
+    VPDriver *vp;
+    const char *socket_path;
+    QList *supported_methods;
+} VARPCClientState;
+
+/* only one virtagent client can exist at a time */
+static VARPCClientState *client_state;
+
+static int va_client_ready(void)
+{
+    if (client_state != NULL && client_state->vp != NULL
+        && client_state->socket_path != NULL) {
+        return 0;
+    }
+
+    return -1;
+}
+
+static void va_set_capabilities(QList *qlist)
+{
+    TRACE("called");
+
+    if (client_state == NULL) {
+        LOG("client is uninitialized, unable to set capabilities");
+        return;
+    }
+
+    if (client_state->supported_methods != NULL) {
+        qobject_decref(QOBJECT(client_state->supported_methods));
+        client_state->supported_methods = NULL;
+        TRACE("capabilities reset");
+    }
+
+    if (qlist != NULL) {
+        client_state->supported_methods = qlist_copy(qlist);
+        TRACE("capabilities set");
+    }
+}
+
+typedef struct VACmpState {
+    const char *method;
+    bool found;
+} VACmpState;
+
+static void va_cmp_capability_iter(QObject *obj, void *opaque)
+{
+    QString *method = qobject_to_qstring(obj);
+    const char *method_str = NULL;
+    VACmpState *cmp_state = opaque;
+
+    if (method) {
+        method_str = qstring_get_str(method);
+    }
+
+    if (method_str && opaque) {
+        if (strcmp(method_str, cmp_state->method) == 0) {
+            cmp_state->found = 1;
+        }
+    }
+}
+
+static bool va_has_capability(const char *method)
+{
+    VACmpState cmp_state;
+
+    if (method == NULL) {
+        return false;
+    }
+
+    /* we can assume method introspection is available */
+    if (strcmp(method, "system.listMethods") == 0) {
+        return true;
+    }
+
+    /* compare method against the last retrieved supported method list */
+    cmp_state.method = method;
+    cmp_state.found = false;
+    if (client_state->supported_methods) {
+        qlist_iter(client_state->supported_methods,
+                   va_cmp_capability_iter,
+                   (void *)&cmp_state);
+    }
+
+    return cmp_state.found;
+}
+
+int va_client_init(VPDriver *vp_drv, bool is_host)
+{
+    const char *service_id, *path;
+    QemuOpts *opts;
+    int fd, ret;
+
+    if (client_state) {
+        LOG("virtagent client already initialized");
+        return -1;
+    }
+    client_state = qemu_mallocz(sizeof(VARPCClientState));
+    client_state->vp = vp_drv;
+
+    /* setup oforwards to connect to to send RPCs. if we're the host, we
+     * want to connect to the guest agent service using the guest agent
+     * client path, and vice-versa */
+    service_id = !is_host ? HOST_AGENT_SERVICE_ID : GUEST_AGENT_SERVICE_ID;
+    /* TODO: host agent path needs to be made unique amongst multiple
+     * qemu instances
+     */
+    path = !is_host ? HOST_AGENT_PATH_CLIENT : GUEST_AGENT_PATH_CLIENT;
+    client_state->socket_path = path;
+
+    /* setup listening socket to forward connections over */
+    opts = qemu_opts_create(qemu_find_opts("net"), "va_client_opts", 0);
+    qemu_opt_set(opts, "path", path);
+    fd = unix_listen_opts(opts);
+    qemu_opts_del(opts);
+    if (fd < 0) {
+        LOG("error setting up listening socket");
+        goto out_bad;
+    }
+
+    /* tell virtproxy to forward connections to this socket to
+     * virtagent service on other end
+     */
+    ret = vp_set_oforward(vp_drv, fd, service_id);
+    if (ret < 0) {
+        LOG("error setting up virtproxy iforward");
+        goto out_bad;
+    }
+
+    return 0;
+out_bad:
+    qemu_free(client_state);
+    client_state = NULL;
+    return -1;
+}
+
+static int rpc_has_error(xmlrpc_env *env)
+{
+    if (env->fault_occurred) {
+        LOG("An RPC error has occurred (%i): %s\n", env->fault_code, env->fault_string);
+        //qerror_report(QERR_RPC_FAILED, env->fault_code, env->fault_string);
+        return -1;
+    }
+    return 0;
+}
+
+/*
+ * Get a connected socket that can be used to make an RPC call
+ * This interface will eventually return the connected virtproxy socket for the
+ * virt-agent channel
+ */
+static int get_transport_fd(void)
+{
+    /* TODO: eventually this will need a path that is unique to other
+     * instances of qemu-vp/qemu. for the integrated qemu-vp we should
+     * explore the possiblity of not requiring a unix socket under the
+     * covers, as well as having client init code set up the oforward
+     * for the service rather than qemu-vp
+     */
+    int ret;
+    int fd = unix_connect(client_state->socket_path);
+    if (fd < 0) {
+        LOG("failed to connect to virtagent service");
+    }
+    ret = fcntl(fd, F_GETFL);
+    ret = fcntl(fd, F_SETFL, ret | O_NONBLOCK);
+    return fd;
+}
+
+static int rpc_execute(xmlrpc_env *const env, const char *function,
+                       xmlrpc_value *params, VARPCData *rpc_data)
+{
+    xmlrpc_mem_block *call_xml;
+    int fd, ret;
+
+    ret = va_client_ready();
+    if (ret < 0) {
+        LOG("client in uninitialized state, unable to execute RPC");
+        ret = -1;
+        goto out;
+    }
+
+    if (!va_has_capability(function)) {
+        LOG("guest agent does not have required capability");
+        ret = -1;
+        goto out;
+    }
+
+    fd = get_transport_fd();
+    if (fd < 0) {
+        LOG("invalid fd");
+        ret = -1;
+        goto out;
+    }
+
+    call_xml = XMLRPC_MEMBLOCK_NEW(char, env, 0);
+    xmlrpc_serialize_call(env, call_xml, function, params);
+    if (rpc_has_error(env)) {
+        ret = -EREMOTE;
+        goto out_callxml;
+    }
+
+    rpc_data->send_req_xml = call_xml;
+
+    ret = va_rpc_send_request(rpc_data, fd);
+    if (ret != 0) {
+        ret = -1;
+        goto out_callxml;
+    } else {
+        ret = 0;
+        goto out;
+    }
+
+out_callxml:
+    XMLRPC_MEMBLOCK_FREE(char, call_xml);
+out:
+    return ret;
+}
diff --git a/virtagent.h b/virtagent.h
new file mode 100644
index 0000000..53efa29
--- /dev/null
+++ b/virtagent.h
@@ -0,0 +1,27 @@ 
+/*
+ * virt-agent - host/guest RPC client functions
+ *
+ * Copyright IBM Corp. 2010
+ *
+ * Authors:
+ *  Adam Litke        <aglitke@linux.vnet.ibm.com>
+ *  Michael Roth      <mdroth@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef VIRTAGENT_H
+#define VIRTAGENT_H
+
+#include "monitor.h"
+#include "virtagent-common.h"
+
+#define GUEST_AGENT_PATH_CLIENT "/tmp/virtagent-guest-client.sock"
+#define HOST_AGENT_PATH_CLIENT "/tmp/virtagent-host-client.sock"
+#define VA_MAX_CHUNK_SIZE 4096 /* max bytes at a time for get/send file */
+
+int va_client_init(VPDriver *vp_drv, bool is_host);
+
+#endif /* VIRTAGENT_H */