On Mon, Apr 24, 2017 at 02:00:20PM -0400, John Ferlan wrote:
> Since we're storing a virUUIDFormat'd string in our Hash Table, let's
> modify the Lookup API to receive a formatted string as well.
It's true that we use formatted UUID as a key for storing the secret in hash
table we don't necessary need to change Lookup APIs to require formatted string
as well. All other objects have similar APIs to Lookup an object by UUID and
they don't require formatted UUID. I like the fact, that the formatting of
UUID is hidden for the caller since we store RAW UUIDs in our object structures.
I'm not sure about this change, I'm more inclined to NACK this patch.
Pavel
>
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
> src/conf/virsecretobj.c | 18 +++++++-----------
> src/conf/virsecretobj.h | 2 +-
> src/secret/secret_driver.c | 10 +++++-----
> 3 files changed, 13 insertions(+), 17 deletions(-)
>
> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> index 5acda4c..ae2b287 100644
> --- a/src/conf/virsecretobj.c
> +++ b/src/conf/virsecretobj.c
> @@ -163,12 +163,8 @@ virSecretObjListDispose(void *obj)
>
> static virSecretObjPtr
> virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
> - const unsigned char *uuid)
> + const char *uuidstr)
> {
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> -
> - virUUIDFormat(uuid, uuidstr);
> -
> return virObjectRef(virHashLookup(secrets->objs, uuidstr));
> }
>
> @@ -176,7 +172,7 @@ virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
> /**
> * virSecretObjFindByUUID:
> * @secrets: list of secret objects
> - * @uuid: secret uuid to find
> + * @uuidstr: secret uuid to find
> *
> * This function locks @secrets and finds the secret object which
> * corresponds to @uuid.
> @@ -185,12 +181,12 @@ virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
> */
> virSecretObjPtr
> virSecretObjListFindByUUID(virSecretObjListPtr secrets,
> - const unsigned char *uuid)
> + const char *uuidstr)
> {
> virSecretObjPtr obj;
>
> virObjectLock(secrets);
> - obj = virSecretObjListFindByUUIDLocked(secrets, uuid);
> + obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr);
> virObjectUnlock(secrets);
> if (obj)
> virObjectLock(obj);
> @@ -328,13 +324,14 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
> if (oldDef)
> *oldDef = NULL;
>
> + virUUIDFormat(newdef->uuid, uuidstr);
> +
> /* Is there a secret already matching this UUID */
> - if ((obj = virSecretObjListFindByUUIDLocked(secrets, newdef->uuid))) {
> + if ((obj = virSecretObjListFindByUUIDLocked(secrets, uuidstr))) {
> virObjectLock(obj);
> def = obj->def;
>
> if (STRNEQ_NULLABLE(def->usage_id, newdef->usage_id)) {
> - virUUIDFormat(def->uuid, uuidstr);
> virReportError(VIR_ERR_INTERNAL_ERROR,
> _("a secret with UUID %s is already defined for "
> "use with %s"),
> @@ -372,7 +369,6 @@ virSecretObjListAdd(virSecretObjListPtr secrets,
> /* Generate the possible configFile and base64File strings
> * using the configDir, uuidstr, and appropriate suffix
> */
> - virUUIDFormat(newdef->uuid, uuidstr);
> if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
> !(base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
> goto cleanup;
> diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
> index bd38f52..092f23c 100644
> --- a/src/conf/virsecretobj.h
> +++ b/src/conf/virsecretobj.h
> @@ -40,7 +40,7 @@ virSecretObjListNew(void);
>
> virSecretObjPtr
> virSecretObjListFindByUUID(virSecretObjListPtr secrets,
> - const unsigned char *uuid);
> + const char *uuidstr);
>
> virSecretObjPtr
> virSecretObjListFindByUsage(virSecretObjListPtr secrets,
> diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
> index cc050ff..2d4091d 100644
> --- a/src/secret/secret_driver.c
> +++ b/src/secret/secret_driver.c
> @@ -86,8 +86,8 @@ secretObjFromSecret(virSecretPtr secret)
> virSecretObjPtr obj;
> char uuidstr[VIR_UUID_STRING_BUFLEN];
>
> - if (!(obj = virSecretObjListFindByUUID(driver->secrets, secret->uuid))) {
> - virUUIDFormat(secret->uuid, uuidstr);
> + virUUIDFormat(secret->uuid, uuidstr);
> + if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuidstr))) {
> virReportError(VIR_ERR_NO_SECRET,
> _("no secret with matching uuid '%s'"), uuidstr);
> return NULL;
> @@ -159,10 +159,10 @@ secretLookupByUUID(virConnectPtr conn,
> virSecretPtr ret = NULL;
> virSecretObjPtr obj;
> virSecretDefPtr def;
> + char uuidstr[VIR_UUID_STRING_BUFLEN];
>
> - if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuid))) {
> - char uuidstr[VIR_UUID_STRING_BUFLEN];
> - virUUIDFormat(uuid, uuidstr);
> + virUUIDFormat(uuid, uuidstr);
> + if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuidstr))) {
> virReportError(VIR_ERR_NO_SECRET,
> _("no secret with matching uuid '%s'"), uuidstr);
> goto cleanup;
> --
> 2.9.3
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
