On Fri, Sep 11, 2009 at 03:19:17PM +0100, Daniel P. Berrange wrote: > Convert all the secret/storage encryption APIs / wire format to > handle UUIDs in raw format instead of non-canonical printable > format. Guarentees data format correctness. [...] > +++ b/include/libvirt/libvirt.h > @@ -1467,12 +1467,17 @@ int virConnectNumOfSecrets (virConnectPtr conn); > int virConnectListSecrets (virConnectPtr conn, > char **uuids, > int maxuuids); > +virSecretPtr virSecretLookupByUUID(virConnectPtr conn, > + const unsigned char *uuid); > virSecretPtr virSecretLookupByUUIDString(virConnectPtr conn, > const char *uuid); > virSecretPtr virSecretDefineXML (virConnectPtr conn, > const char *xml, > unsigned int flags); > -char * virSecretGetUUIDString (virSecretPtr secret); > +int virSecretGetUUID (virSecretPtr secret, > + unsigned char *buf); > +int virSecretGetUUIDString (virSecretPtr secret, > + char *buf); > char * virSecretGetXMLDesc (virSecretPtr secret, > unsigned int flags); > int virSecretSetValue (virSecretPtr secret, explicit ACK, this is better ! [..] > @@ -4828,7 +4828,7 @@ get_nonnull_storage_vol (virConnectPtr conn, remote_nonnull_storage_vol vol) > static virSecretPtr > get_nonnull_secret (virConnectPtr conn, remote_nonnull_secret secret) > { > - return virGetSecret (conn, secret.uuid); > + return virGetSecret (conn, BAD_CAST secret.uuid); heh I didn't expect BAD_CAST to be used outside libxml2 :-) [...] > @@ -1179,7 +1181,9 @@ virGetSecret(virConnectPtr conn, const char *uuid) > } > virMutexLock(&conn->lock); > > - ret = virHashLookup(conn->secrets, uuid); > + virUUIDFormat(uuid, uuidstr); > + > + ret = virHashLookup(conn->secrets, uuidstr); Would you mind also removing the Returns 0 in case of success and -1 in case of error. comment for virUUIDFormat() as it's void and doesn't do any checking (it has no way to). [...] > /** > - * virSecretLookupByUUIDString: > - * @conn: virConnect connection > - * @uuid: ID of a secret > + * virSecretLookupByUUID: > + * @conn: pointer to the hypervisor connection > + * @uuid: the raw UUID for the secret > * > - * Fetches a secret based on uuid. > + * Try to lookup a secret on the given hypervisor based on its UUID. + * uses the 16 bytes of raw data to describe the UUID > * > - * Returns the secret on success, or NULL on failure. > + * Returns a new secret object or NULL in case of failure. If the > + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. > */ > virSecretPtr > -virSecretLookupByUUIDString(virConnectPtr conn, const char *uuid) > +virSecretLookupByUUID(virConnectPtr conn, const unsigned char *uuid) [...] > > /** > + * virSecretLookupByUUIDString: > + * @conn: pointer to the hypervisor connection > + * @uuidstr: the string UUID for the secret > + * > + * Try to lookup a secret on the given hypervisor based on its UUID. + * Uses the printable string value to describe the UUID > + * > + * Returns a new secret object or NULL in case of failure. If the > + * secret cannot be found, then VIR_ERR_NO_SECRET error is raised. > + */ > +virSecretPtr > +virSecretLookupByUUIDString(virConnectPtr conn, const char *uuidstr) > +{ > + int raw[VIR_UUID_BUFLEN], i; > + unsigned char uuid[VIR_UUID_BUFLEN]; > + int ret; > + > + DEBUG("conn=%p, uuidstr=%s", conn, uuidstr); > + > + virResetLastError(); > + > + if (!VIR_IS_CONNECT(conn)) { > + virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__); > + return (NULL); > + } > + if (uuidstr == NULL) { > + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); > + goto error; > + } > + /* XXX: sexpr_uuid() also supports 'xxxx-xxxx-xxxx-xxxx' format. > + * We needn't it here. Right? > + */ Hum, I would rather use virUUIDParse() to make sure there is no confusion, a public API should be tolerant about this I think and it's cleaner to reuse it ! > + ret = sscanf(uuidstr, > + "%02x%02x%02x%02x-" > + "%02x%02x-" > + "%02x%02x-" > + "%02x%02x-" > + "%02x%02x%02x%02x%02x%02x", > + raw + 0, raw + 1, raw + 2, raw + 3, > + raw + 4, raw + 5, raw + 6, raw + 7, > + raw + 8, raw + 9, raw + 10, raw + 11, > + raw + 12, raw + 13, raw + 14, raw + 15); > + > + if (ret!=VIR_UUID_BUFLEN) { > + virLibConnError(conn, VIR_ERR_INVALID_ARG, __FUNCTION__); > + goto error; > + } > + for (i = 0; i < VIR_UUID_BUFLEN; i++) > + uuid[i] = raw[i] & 0xFF; ACK, overall an important cleanup, as it affects the API and the network format ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list