Re: [PATCH 09/14] secret: Create a 'configFile' in virSecretObj

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 02/25/2016 09:03 AM, John Ferlan wrote:
> This patch removes the need for secretXMLPath. Instead save 'path' during
> loadSecret as 'configFile'. The secretXMLPath is nothing more than an
> open coded virFileBuildPath.  All that code did was concantenate the
> driver->configDir, the UUID of the secret, and the ".xml" suffix to form
> the configFile name which we now will generate and save instead.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/secret/secret_driver.c | 39 ++++++++++++++++++++-------------------
>  1 file changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
> index 4dee2a6..d4b0207 100644
> --- a/src/secret/secret_driver.c
> +++ b/src/secret/secret_driver.c
> @@ -56,6 +56,7 @@ typedef struct _virSecretObj virSecretObj;
>  typedef virSecretObj *virSecretObjPtr;
>  struct _virSecretObj {
>      virSecretObjPtr next;
> +    char *configFile;
>      virSecretDefPtr def;
>      unsigned char *value;       /* May be NULL */
>      size_t value_size;
> @@ -112,6 +113,7 @@ secretFree(virSecretObjPtr secret)
>          memset(secret->value, 0, secret->value_size);
>          VIR_FREE(secret->value);
>      }
> +    VIR_FREE(secret->configFile);
>      VIR_FREE(secret);
>  }
>  
> @@ -197,11 +199,6 @@ secretComputePath(const virSecretObj *secret,
>      return ret;
>  }
>  
> -static char *
> -secretXMLPath(const virSecretObj *secret)
> -{
> -    return secretComputePath(secret, ".xml");
> -}
>  
>  static char *
>  secretBase64Path(const virSecretObj *secret)
> @@ -223,19 +220,16 @@ secretEnsureDirectory(void)
>  static int
>  secretSaveDef(const virSecretObj *secret)
>  {
> -    char *filename = NULL, *xml = NULL;
> +    char *xml = NULL;
>      int ret = -1;
>  
>      if (secretEnsureDirectory() < 0)
>          goto cleanup;
>  
> -    if (!(filename = secretXMLPath(secret)))
> -        goto cleanup;
> -
>      if (!(xml = virSecretDefFormat(secret->def)))
>          goto cleanup;
>  
> -    if (virFileRewrite(filename, S_IRUSR | S_IWUSR,
> +    if (virFileRewrite(secret->configFile, S_IRUSR | S_IWUSR,
>                         secretRewriteFile, xml) < 0)
>          goto cleanup;
>  
> @@ -243,7 +237,6 @@ secretSaveDef(const virSecretObj *secret)
>  
>   cleanup:
>      VIR_FREE(xml);
> -    VIR_FREE(filename);
>      return ret;
>  }
>  
> @@ -284,16 +277,13 @@ secretSaveValue(const virSecretObj *secret)
>  static int
>  secretDeleteSaved(const virSecretObj *secret)
>  {
> -    char *xml_filename = NULL, *value_filename = NULL;
> +    char *value_filename = NULL;
>      int ret = -1;
>  
> -    if (!(xml_filename = secretXMLPath(secret)))
> -        goto cleanup;
> -
>      if (!(value_filename = secretBase64Path(secret)))
>          goto cleanup;
>  
> -    if (unlink(xml_filename) < 0 && errno != ENOENT)
> +    if (unlink(secret->configFile) < 0 && errno != ENOENT)
>          goto cleanup;
>      /* When the XML is missing, the rest may waste disk space, but the secret
>         won't be loaded again, so we have succeeded already. */
> @@ -303,7 +293,6 @@ secretDeleteSaved(const virSecretObj *secret)
>  
>   cleanup:
>      VIR_FREE(value_filename);
> -    VIR_FREE(xml_filename);
>      return ret;
>  }
>  
> @@ -412,6 +401,9 @@ secretLoad(const char *file,
>      secret->def = def;
>      def = NULL;
>  
> +    if (VIR_STRDUP(secret->configFile, path) < 0)
> +        goto cleanup;
> +
>      if (secretLoadValue(secret) < 0)
>          goto cleanup;
>  
> @@ -731,9 +723,10 @@ secretDefineXML(virConnectPtr conn,
>          /* No existing secret with same UUID,
>           * try look for matching usage instead */
>          const char *usageID = secretUsageIDForDef(new_attrs);
> +        char uuidstr[VIR_UUID_STRING_BUFLEN];
> +
> +        virUUIDFormat(secret->def->uuid, uuidstr);
>          if ((secret = secretFindByUsage(new_attrs->usage_type, usageID))) {
> -            char uuidstr[VIR_UUID_STRING_BUFLEN];
> -            virUUIDFormat(secret->def->uuid, uuidstr);
>              virReportError(VIR_ERR_INTERNAL_ERROR,
>                             _("a secret with UUID %s already defined for "
>                               "use with %s"),
> @@ -745,6 +738,14 @@ secretDefineXML(virConnectPtr conn,
>          if (VIR_ALLOC(secret) < 0)
>              goto cleanup;
>  
> +        /* Generate configFile using driver->configDir,
> +         * the uuidstr, and .xml suffix */
> +        if (!(secret->configFile = virFileBuildPath(driver->configDir,
> +                                                    uuidstr, ".xml"))) {
> +            secretFree(secret);
> +            goto cleanup;
> +        }
> +
>          listInsert(&driver->secrets, secret);
>          secret->def = new_attrs;
>      } else {
> 

<sigh> Move code and neglected to retest... Anyway, consider the
following squashed in (it has ramifications a few patches later too.
Essentially, cannot deref secret-> yet - do it only after we're
sure we have a non NULL secret pointer):

diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
index d4b0207..9ad3d68 100644
--- a/src/secret/secret_driver.c
+++ b/src/secret/secret_driver.c
@@ -725,8 +725,8 @@ secretDefineXML(virConnectPtr conn,
         const char *usageID = secretUsageIDForDef(new_attrs);
         char uuidstr[VIR_UUID_STRING_BUFLEN];
 
-        virUUIDFormat(secret->def->uuid, uuidstr);
         if ((secret = secretFindByUsage(new_attrs->usage_type, usageID))) {
+            virUUIDFormat(secret->def->uuid, uuidstr);
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("a secret with UUID %s already defined for "
                              "use with %s"),
@@ -738,6 +738,8 @@ secretDefineXML(virConnectPtr conn,
         if (VIR_ALLOC(secret) < 0)
             goto cleanup;
 
+        virUUIDFormat(secret->def->uuid, uuidstr);
+
         /* Generate configFile using driver->configDir,
          * the uuidstr, and .xml suffix */
         if (!(secret->configFile = virFileBuildPath(driver->configDir,

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]