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

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