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, virDrvFeaturefeature);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.htmlThe 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'swon'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
-- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list