[libvirt] [PATCH 05/10] secret: Introduce virSecretObjListAdd* and virSecretObjListRemove

[Eric Blake]

On 03/02/2016 11:55 AM, John Ferlan wrote:
> Add the functions to add/remove elements from the hashed secret obj list.
> These will replace secret_driver functions secretAssignDef and secretObjRemove.
> 
> The virSecretObjListAddLocked will perform the necessary lookups and
> decide whether to replace an existing hash entry or create a new one.
> This includes setting up the configPath and base64Path as well as being
> able to support the caller's need to restore from a previous definition
> in case something goes wrong in the caller.
> 
> Need to also move and make global virSecretUsageIDForDef since it'll
> be used by both secret_driver.c and secret_conf.c
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/conf/secret_conf.c     | 173 +++++++++++++++++++++++++++++++++++++++++++++
>  src/conf/secret_conf.h     |  15 ++++
>  src/libvirt_private.syms   |   1 +
>  src/secret/secret_driver.c |  34 ++-------
>  4 files changed, 196 insertions(+), 27 deletions(-)
> 

> +virSecretObjPtr
> +virSecretObjListAddLocked(virSecretObjListPtr secrets,
> +                          virSecretDefPtr def,
> +                          const char *configDir,
> +                          virSecretDefPtr *oldDef)

> +
> +        if (secret->def->private && !def->private) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("cannot change private flag on existing secret"));
> +            goto cleanup;
> +        }

This says we can't turn an existing public secret into a private one,
but nothing is protecting us from the converse of someone attempting to
redefine a private secret into a public one.  I think this should be:

if (secret->def->private != def->private)


> +
> +        /* Generate the possible configFile and base64File strings
> +         * using the configDir, uuidstr, and appropriate suffix
> +         */
> +        virUUIDFormat(def->uuid, uuidstr);
> +        if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
> +            !(base64File = virFileBuildPath(configDir, uuidstr, ".base64"))) {
> +            VIR_FREE(configFile);

Redundant; the cleanup label also takes care of freeing configFile.

> +            goto cleanup;
> +        }
> +

Everything else looked okay.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 604 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20160318/25bc3b7f/attachment-0001.sig>