Re: [PATCH v2 7/9] admin: Introduce virAdmServer structure

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

 



On Thu, Sep 03, 2015 at 11:26:06AM +0200, Martin Kletzander wrote:
> On Fri, Aug 21, 2015 at 08:04:08PM +0200, Erik Skultety wrote:
> >This is the key structure of all management operations performed on the
> >daemon/clients. An admin client needs to be able to identify
> >another client (either admin or non-privileged client) to perform an
> >action on it. This identification includes a server the client is
> >connected to, thus a client-side representation of a server is needed.
> >---
> >include/libvirt/libvirt-admin.h |  4 ++++
> >src/admin/admin_protocol.x      |  9 +++++++++
> >src/datatypes.c                 | 35 +++++++++++++++++++++++++++++++++++
> >src/datatypes.h                 | 36 ++++++++++++++++++++++++++++++++++++
> >src/libvirt_admin_private.syms  |  5 +++++
> >5 files changed, 89 insertions(+)
> >
> >diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
> >index 9997cc2..f0752f5 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,6 +50,8 @@ typedef struct _virAdmConnect virAdmConnect;
> > */
> >typedef virAdmConnect *virAdmConnectPtr;
> >
> >+typedef virAdmServer *virAdmServerPtr;
> >+
> >virAdmConnectPtr virAdmConnectOpen(const char *name, unsigned int flags);
> >int virAdmConnectClose(virAdmConnectPtr conn);
> >
> >diff --git a/src/admin/admin_protocol.x b/src/admin/admin_protocol.x
> >index cfc92ff..d062e9a 100644
> >--- a/src/admin/admin_protocol.x
> >+++ b/src/admin/admin_protocol.x
> >@@ -30,12 +30,21 @@
> > */
> >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;
> 
> 64 bits is a lot, I think, but no biggie, I feel agnostic to this.
> 
> >+};
> >+
> >/*----- Protocol. -----*/
> >struct admin_connect_open_args {
> >    unsigned int flags;
> >diff --git a/src/datatypes.c b/src/datatypes.c
> >index 12bcfc1..9c61ece 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, const int id)
> >+{
> >+    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 = id;
> >+
> >+    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..1147a7e 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 */
> 
> If it's 64bits though, this needs to change, so at least one of these
> declarations must be changed to match the other.  My agnosticism
> swirls slowly into simple int-preference.

>From a theoretical POV, the number of servers is limited by the
size of the array holding the virAdmServer structs. This is in
turn size_t limited (well in practice its $RAM limited). So I
think we should just use size_t in the struct here, and then
use unsigned hyper in the wire which is big enough for any sizet


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

--
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]