[libvirt] [PATCH 5/6] admin: Introduce adminDaemonConnectListServers API

Martin Kletzander mkletzan at redhat.com
Tue Aug 11 08:46:18 UTC 2015


On Tue, Aug 11, 2015 at 09:59:00AM +0200, Erik Skultety wrote:
>This is the first API to the admin interface. This particular API is
>a convenience API, i.e. when managing clients connected to daemon's servers,
>we should know (convenience) which server the specific client is connected to.
>This implies a client-side representation of a server along with a basic
>API to let the administrating client know what servers are actually
>available on the daemon.
>---
> daemon/Makefile.am              |   2 +-
> daemon/admin.c                  |  57 +++++++++++++
> daemon/admin_server.c           |  83 +++++++++++++++++++
> daemon/admin_server.h           |  33 ++++++++
> include/libvirt/libvirt-admin.h |  12 +++
> src/admin/admin_protocol.x      |  27 ++++++-
> src/datatypes.c                 |  35 ++++++++
> src/datatypes.h                 |  35 ++++++++
> src/libvirt-admin.c             | 173 ++++++++++++++++++++++++++++++++++++++++
> src/libvirt_admin.syms          |   4 +
> src/rpc/virnetdaemon.c          |  15 ++++
> src/rpc/virnetdaemon.h          |   2 +
> src/rpc/virnetserver.c          |  24 ++++++
> src/rpc/virnetserver.h          |   3 +
> 14 files changed, 503 insertions(+), 2 deletions(-)
> create mode 100644 daemon/admin_server.c
> create mode 100644 daemon/admin_server.h
>

I would probably split the introduction of the virAdmServer structure
into separate patch.  You also forgot to update the
admin_protocol-structs, I guess.

>diff --git a/daemon/Makefile.am b/daemon/Makefile.am
>index 5637f5a..d4dd4ac 100644
>--- a/daemon/Makefile.am
>+++ b/daemon/Makefile.am
>@@ -128,7 +128,7 @@ libvirtd_conf_la_LIBADD = $(LIBXML_LIBS)
>
> noinst_LTLIBRARIES += libvirtd_admin.la
> libvirtd_admin_la_SOURCES = \
>-		admin.c admin.h
>+		admin.c admin.h admin_server.c admin_server.h
>
> libvirtd_admin_la_CFLAGS = \
> 		$(AM_CFLAGS)		\
>diff --git a/daemon/admin.c b/daemon/admin.c
>index ef404a9..d765231 100644
>--- a/daemon/admin.c
>+++ b/daemon/admin.c
>@@ -28,6 +28,7 @@
>
> #include "admin_protocol.h"
> #include "admin.h"
>+#include "admin_server.h"
> #include "datatypes.h"
> #include "viralloc.h"
> #include "virerror.h"
>@@ -77,6 +78,16 @@ remoteAdmClientInitHook(virNetServerClientPtr client ATTRIBUTE_UNUSED,
>     return priv;
> }
>
>+/* Helpers */
>+
>+static void
>+make_nonnull_server(admin_nonnull_server *srv_dst,
>+                                virAdmServerPtr srv_src)

Indentation's off.

>+{
>+    srv_dst->id = srv_src->id;
>+    ignore_value(VIR_STRDUP_QUIET(srv_dst->name, srv_src->name));
>+}
>+
> /* Functions */
> static int
> adminDispatchConnectOpen(virNetServerPtr server ATTRIBUTE_UNUSED,
>@@ -114,4 +125,50 @@ adminDispatchConnectClose(virNetServerPtr server ATTRIBUTE_UNUSED,
>     return 0;
> }
>
>+
>+static int
>+adminDispatchConnectListServers(virNetServerPtr server ATTRIBUTE_UNUSED,
>+                                virNetServerClientPtr client,
>+                                virNetMessagePtr msg ATTRIBUTE_UNUSED,
>+                                virNetMessageErrorPtr rerr ATTRIBUTE_UNUSED,
>+                                admin_connect_list_servers_args *args,
>+                                admin_connect_list_servers_ret *ret)
>+{
>+    virAdmServerPtr *servers = NULL;
>+    int nservers = 0;
>+    int rv = -1;
>+    size_t i;
>+    struct daemonAdmClientPrivate *priv =
>+        virNetServerClientGetPrivateData(client);
>+
>+    if ((nservers =
>+            adminDaemonConnectListServers(priv->dmn,
>+                                          args->need_results ? &servers : NULL,
>+                                          args->flags)) < 0)
>+        goto cleanup;
>+
>+    if (servers && nservers) {
>+        if (VIR_ALLOC_N(ret->servers.servers_val, nservers) < 0)
>+            goto cleanup;
>+
>+        ret->servers.servers_len = nservers;
>+        for (i = 0; i < nservers; i++)
>+            make_nonnull_server(ret->servers.servers_val + i, servers[i]);
>+    } else {
>+        ret->servers.servers_len = 0;
>+        ret->servers.servers_val = NULL;
>+    }
>+
>+    ret->ret = nservers;
>+    rv = 0;
>+
>+ cleanup:
>+    if (rv < 0)
>+        virNetMessageSaveError(rerr);
>+    if (servers && nservers > 0)
>+        for (i = 0; i < nservers; i++)
>+            virObjectUnref(servers[i]);
>+    VIR_FREE(servers);
>+    return rv;
>+}
> #include "admin_dispatch.h"
>diff --git a/daemon/admin_server.c b/daemon/admin_server.c
>new file mode 100644
>index 0000000..9af94ee
>--- /dev/null
>+++ b/daemon/admin_server.c
>@@ -0,0 +1,83 @@
>+/*
>+ * admin_server.c: admin methods to manage daemons and clients
>+ *
>+ * Copyright (C) 2014-2015 Red Hat, Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library.  If not, see
>+ * <http://www.gnu.org/licenses/>.
>+ *
>+ * Author: Martin Kletzander <mkletzan at redhat.com>
>+ */
>+
>+#include <config.h>
>+
>+#include "admin_server.h"
>+#include "datatypes.h"
>+#include "viralloc.h"
>+#include "virerror.h"
>+#include "virlog.h"
>+#include "virnetdaemon.h"
>+#include "virnetserver.h"
>+#include "virstring.h"
>+
>+#define VIR_FROM_THIS VIR_FROM_ADMIN
>+
>+VIR_LOG_INIT("daemon.admin_server");
>+
>+int
>+adminDaemonConnectListServers(virNetDaemonPtr dmn,
>+                              virAdmServerPtr **servers,
>+                              unsigned int flags)
>+{
>+    int ret = -1;
>+    unsigned int id;
>+    const char *name = NULL;
>+    virNetServerPtr *srv_objs = NULL;
>+    virAdmServerPtr *srvs = NULL;
>+    size_t i;
>+    size_t nsrvs = 0;
>+
>+    virCheckFlags(0, -1);
>+
>+    nsrvs = virNetDaemonGetServers(dmn, &srv_objs);
>+    if (servers) {
>+        if (VIR_ALLOC_N(srvs, nsrvs + 1) < 0)
>+            goto cleanup;
>+
>+        for (i = 0; i < nsrvs; i++) {
>+            virNetServerPtr srv = srv_objs[i];
>+
>+            name = virNetServerGetName(srv);
>+            id = virNetServerGetID(srv);
>+
>+            virObjectLock(srv);
>+            if (!(srvs[i] = virAdmGetServer(NULL, name))) {
>+                virObjectUnlock(srv);
>+                goto cleanup;
>+            }
>+
>+            srvs[i]->id = id;
>+            virObjectUnlock(srv);
>+        }
>+
>+        *servers = srvs;
>+        srvs = NULL;
>+    }
>+
>+    ret = nsrvs;
>+
>+ cleanup:
>+    virObjectListFree(srvs);
>+    return ret;
>+}
>diff --git a/daemon/admin_server.h b/daemon/admin_server.h
>new file mode 100644
>index 0000000..aa8f060
>--- /dev/null
>+++ b/daemon/admin_server.h
>@@ -0,0 +1,33 @@
>+/*
>+ * admin_server.h: admin methods to manage daemons and clients
>+ *
>+ * Copyright (C) 2015 Red Hat, Inc.
>+ *
>+ * This library is free software; you can redistribute it and/or
>+ * modify it under the terms of the GNU Lesser General Public
>+ * License as published by the Free Software Foundation; either
>+ * version 2.1 of the License, or (at your option) any later version.
>+ *
>+ * This library is distributed in the hope that it will be useful,
>+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
>+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>+ * Lesser General Public License for more details.
>+ *
>+ * You should have received a copy of the GNU Lesser General Public
>+ * License along with this library.  If not, see
>+ * <http://www.gnu.org/licenses/>.
>+ *
>+ * Author: Martin Kletzander <mkletzan at redhat.com>
>+ */
>+
>+#ifndef __LIBVIRTD_ADMIN_SERVER_H__
>+# define __LIBVIRTD_ADMIN_SERVER_H__
>+
>+# include "rpc/virnetdaemon.h"
>+
>+int
>+adminDaemonConnectListServers(virNetDaemonPtr dmn,
>+                              virAdmServerPtr **servers,
>+                              unsigned int flags);
>+
>+#endif /* __LIBVIRTD_ADMIN_SERVER_H__ */
>diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
>index 9997cc2..83e73ca 100644
>--- a/include/libvirt/libvirt-admin.h
>+++ b/include/libvirt/libvirt-admin.h
>@@ -39,6 +39,8 @@ extern "C" {
>  */
> typedef struct _virAdmConnect virAdmConnect;
>
>+typedef struct _virAdmServer virAdmServer;
>+
> /**
>  * virAdmConnectPtr:
>  *
>@@ -48,11 +50,21 @@ typedef struct _virAdmConnect virAdmConnect;
>  */
> typedef virAdmConnect *virAdmConnectPtr;
>
>+typedef virAdmServer *virAdmServerPtr;
>+
> virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags);
> int virAdmConnectClose(virAdmConnectPtr conn);
>
>+int virAdmConnectListServers(virAdmConnectPtr conn,
>+                             virAdmServerPtr **servers,
>+                             unsigned int flags);
>+
>+int virAdmServerFree(virAdmServerPtr srv);
> int virAdmConnectRef(virAdmConnectPtr conn);
>
>+unsigned int virAdmGetServerID(virAdmServerPtr srv);
>+const char *virAdmGetServerName(virAdmServerPtr srv);
>+
> # ifdef __cplusplus
> }
> # endif
>diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
>index cfc92ff..05b31ea 100644
>--- a/src/admin/admin_protocol.x
>+++ b/src/admin/admin_protocol.x
>@@ -30,17 +30,36 @@
>  */
> const ADMIN_STRING_MAX = 4194304;
>
>+/* Upper limit on list of servers */
>+const ADMIN_SERVER_LIST_MAX = 16384;
>+
> /* A long string, which may NOT be NULL. */
> typedef string admin_nonnull_string<ADMIN_STRING_MAX>;
>
> /* A long string, which may be NULL. */
> typedef admin_nonnull_string *admin_string;
>
>+/* A server which may NOT be NULL */
>+struct admin_nonnull_server {
>+    admin_nonnull_string name;
>+    unsigned hyper id;
>+};
>+
> /*----- Protocol. -----*/
> struct admin_connect_open_args {
>     unsigned int flags;
> };
>
>+struct admin_connect_list_servers_args {
>+    unsigned int need_results;
>+    unsigned int flags;
>+};
>+
>+struct admin_connect_list_servers_ret {
>+    admin_nonnull_server servers<ADMIN_SERVER_LIST_MAX>;
>+    unsigned int ret;
>+};
>+
> /* Define the program number, protocol version and procedure numbers here. */
> const ADMIN_PROGRAM = 0x06900690;
> const ADMIN_PROTOCOL_VERSION = 1;
>@@ -71,5 +90,11 @@ enum admin_procedure {
>     /**
>      * @generate: client
>      */
>-    ADMIN_PROC_CONNECT_CLOSE = 2
>+    ADMIN_PROC_CONNECT_CLOSE = 2,
>+
>+    /**
>+      * @generate: none
>+      * @priority: high
>+      */
>+    ADMIN_PROC_CONNECT_LIST_SERVERS = 3
> };
>diff --git a/src/datatypes.c b/src/datatypes.c
>index 12bcfc1..98d0661 100644
>--- a/src/datatypes.c
>+++ b/src/datatypes.c
>@@ -60,8 +60,10 @@ static void virStorageVolDispose(void *obj);
> static void virStoragePoolDispose(void *obj);
>
> virClassPtr virAdmConnectClass;
>+virClassPtr virAdmServerClass;
>
> static void virAdmConnectDispose(void *obj);
>+static void virAdmServerDispose(void *obj);
>
> static int
> virDataTypesOnceInit(void)
>@@ -90,6 +92,7 @@ virDataTypesOnceInit(void)
>     DECLARE_CLASS(virStorageVol);
>     DECLARE_CLASS(virStoragePool);
>
>+    DECLARE_CLASS(virAdmServer);
>     DECLARE_CLASS_LOCKABLE(virAdmConnect);
>
> #undef DECLARE_CLASS_COMMON
>@@ -833,3 +836,35 @@ virAdmConnectDispose(void *obj)
>     if (conn->privateDataFreeFunc)
>         conn->privateDataFreeFunc(conn);
> }
>+
>+virAdmServerPtr
>+virAdmGetServer(virAdmConnectPtr conn, const char *name)
>+{
>+    virAdmServerPtr ret = NULL;
>+
>+    if (virDataTypesInitialize() < 0)
>+        goto error;
>+
>+    if (!(ret = virObjectNew(virAdmServerClass)))
>+        goto error;
>+    if (VIR_STRDUP(ret->name, name) < 0)
>+        goto error;
>+
>+    ret->conn = virObjectRef(conn);
>+    ret->id = -1;
>+
>+    return ret;
>+ error:
>+    virObjectUnref(ret);
>+    return NULL;
>+}
>+
>+static void
>+virAdmServerDispose(void *obj)
>+{
>+    virAdmServerPtr srv = obj;
>+    VIR_DEBUG("release server %p %d", srv, srv->id);
>+
>+    VIR_FREE(srv->name);
>+    virObjectUnref(srv->conn);
>+}
>diff --git a/src/datatypes.h b/src/datatypes.h
>index be108fe..4e0603d 100644
>--- a/src/datatypes.h
>+++ b/src/datatypes.h
>@@ -42,6 +42,7 @@ extern virClassPtr virStorageVolClass;
> extern virClassPtr virStoragePoolClass;
>
> extern virClassPtr virAdmConnectClass;
>+extern virClassPtr virAdmServerClass;
>
> # define virCheckConnectReturn(obj, retval)                             \
>     do {                                                                \
>@@ -317,6 +318,26 @@ extern virClassPtr virAdmConnectClass;
>         }                                                               \
>     } while (0)
>
>+# define virCheckAdmServerReturn(obj, retval)                           \
>+    do {                                                                \
>+        if (!virObjectIsClass(obj, virAdmServerClass)) {                \
>+            virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN,   \
>+                                 __FILE__, __FUNCTION__, __LINE__,      \
>+                                 __FUNCTION__);                         \
>+            virDispatchError(NULL);                                     \
>+            return retval;                                              \
>+        }                                                               \
>+    } while (0)
>+# define virCheckAdmServerGoto(obj, label)                              \
>+    do {                                                                \
>+        if (!virObjectIsClass(obj, virAdmServerClass)) {                \
>+            virReportErrorHelper(VIR_FROM_THIS, VIR_ERR_INVALID_CONN,   \
>+                                 __FILE__, __FUNCTION__, __LINE__,      \
>+                                 __FUNCTION__);                         \
>+            goto label;                                                 \
>+        }                                                               \
>+    } while (0);
>+
> /**
>  * VIR_DOMAIN_DEBUG:
>  * @dom: domain
>@@ -402,6 +423,18 @@ struct _virAdmConnect {
>     virFreeCallback privateDataFreeFunc;
> };
>
>+/**
>+ * _virAdmServer:
>+ *
>+ * Internal structure associated to a daemon server
>+ */
>+struct _virAdmServer {
>+    virObject object;
>+    virAdmConnectPtr conn;          /* pointer back to the admin connection */
>+    char *name;                     /* the server external name */
>+    unsigned int id;                /* the server unique ID */
>+};
>+
>
> /**
> * _virDomain:
>@@ -586,4 +619,6 @@ virDomainSnapshotPtr virGetDomainSnapshot(virDomainPtr domain,
>
> virAdmConnectPtr virAdmConnectNew(void);
>
>+virAdmServerPtr virAdmGetServer(virAdmConnectPtr conn,
>+                                const char *name);
> #endif /* __VIR_DATATYPES_H__ */
>diff --git a/src/libvirt-admin.c b/src/libvirt-admin.c
>index b3fd0b3..c8ea877 100644
>--- a/src/libvirt-admin.c
>+++ b/src/libvirt-admin.c
>@@ -123,6 +123,18 @@ call(virAdmConnectPtr conn,
> #include "admin_protocol.h"
> #include "admin_client.h"
>
>+/* Helpers */
>+static virAdmServerPtr
>+get_nonnull_server(virAdmConnectPtr conn, admin_nonnull_server server)
>+{
>+    virAdmServerPtr srv;
>+
>+    srv = virAdmGetServer(conn, server.name);
>+    if (srv)
>+        srv->id = server.id;

I wonder why you set the name in the virAdmGetServer() function but id
outside of that function.  Is there any reason for that which I missed?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150811/d3c41bad/attachment-0001.sig>


More information about the libvir-list mailing list