[libvirt] [PATCH v2 7/9] admin: Introduce virAdmServer structure
Martin Kletzander
mkletzan at redhat.com
Thu Sep 3 10:42:43 UTC 2015
On Thu, Sep 03, 2015 at 11:19:02AM +0100, Daniel P. Berrange wrote:
>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
>
On the other hand, we use integer in the list of domains and I am
pretty sure we won't have more than three servers per daemon for a
*long* time. Maybe even two. And we didn't have the problem with
number of domains and there could be *way* more of those. We also
talked about identifying the servers by their unique name, but the id
is added there to keep it consistent with other following structures,
won't hurt and because we can directly access particular server from
the array (even if it is a sparse one). So I think int is *way* more
than needed.
>
>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 :|
-------------- 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/20150903/67445bbf/attachment-0001.sig>
More information about the libvir-list
mailing list