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