[libvirt] [PATCH v3 4/6] admin: Introduce virAdmServer structure

[Martin Kletzander]

On Mon, Nov 30, 2015 at 04:03:03PM +0100, 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      |  8 ++++++++
> src/datatypes.c                 | 36 ++++++++++++++++++++++++++++++++++++
> src/datatypes.h                 | 34 ++++++++++++++++++++++++++++++++++
> src/libvirt_admin_private.syms  |  3 +++
> 5 files changed, 85 insertions(+)
>
>diff --git a/include/libvirt/libvirt-admin.h b/include/libvirt/libvirt-admin.h
>index ab9df96..23420c7 100644
>--- a/include/libvirt/libvirt-admin.h
>+++ b/include/libvirt/libvirt-admin.h
>@@ -42,6 +42,8 @@ extern "C" {
>  */
> typedef struct _virAdmConnect virAdmConnect;
>
>+typedef struct _virAdmServer virAdmServer;
>+
> /**
>  * virAdmConnectPtr:
>  *
>@@ -51,6 +53,8 @@ typedef struct _virAdmConnect virAdmConnect;
>  */
> typedef virAdmConnect *virAdmConnectPtr;
>
>+typedef virAdmServer *virAdmServerPtr;
>+

Some (even one-line) docstring would be nice since this is a public
header file.

>diff --git a/src/datatypes.c b/src/datatypes.c
>index c832d80..cfc0d06 100644
>--- a/src/datatypes.c
>+++ b/src/datatypes.c
>@@ -61,10 +61,14 @@ static void virStoragePoolDispose(void *obj);
>
> virClassPtr virAdmConnectClass;
> virClassPtr virAdmConnectCloseCallbackDataClass;
>+virClassPtr virAdmServerClass;
>
> static void virAdmConnectDispose(void *obj);
> static void virAdmConnectCloseCallbackDataDispose(void *obj);
>
>+static void virAdmConnectDispose(void *obj);

Duplicated for no reason.

>+static void virAdmServerDispose(void *obj);
>+
> static int
> virDataTypesOnceInit(void)
> {
>@@ -92,6 +96,7 @@ virDataTypesOnceInit(void)
>     DECLARE_CLASS(virStorageVol);
>     DECLARE_CLASS(virStoragePool);
>
>+    DECLARE_CLASS(virAdmServer);
>     DECLARE_CLASS_LOCKABLE(virAdmConnect);
>     DECLARE_CLASS_LOCKABLE(virAdmConnectCloseCallbackData);
>
>@@ -859,3 +864,34 @@ virAdmConnectCloseCallbackDataDispose(void *obj)
>
>     virObjectUnlock(cb_data);
> }
>+
>+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);
>+
>+    return ret;
>+ error:
>+    virObjectUnref(ret);
>+    return NULL;
>+}
>+
>+static void
>+virAdmServerDispose(void *obj)
>+{
>+    virAdmServerPtr srv = obj;
>+    VIR_DEBUG("release server %p %s", srv, srv->name);
>+

Some labels for the values would be nice, at least single quotes.

>+    VIR_FREE(srv->name);
>+    virObjectUnref(srv->conn);
>+}
>diff --git a/src/datatypes.h b/src/datatypes.h
>index 1b1777d..fc84ac6 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)) {                \

You should check that obj->conn is virAdmConnectClass

>+            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)) {                \

same here

But the thing I was wondering the most is, how would you call a public
function that will return virAdmServerPtr based on its name?  I hope
it's not virAdmGetServerFlags() =D  Anyway, that's just a joke.

ACK with those aforementioned things fixed.
-------------- 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/20160110/46861018/attachment-0001.sig>