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

Re: [Libvir] [PATCH] Check availability of driver calls



Daniel Veillard wrote:
On Mon, Aug 20, 2007 at 03:13:34PM +0100, Richard W.M. Jones wrote:
This is a repost of a patch I sent before which adds the ability to check for driver features. This is completely internal to libvirt and does not add any public APIs.

The original discussion is here:
https://www.redhat.com/archives/libvir-list/2007-July/thread.html#00437
and continues here:
https://www.redhat.com/archives/libvir-list/2007-August/thread.html#00000

Daniel Veillard said:
+typedef int
+ (*virDrvSupportsFeature) (virConnectPtr conn, virDrvFeature
feature);
I would rather use , int features) where you OR the features, allows
to know in one call if you get what you want or not.
My objection to that is here:
https://www.redhat.com/archives/libvir-list/2007-August/msg00000.html

  The complexity comes if you think you need to return an array of integer.
I guess if you just consider passing an unsigned long in and back, filled
as an OR'ed bitfiled with the set of features you ask for you do 1 round trip

This doesn't work though, because different parts of the system have to answer different parts of a request such as:

  VIR_DRV_FEATURE_MIGRATION_V1 | VIR_DRV_FEATURE_REMOTE.

VIR_DRV_FEATURE_MIGRATION_V1 goes through to end driver. VIR_DRV_FEATURE_REMOTE is answered by the local end of remote (src/remote_internal.c). My real point is I don't understand the case where it is ever useful to query more than one feature.

you still pas and return an atomic value, you just need a little bit of decoding work at the C level to analyze the bits in the values, but that's
won't be much more complex than a switch based on the enum, and IMHO
more reliable, because I still (sorry) consider enums in APIs to be a big
problem in C. Even if it's considered an internal API, I would avoid enums
there because of the RPC.

I meant to get rid of the enums. Attached is a version which uses macros instead so the type should always be 'int' (ie. ABI stable).

Rich.

--
Emerging Technologies, Red Hat - http://et.redhat.com/~rjones/
Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod
Street, Windsor, Berkshire, SL4 1TE, United Kingdom.  Registered in
England and Wales under Company Registration No. 03798903
Index: qemud/remote.c
===================================================================
RCS file: /data/cvs/libvirt/qemud/remote.c,v
retrieving revision 1.6
diff -u -p -r1.6 remote.c
--- qemud/remote.c	24 Jul 2007 14:21:03 -0000	1.6
+++ qemud/remote.c	20 Aug 2007 15:43:21 -0000
@@ -418,6 +418,18 @@ remoteDispatchClose (struct qemud_client
 }
 
 static int
+remoteDispatchSupportsFeature (struct qemud_client *client, remote_message_header *req,
+                               remote_supports_feature_args *args, remote_supports_feature_ret *ret)
+{
+    CHECK_CONN(client);
+
+    ret->supported = __virDrvSupportsFeature (client->conn, args->feature);
+    if (ret->supported == -1) return -1;
+
+    return 0;
+}
+
+static int
 remoteDispatchGetType (struct qemud_client *client, remote_message_header *req,
                        void *args ATTRIBUTE_UNUSED, remote_get_type_ret *ret)
 {
Index: qemud/remote_protocol.x
===================================================================
RCS file: /data/cvs/libvirt/qemud/remote_protocol.x,v
retrieving revision 1.3
diff -u -p -r1.3 remote_protocol.x
--- qemud/remote_protocol.x	26 Jun 2007 11:42:46 -0000	1.3
+++ qemud/remote_protocol.x	20 Aug 2007 15:43:21 -0000
@@ -170,6 +170,14 @@ struct remote_open_args {
     int flags;
 };
 
+struct remote_supports_feature_args {
+    int feature;
+};
+
+struct remote_supports_feature_ret {
+    int supported;
+};
+
 struct remote_get_type_ret {
     remote_nonnull_string type;
 };
@@ -610,7 +618,8 @@ enum remote_procedure {
     REMOTE_PROC_DOMAIN_GET_SCHEDULER_TYPE = 56,
     REMOTE_PROC_DOMAIN_GET_SCHEDULER_PARAMETERS = 57,
     REMOTE_PROC_DOMAIN_SET_SCHEDULER_PARAMETERS = 58,
-    REMOTE_PROC_GET_HOSTNAME = 59
+    REMOTE_PROC_GET_HOSTNAME = 59,
+    REMOTE_PROC_SUPPORTS_FEATURE = 60
 };
 
 /* Custom RPC structure. */
Index: src/driver.h
===================================================================
RCS file: /data/cvs/libvirt/src/driver.h,v
retrieving revision 1.32
diff -u -p -r1.32 driver.h
--- src/driver.h	27 Jul 2007 23:23:00 -0000	1.32
+++ src/driver.h	20 Aug 2007 15:43:22 -0000
@@ -44,12 +44,42 @@ typedef enum {
     VIR_DRV_OPEN_ERROR = -2,
 } virDrvOpenStatus;
 
+/* Feature detection.  This is a libvirt-private interface for determining
+ * what features are supported by the driver.
+ *
+ * The remote driver passes features through to the real driver at the
+ * remote end unmodified, except if you query a VIR_DRV_FEATURE_REMOTE*
+ * feature.
+ */
+    /* Driver supports V1-style virDomainMigrate, ie. domainMigratePrepare/
+     * domainMigratePerform/domainMigrateFinish.
+     */
+#define VIR_DRV_FEATURE_MIGRATION_V1 1
+
+    /* Driver is not local. */
+#define VIR_DRV_FEATURE_REMOTE 2
+
+/* Internal feature-detection macro.  Don't call drv->supports_feature
+ * directly, because it may be NULL, use this macro instead.
+ *
+ * Note that you must check for errors.
+ *
+ * Returns:
+ *   >= 1  Feature is supported.
+ *   0     Feature is not supported.
+ *   -1    Error.
+ */
+#define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature)                      \
+    ((drv)->supports_feature ? (drv)->supports_feature((conn),(feature)) : 0)
+
 typedef virDrvOpenStatus
 	(*virDrvOpen)			(virConnectPtr conn,
 					 const char *name,
 					 int flags);
 typedef int
 	(*virDrvClose)			(virConnectPtr conn);
+typedef int
+    (*virDrvSupportsFeature) (virConnectPtr conn, int feature);
 typedef const char *
 	(*virDrvGetType)		(virConnectPtr conn);
 typedef int
@@ -202,6 +232,7 @@ struct _virDriver {
 	unsigned long ver;	/* the version of the backend */
 	virDrvOpen			open;
 	virDrvClose			close;
+    virDrvSupportsFeature   supports_feature;
 	virDrvGetType			type;
 	virDrvGetVersion		version;
     virDrvGetHostname       getHostname;
Index: src/internal.h
===================================================================
RCS file: /data/cvs/libvirt/src/internal.h,v
retrieving revision 1.47
diff -u -p -r1.47 internal.h
--- src/internal.h	25 Jul 2007 23:16:30 -0000	1.47
+++ src/internal.h	20 Aug 2007 15:43:22 -0000
@@ -229,6 +229,8 @@ int __virStateActive(void);
 #define virStateReload() __virStateReload()
 #define virStateActive() __virStateActive()
 
+int __virDrvSupportsFeature (virConnectPtr conn, int feature);
+
 #ifdef __cplusplus
 }
 #endif                          /* __cplusplus */
Index: src/libvirt.c
===================================================================
RCS file: /data/cvs/libvirt/src/libvirt.c,v
retrieving revision 1.93
diff -u -p -r1.93 libvirt.c
--- src/libvirt.c	9 Aug 2007 20:19:12 -0000	1.93
+++ src/libvirt.c	20 Aug 2007 15:43:24 -0000
@@ -540,6 +540,20 @@ virConnectClose(virConnectPtr conn)
     return (0);
 }
 
+/* Not for public use.  This function is part of the internal
+ * implementation of driver features in the remote case.
+ */
+int
+__virDrvSupportsFeature (virConnectPtr conn, int feature)
+{
+    DEBUG("conn=%p, feature=%d", conn, feature);
+
+    if (!VIR_IS_CONNECT(conn))
+        return (-1);
+
+    return VIR_DRV_SUPPORTS_FEATURE (conn->driver, conn, feature);
+}
+
 /**
  * virConnectGetType:
  * @conn: pointer to the hypervisor connection
Index: src/libvirt_sym.version
===================================================================
RCS file: /data/cvs/libvirt/src/libvirt_sym.version,v
retrieving revision 1.25
diff -u -p -r1.25 libvirt_sym.version
--- src/libvirt_sym.version	26 Jun 2007 22:56:14 -0000	1.25
+++ src/libvirt_sym.version	20 Aug 2007 15:43:24 -0000
@@ -116,5 +116,7 @@
  	__virStateReload;
  	__virStateActive;
 
+	__virDrvSupportsFeature;
+
     local: *;
 };
Index: src/qemu_driver.c
===================================================================
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.20
diff -u -p -r1.20 qemu_driver.c
--- src/qemu_driver.c	14 Aug 2007 01:47:24 -0000	1.20
+++ src/qemu_driver.c	20 Aug 2007 15:43:25 -0000
@@ -2618,6 +2618,7 @@ static virDriver qemuDriver = {
     LIBVIR_VERSION_NUMBER,
     qemudOpen, /* open */
     qemudClose, /* close */
+    NULL, /* supports_feature */
     qemudGetType, /* type */
     qemudGetVersion, /* version */
     NULL, /* hostname */
Index: src/remote_internal.c
===================================================================
RCS file: /data/cvs/libvirt/src/remote_internal.c,v
retrieving revision 1.18
diff -u -p -r1.18 remote_internal.c
--- src/remote_internal.c	7 Aug 2007 14:29:45 -0000	1.18
+++ src/remote_internal.c	20 Aug 2007 15:43:27 -0000
@@ -1195,6 +1195,27 @@ remoteClose (virConnectPtr conn)
     return ret;
 }
 
+static int
+remoteSupportsFeature (virConnectPtr conn, int feature)
+{
+    remote_supports_feature_args args;
+    remote_supports_feature_ret ret;
+    GET_PRIVATE (conn, -1);
+
+    /* VIR_DRV_FEATURE_REMOTE* features are handled directly. */
+    if (feature == VIR_DRV_FEATURE_REMOTE) return 1;
+
+    args.feature = feature;
+
+    memset (&ret, 0, sizeof ret);
+    if (call (conn, priv, 0, REMOTE_PROC_SUPPORTS_FEATURE,
+              (xdrproc_t) xdr_remote_supports_feature_args, (char *) &args,
+              (xdrproc_t) xdr_remote_supports_feature_ret, (char *) &ret) == -1)
+        return -1;
+
+    return ret.supported;
+}
+
 /* Unfortunately this function is defined to return a static string.
  * Since the remote end always answers with the same type (for a
  * single connection anyway) we cache the type in the connection's
@@ -2898,6 +2919,7 @@ static virDriver driver = {
     .ver = REMOTE_PROTOCOL_VERSION,
     .open = remoteOpen,
     .close = remoteClose,
+    .supports_feature = remoteSupportsFeature,
 	.type = remoteType,
 	.version = remoteVersion,
     .getHostname = remoteGetHostname,
Index: src/test.c
===================================================================
RCS file: /data/cvs/libvirt/src/test.c,v
retrieving revision 1.44
diff -u -p -r1.44 test.c
--- src/test.c	9 Aug 2007 20:19:12 -0000	1.44
+++ src/test.c	20 Aug 2007 15:43:29 -0000
@@ -1920,6 +1920,7 @@ static virDriver testDriver = {
     LIBVIR_VERSION_NUMBER,
     testOpen, /* open */
     testClose, /* close */
+    NULL, /* supports_feature */
     NULL, /* type */
     testGetVersion, /* version */
     testGetHostname, /* hostname */

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature


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