4:24 p.m.
On 05/02/2012 05:44 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange" <berrange(a)redhat.com>
A bit sparse on the commit message; even mentioning the term
virIdentityPtr would help future crawls through 'git log' find this patch.
+++ b/include/libvirt/libvirt.h.in
@@ -1088,6 +1088,36 @@ typedef virConnectAuth *virConnectAuthPtr;
VIR_EXPORT_VAR virConnectAuthPtr virConnectAuthPtrDefault;
+typedef struct _virIdentity virIdentity;
+typedef virIdentity *virIdentityPtr;
+
+typedef enum {
+ VIR_IDENTITY_ATTR_UNIX_USER_NAME,
+ VIR_IDENTITY_ATTR_UNIX_GROUP_NAME,
+ VIR_IDENTITY_ATTR_UNIX_PROCESS_ID,
+ VIR_IDENTITY_ATTR_SASL_USER_NAME,
+ VIR_IDENTITY_ATTR_X509_DISTINGUISHED_NAME,
+ VIR_IDENTITY_ATTR_SECURITY_CONTEXT,
+
+ VIR_IDENTITY_ATTR_LAST,
This last value should be guarded by VIR_ENUM_SENTINELS.
+
+virIdentityPtr virIdentityNew(void);
+int virIdentityRef(virIdentityPtr ident);
+int virIdentitySetAttr(virIdentityPtr ident,
+ unsigned int attr,
+ const char *value);
+
+int virIdentityGetAttr(virIdentityPtr ident,
+ unsigned int attr,
+ const char **value);
+
+int virIdentityIsEqual(virIdentityPtr identA,
+ virIdentityPtr identB);
+
+int virIdentityFree(virIdentityPtr ident);
Are there any other useful operations, such as knowing how many
attributes in the identity are currently set?
/**
+ * VIR_IDENTITY_MAGIC:
+ *
+ * magic value used to protect the API when pointers to identity structures
+ * are passed down by the users.
+ */
+# define VIR_IDENTITY_MAGIC 0xB33FCAF3
How do we pick these magic numbers, anyway? :)
+
+struct _virIdentity {
+ unsigned int magic;
+ virMutex lock;
+ int refs;
+
+ char *attrs[VIR_IDENTITY_ATTR_LAST];
+};
It looks like your intent is to store everything in this identity
locally (contrast it to _virDomain, which stores only enough information
locally to pass back over RPC to the remote side and have the remote
side do a hash lookup for the rest of the domain information). It
should be okay, though, especially since this identity does not include
a name or uuid which could be used for a hash lookup for additional
contents, anyway.
+
+/**
+ * virIdentityNew:
+ *
+ * Creates a new empty identity object. After creating, one or
+ * more identifying attributes should be set on the identity.
+ *
+ * Returns: a new empty identity
or NULL on error.
+/**
+ * virIdentityRef:
+ * @ident: the identity to hold a reference on
+ *
+ * Increment the reference count on the identity. For each
+ * additional call to this method, there shall be a corresponding
+ * call to virIdentityFree to release the reference count, once
+ * the caller no longer needs the reference to this object.
+ *
+ * This method is typically useful for applications where multiple
+ * threads are using an identity object, and it is required that
+ * the identity remain around until all threads have finished using
+ * it. ie, each new thread using a identity would increment
It looks weird to start a sentence with "ie,", but I don't have an
alternative wording on the tip of my tongue.
+ * the reference count.
+ *
+ * Returns 0 in case of success and -1 in case of failure.
+ */
+int virIdentityRef(virIdentityPtr ident)
+{
+ if ((!VIR_IS_IDENTITY(ident))) {
+ virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
We really should be using a better error message than __FUNCTION__,
especially since virLibConnError is already adding __FUNCTION__ into the
list of arguments. (Throughout the patch).
+int virIdentitySetAttr(virIdentityPtr ident,
+ unsigned int attr,
+ const char *value)
+{
+ VIR_DEBUG("ident=%p attribute=%u value=%s", ident, attr, NULLSTR(value));
+
+ if ((!VIR_IS_IDENTITY(ident))) {
+ virLibConnError(VIR_ERR_INVALID_CONN, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ if (attr >= VIR_IDENTITY_ATTR_LAST) {
+ virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ if (!value) {
+ virLibConnError(VIR_ERR_INVALID_ARG, __FUNCTION__);
Really? This means I can't clear out a value by passing in NULL.
Should there be a counterpart API for clearing an attribute?
+ virDispatchError(NULL);
+ return -1;
+ }
+
+ virMutexLock(&ident->lock);
+
+ if (ident->attrs[attr]) {
+ virLibConnError(VIR_ERR_OPERATION_DENIED, "%s",
+ _("Identity attribute is already set"));
Then again, this makes it a write-once interface (once an identity
attribute is set, you can't change it). Should we document that better?
+int virIdentityIsEqual(virIdentityPtr identA,
+ virIdentityPtr identB)
+
+ for (i = 0 ; i < VIR_IDENTITY_ATTR_LAST ; i++) {
+ if (identA->attrs[i] == NULL &&
+ identB->attrs[i] != NULL)
+ goto cleanup;
+ if (identA->attrs[i] != NULL &&
+ identB->attrs[i] == NULL)
+ goto cleanup;
+ if (STRNEQ(identA->attrs[i],
+ identB->attrs[i]))
+ goto cleanup;
Use STRNEQ_NULLABLE to shorten this.
+++ b/src/libvirt_public.syms
@@ -527,6 +527,11 @@ LIBVIRT_0.9.10 {
virDomainShutdownFlags;
virStorageVolResize;
virStorageVolWipePattern;
+ virIdentityNew;
+ virIdentityIsEqual;
+ virIdentitySetAttr;
+ virIdentityGetAttr;
+ virIdentityFree;
} LIBVIRT_0.9.9;
Wrong release. This should be in the LIBVIRT_0.9.12 section.
+++ b/src/util/virterror.c
@@ -1259,6 +1259,12 @@ virErrorMsg(virErrorNumber error, const char *info)
else
errmsg = _("block copy still active: %s");
break;
+ case VIR_ERR_INVALID_IDENTITY:
+ if (info == NULL)
+ errmsg = _("invalid identity pointer in");
That reads awkwardly. That says if I call:
reportError(VIR_ERR_INVALID_IDENTITY, NULL);
my error message will be "function: invalid identity pointer in".
+ else
+ errmsg = _("invalid identity pointer in %s");
Then again, if I call
reportError(VIR_ERR_INVALID_IDENTITY, __FUNCTION__);
the error message will be "function: invalid identity pointer in function"
But it's copy and paste from existing practice, so it's no worse than
before.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org