On Tue, Apr 25, 2017 at 07:57:44AM -0400, John Ferlan wrote:
>
>
> On 04/25/2017 04:29 AM, Pavel Hrdina wrote:
> > On Mon, Apr 24, 2017 at 02:00:14PM -0400, John Ferlan wrote:
> >> When processing a virSecretPtr use 'secret' as a variable name.
> >>
> >> When processing a virSecretObjPtr use 'obj' as a variable name.
> >>
> >> When processing a virSecretDefPtr use 'def' as a variable name,
> >> unless a distinction needs to be made with a 'newdef' such as
> >> virSecretObjListAddLocked (which also used the VIR_STEAL_PTR macro
> >> for the configFile and base64File).
> >>
> >> NB: Also made a slight algorithm adjustment for virSecretObjListRemove
> >> to check if the passed obj was NULL rather than having the caller
> >> virSecretLoad need to check prior to calling.
> >
> > I can see the motivation for this change, it's easier to follow the code
> > if the naming is consistent, however this patch adds one extra step when
> > tracing history of the code via git blame.
>
> Yes and I'm certainly not the first one down this path! Code motion
> tends to be the biggest offender though. At least with git blame
> variable name changes don't cause one to jump into some other module to
> find the code.... My least favorite is when a module "disappears" or is
> renamed - those always cause heartache, but I always find that gitk will
> find whatever I need without having to know some magic git command.
I'm not against this change, I just think that it's not necessary. If there
are no objections from others I guess that it can be ACKed.
>
> >
> > All the changes except the variable rename should be in separate patch as
> > they are unrelated to the rename.
>
> I can split out the virSecretObjListRemove change into a separate patch
> since it's the only non name change patch.
I guess that it would be better to split it.
Pavel
>
> John
> >
> > Pavel
> >
> >> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> >> ---
> >> src/conf/virsecretobj.c | 275 +++++++++++++++++++++++----------------------
> >> src/conf/virsecretobj.h | 26 ++---
> >> src/secret/secret_driver.c | 139 ++++++++++++-----------
> >> 3 files changed, 226 insertions(+), 214 deletions(-)
> >>
> >> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> >> index 348feb3..42f36c8 100644
> >> --- a/src/conf/virsecretobj.c
> >> +++ b/src/conf/virsecretobj.c
> >> @@ -89,29 +89,29 @@ VIR_ONCE_GLOBAL_INIT(virSecretObj)
> >> static virSecretObjPtr
> >> virSecretObjNew(void)
> >> {
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >>
> >> if (virSecretObjInitialize() < 0)
> >> return NULL;
> >>
> >> - if (!(secret = virObjectLockableNew(virSecretObjClass)))
> >> + if (!(obj = virObjectLockableNew(virSecretObjClass)))
> >> return NULL;
> >>
> >> - virObjectLock(secret);
> >> + virObjectLock(obj);
> >>
> >> - return secret;
> >> + return obj;
> >> }
> >>
> >>
> >> void
> >> -virSecretObjEndAPI(virSecretObjPtr *secret)
> >> +virSecretObjEndAPI(virSecretObjPtr *obj)
> >> {
> >> - if (!*secret)
> >> + if (!*obj)
> >> return;
> >>
> >> - virObjectUnlock(*secret);
> >> - virObjectUnref(*secret);
> >> - *secret = NULL;
> >> + virObjectUnlock(*obj);
> >> + virObjectUnref(*obj);
> >> + *obj = NULL;
> >> }
> >>
> >>
> >> @@ -136,18 +136,18 @@ virSecretObjListNew(void)
> >>
> >>
> >> static void
> >> -virSecretObjDispose(void *obj)
> >> +virSecretObjDispose(void *opaque)
> >> {
> >> - virSecretObjPtr secret = obj;
> >> + virSecretObjPtr obj = opaque;
> >>
> >> - virSecretDefFree(secret->def);
> >> - if (secret->value) {
> >> + virSecretDefFree(obj->def);
> >> + if (obj->value) {
> >> /* Wipe before free to ensure we don't leave a secret on the heap */
> >> - memset(secret->value, 0, secret->value_size);
> >> - VIR_FREE(secret->value);
> >> + memset(obj->value, 0, obj->value_size);
> >> + VIR_FREE(obj->value);
> >> }
> >> - VIR_FREE(secret->configFile);
> >> - VIR_FREE(secret->base64File);
> >> + VIR_FREE(obj->configFile);
> >> + VIR_FREE(obj->base64File);
> >> }
> >>
> >>
> >> @@ -186,14 +186,14 @@ virSecretObjPtr
> >> virSecretObjListFindByUUID(virSecretObjListPtr secrets,
> >> const unsigned char *uuid)
> >> {
> >> - virSecretObjPtr ret;
> >> + virSecretObjPtr obj;
> >>
> >> virObjectLock(secrets);
> >> - ret = virSecretObjListFindByUUIDLocked(secrets, uuid);
> >> + obj = virSecretObjListFindByUUIDLocked(secrets, uuid);
> >> virObjectUnlock(secrets);
> >> - if (ret)
> >> - virObjectLock(ret);
> >> - return ret;
> >> + if (obj)
> >> + virObjectLock(obj);
> >> + return obj;
> >> }
> >>
> >>
> >> @@ -202,21 +202,21 @@ virSecretObjSearchName(const void *payload,
> >> const void *name ATTRIBUTE_UNUSED,
> >> const void *opaque)
> >> {
> >> - virSecretObjPtr secret = (virSecretObjPtr) payload;
> >> + virSecretObjPtr obj = (virSecretObjPtr) payload;
> >> struct virSecretSearchData *data = (struct virSecretSearchData *) opaque;
> >> int found = 0;
> >>
> >> - virObjectLock(secret);
> >> + virObjectLock(obj);
> >>
> >> - if (secret->def->usage_type != data->usageType)
> >> + if (obj->def->usage_type != data->usageType)
> >> goto cleanup;
> >>
> >> if (data->usageType != VIR_SECRET_USAGE_TYPE_NONE &&
> >> - STREQ(secret->def->usage_id, data->usageID))
> >> + STREQ(obj->def->usage_id, data->usageID))
> >> found = 1;
> >>
> >> cleanup:
> >> - virObjectUnlock(secret);
> >> + virObjectUnlock(obj);
> >> return found;
> >> }
> >>
> >> @@ -226,14 +226,14 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
> >> int usageType,
> >> const char *usageID)
> >> {
> >> - virSecretObjPtr ret = NULL;
> >> + virSecretObjPtr obj = NULL;
> >> struct virSecretSearchData data = { .usageType = usageType,
> >> .usageID = usageID };
> >>
> >> - ret = virHashSearch(secrets->objs, virSecretObjSearchName, &data);
> >> - if (ret)
> >> - virObjectRef(ret);
> >> - return ret;
> >> + obj = virHashSearch(secrets->objs, virSecretObjSearchName, &data);
> >> + if (obj)
> >> + virObjectRef(obj);
> >> + return obj;
> >> }
> >>
> >>
> >> @@ -253,14 +253,14 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
> >> int usageType,
> >> const char *usageID)
> >> {
> >> - virSecretObjPtr ret;
> >> + virSecretObjPtr obj;
> >>
> >> virObjectLock(secrets);
> >> - ret = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
> >> + obj = virSecretObjListFindByUsageLocked(secrets, usageType, usageID);
> >> virObjectUnlock(secrets);
> >> - if (ret)
> >> - virObjectLock(ret);
> >> - return ret;
> >> + if (obj)
> >> + virObjectLock(obj);
> >> + return obj;
> >> }
> >>
> >>
> >> @@ -275,19 +275,22 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
> >> */
> >> void
> >> virSecretObjListRemove(virSecretObjListPtr secrets,
> >> - virSecretObjPtr secret)
> >> + virSecretObjPtr obj)
> >> {
> >> char uuidstr[VIR_UUID_STRING_BUFLEN];
> >>
> >> - virUUIDFormat(secret->def->uuid, uuidstr);
> >> - virObjectRef(secret);
> >> - virObjectUnlock(secret);
> >> + if (!obj)
> >> + return;
> >> +
> >> + virUUIDFormat(obj->def->uuid, uuidstr);
> >> + virObjectRef(obj);
> >> + virObjectUnlock(obj);
> >>
> >> virObjectLock(secrets);
> >> - virObjectLock(secret);
> >> + virObjectLock(obj);
> >> virHashRemoveEntry(secrets->objs, uuidstr);
> >> - virObjectUnlock(secret);
> >> - virObjectUnref(secret);
> >> + virObjectUnlock(obj);
> >> + virObjectUnref(obj);
> >> virObjectUnlock(secrets);
> >> }
> >>
> >> @@ -295,11 +298,11 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
> >> /*
> >> * virSecretObjListAddLocked:
> >> * @secrets: list of secret objects
> >> - * @def: new secret definition
> >> + * @newdef: new secret definition
> >> * @configDir: directory to place secret config files
> >> * @oldDef: Former secret def (e.g. a reload path perhaps)
> >> *
> >> - * Add the new def to the secret obj table hash
> >> + * Add the new @newdef to the secret obj table hash
> >> *
> >> * This functions requires @secrets to be locked already!
> >> *
> >> @@ -307,11 +310,11 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
> >> */
> >> static virSecretObjPtr
> >> virSecretObjListAddLocked(virSecretObjListPtr secrets,
> >> - virSecretDefPtr def,
> >> + virSecretDefPtr newdef,
> >> const char *configDir,
> >> virSecretDefPtr *oldDef)
> >> {
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >> virSecretObjPtr ret = NULL;
> >> char uuidstr[VIR_UUID_STRING_BUFLEN];
> >> char *configFile = NULL, *base64File = NULL;
> >> @@ -320,71 +323,69 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
> >> *oldDef = NULL;
> >>
> >> /* Is there a secret already matching this UUID */
> >> - if ((secret = virSecretObjListFindByUUIDLocked(secrets, def->uuid))) {
> >> - virObjectLock(secret);
> >> + if ((obj = virSecretObjListFindByUUIDLocked(secrets, newdef->uuid))) {
> >> + virObjectLock(obj);
> >>
> >> - if (STRNEQ_NULLABLE(secret->def->usage_id, def->usage_id)) {
> >> - virUUIDFormat(secret->def->uuid, uuidstr);
> >> + if (STRNEQ_NULLABLE(obj->def->usage_id, newdef->usage_id)) {
> >> + virUUIDFormat(obj->def->uuid, uuidstr);
> >> virReportError(VIR_ERR_INTERNAL_ERROR,
> >> _("a secret with UUID %s is already defined for "
> >> "use with %s"),
> >> - uuidstr, secret->def->usage_id);
> >> + uuidstr, obj->def->usage_id);
> >> goto cleanup;
> >> }
> >>
> >> - if (secret->def->isprivate && !def->isprivate) {
> >> + if (obj->def->isprivate && !newdef->isprivate) {
> >> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >> _("cannot change private flag on existing secret"));
> >> goto cleanup;
> >> }
> >>
> >> if (oldDef)
> >> - *oldDef = secret->def;
> >> + *oldDef = obj->def;
> >> else
> >> - virSecretDefFree(secret->def);
> >> - secret->def = def;
> >> + virSecretDefFree(obj->def);
> >> + obj->def = newdef;
> >> } else {
> >> /* No existing secret with same UUID,
> >> * try look for matching usage instead */
> >> - if ((secret = virSecretObjListFindByUsageLocked(secrets,
> >> - def->usage_type,
> >> - def->usage_id))) {
> >> - virObjectLock(secret);
> >> - virUUIDFormat(secret->def->uuid, uuidstr);
> >> + if ((obj = virSecretObjListFindByUsageLocked(secrets,
> >> + newdef->usage_type,
> >> + newdef->usage_id))) {
> >> + virObjectLock(obj);
> >> + virUUIDFormat(obj->def->uuid, uuidstr);
> >> virReportError(VIR_ERR_INTERNAL_ERROR,
> >> _("a secret with UUID %s already defined for "
> >> "use with %s"),
> >> - uuidstr, def->usage_id);
> >> + uuidstr, newdef->usage_id);
> >> goto cleanup;
> >> }
> >>
> >> /* Generate the possible configFile and base64File strings
> >> * using the configDir, uuidstr, and appropriate suffix
> >> */
> >> - virUUIDFormat(def->uuid, uuidstr);
> >> + virUUIDFormat(newdef->uuid, uuidstr);
> >> if (!(configFile = virFileBuildPath(configDir, uuidstr, ".xml")) ||
> >> !(base64File = virFileBuildPath(configDir, uuidstr, ".base64")))
> >> goto cleanup;
> >>
> >> - if (!(secret = virSecretObjNew()))
> >> + if (!(obj = virSecretObjNew()))
> >> goto cleanup;
> >>
> >> - if (virHashAddEntry(secrets->objs, uuidstr, secret) < 0)
> >> + if (virHashAddEntry(secrets->objs, uuidstr, obj) < 0)
> >> goto cleanup;
> >>
> >> - secret->def = def;
> >> - secret->configFile = configFile;
> >> - secret->base64File = base64File;
> >> - configFile = NULL;
> >> - base64File = NULL;
> >> - virObjectRef(secret);
> >> + obj->def = newdef;
> >> + VIR_STEAL_PTR(obj->configFile, configFile);
> >> + VIR_STEAL_PTR(obj->base64File, base64File);
> >> + virObjectRef(obj);
> >> }
> >>
> >> - ret = secret;
> >> - secret = NULL;
> >> + ret = obj;
> >> + obj = NULL;
> >>
> >> cleanup:
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >> VIR_FREE(configFile);
> >> VIR_FREE(base64File);
> >> return ret;
> >> @@ -393,16 +394,16 @@ virSecretObjListAddLocked(virSecretObjListPtr secrets,
> >>
> >> virSecretObjPtr
> >> virSecretObjListAdd(virSecretObjListPtr secrets,
> >> - virSecretDefPtr def,
> >> + virSecretDefPtr newdef,
> >> const char *configDir,
> >> virSecretDefPtr *oldDef)
> >> {
> >> - virSecretObjPtr ret;
> >> + virSecretObjPtr obj;
> >>
> >> virObjectLock(secrets);
> >> - ret = virSecretObjListAddLocked(secrets, def, configDir, oldDef);
> >> + obj = virSecretObjListAddLocked(secrets, newdef, configDir, oldDef);
> >> virObjectUnlock(secrets);
> >> - return ret;
> >> + return obj;
> >> }
> >>
> >>
> >> @@ -474,23 +475,23 @@ virSecretObjListNumOfSecrets(virSecretObjListPtr secrets,
> >>
> >> #define MATCH(FLAG) (flags & (FLAG))
> >> static bool
> >> -virSecretObjMatchFlags(virSecretObjPtr secret,
> >> +virSecretObjMatchFlags(virSecretObjPtr obj,
> >> unsigned int flags)
> >> {
> >> /* filter by whether it's ephemeral */
> >> if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL) &&
> >> !((MATCH(VIR_CONNECT_LIST_SECRETS_EPHEMERAL) &&
> >> - secret->def->isephemeral) ||
> >> + obj->def->isephemeral) ||
> >> (MATCH(VIR_CONNECT_LIST_SECRETS_NO_EPHEMERAL) &&
> >> - !secret->def->isephemeral)))
> >> + !obj->def->isephemeral)))
> >> return false;
> >>
> >> /* filter by whether it's private */
> >> if (MATCH(VIR_CONNECT_LIST_SECRETS_FILTERS_PRIVATE) &&
> >> !((MATCH(VIR_CONNECT_LIST_SECRETS_PRIVATE) &&
> >> - secret->def->isprivate) ||
> >> + obj->def->isprivate) ||
> >> (MATCH(VIR_CONNECT_LIST_SECRETS_NO_PRIVATE) &&
> >> - !secret->def->isprivate)))
> >> + !obj->def->isprivate)))
> >> return false;
> >>
> >> return true;
> >> @@ -621,12 +622,12 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
> >>
> >>
> >> int
> >> -virSecretObjDeleteConfig(virSecretObjPtr secret)
> >> +virSecretObjDeleteConfig(virSecretObjPtr obj)
> >> {
> >> - if (!secret->def->isephemeral &&
> >> - unlink(secret->configFile) < 0 && errno != ENOENT) {
> >> + if (!obj->def->isephemeral &&
> >> + unlink(obj->configFile) < 0 && errno != ENOENT) {
> >> virReportSystemError(errno, _("cannot unlink '%s'"),
> >> - secret->configFile);
> >> + obj->configFile);
> >> return -1;
> >> }
> >>
> >> @@ -635,11 +636,11 @@ virSecretObjDeleteConfig(virSecretObjPtr secret)
> >>
> >>
> >> void
> >> -virSecretObjDeleteData(virSecretObjPtr secret)
> >> +virSecretObjDeleteData(virSecretObjPtr obj)
> >> {
> >> /* The configFile will already be removed, so secret won't be
> >> * loaded again if this fails */
> >> - (void)unlink(secret->base64File);
> >> + (void)unlink(obj->base64File);
> >> }
> >>
> >>
> >> @@ -650,15 +651,15 @@ virSecretObjDeleteData(virSecretObjPtr secret)
> >> secret is defined, it is stored as base64 (with no formatting) in
> >> "$basename.base64". "$basename" is in both cases the base64-encoded UUID. */
> >> int
> >> -virSecretObjSaveConfig(virSecretObjPtr secret)
> >> +virSecretObjSaveConfig(virSecretObjPtr obj)
> >> {
> >> char *xml = NULL;
> >> int ret = -1;
> >>
> >> - if (!(xml = virSecretDefFormat(secret->def)))
> >> + if (!(xml = virSecretDefFormat(obj->def)))
> >> goto cleanup;
> >>
> >> - if (virFileRewriteStr(secret->configFile, S_IRUSR | S_IWUSR, xml) < 0)
> >> + if (virFileRewriteStr(obj->configFile, S_IRUSR | S_IWUSR, xml) < 0)
> >> goto cleanup;
> >>
> >> ret = 0;
> >> @@ -670,18 +671,18 @@ virSecretObjSaveConfig(virSecretObjPtr secret)
> >>
> >>
> >> int
> >> -virSecretObjSaveData(virSecretObjPtr secret)
> >> +virSecretObjSaveData(virSecretObjPtr obj)
> >> {
> >> char *base64 = NULL;
> >> int ret = -1;
> >>
> >> - if (!secret->value)
> >> + if (!obj->value)
> >> return 0;
> >>
> >> - if (!(base64 = virStringEncodeBase64(secret->value, secret->value_size)))
> >> + if (!(base64 = virStringEncodeBase64(obj->value, obj->value_size)))
> >> goto cleanup;
> >>
> >> - if (virFileRewriteStr(secret->base64File, S_IRUSR | S_IWUSR, base64) < 0)
> >> + if (virFileRewriteStr(obj->base64File, S_IRUSR | S_IWUSR, base64) < 0)
> >> goto cleanup;
> >>
> >> ret = 0;
> >> @@ -693,36 +694,36 @@ virSecretObjSaveData(virSecretObjPtr secret)
> >>
> >>
> >> virSecretDefPtr
> >> -virSecretObjGetDef(virSecretObjPtr secret)
> >> +virSecretObjGetDef(virSecretObjPtr obj)
> >> {
> >> - return secret->def;
> >> + return obj->def;
> >> }
> >>
> >>
> >> void
> >> -virSecretObjSetDef(virSecretObjPtr secret,
> >> +virSecretObjSetDef(virSecretObjPtr obj,
> >> virSecretDefPtr def)
> >> {
> >> - secret->def = def;
> >> + obj->def = def;
> >> }
> >>
> >>
> >> unsigned char *
> >> -virSecretObjGetValue(virSecretObjPtr secret)
> >> +virSecretObjGetValue(virSecretObjPtr obj)
> >> {
> >> unsigned char *ret = NULL;
> >>
> >> - if (!secret->value) {
> >> + if (!obj->value) {
> >> char uuidstr[VIR_UUID_STRING_BUFLEN];
> >> - virUUIDFormat(secret->def->uuid, uuidstr);
> >> + virUUIDFormat(obj->def->uuid, uuidstr);
> >> virReportError(VIR_ERR_NO_SECRET,
> >> _("secret '%s' does not have a value"), uuidstr);
> >> goto cleanup;
> >> }
> >>
> >> - if (VIR_ALLOC_N(ret, secret->value_size) < 0)
> >> + if (VIR_ALLOC_N(ret, obj->value_size) < 0)
> >> goto cleanup;
> >> - memcpy(ret, secret->value, secret->value_size);
> >> + memcpy(ret, obj->value, obj->value_size);
> >>
> >> cleanup:
> >> return ret;
> >> @@ -730,7 +731,7 @@ virSecretObjGetValue(virSecretObjPtr secret)
> >>
> >>
> >> int
> >> -virSecretObjSetValue(virSecretObjPtr secret,
> >> +virSecretObjSetValue(virSecretObjPtr obj,
> >> const unsigned char *value,
> >> size_t value_size)
> >> {
> >> @@ -740,14 +741,14 @@ virSecretObjSetValue(virSecretObjPtr secret,
> >> if (VIR_ALLOC_N(new_value, value_size) < 0)
> >> return -1;
> >>
> >> - old_value = secret->value;
> >> - old_value_size = secret->value_size;
> >> + old_value = obj->value;
> >> + old_value_size = obj->value_size;
> >>
> >> memcpy(new_value, value, value_size);
> >> - secret->value = new_value;
> >> - secret->value_size = value_size;
> >> + obj->value = new_value;
> >> + obj->value_size = value_size;
> >>
> >> - if (!secret->def->isephemeral && virSecretObjSaveData(secret) < 0)
> >> + if (!obj->def->isephemeral && virSecretObjSaveData(obj) < 0)
> >> goto error;
> >>
> >> /* Saved successfully - drop old value */
> >> @@ -760,8 +761,8 @@ virSecretObjSetValue(virSecretObjPtr secret,
> >>
> >> error:
> >> /* Error - restore previous state and free new value */
> >> - secret->value = old_value;
> >> - secret->value_size = old_value_size;
> >> + obj->value = old_value;
> >> + obj->value_size = old_value_size;
> >> memset(new_value, 0, value_size);
> >> VIR_FREE(new_value);
> >> return -1;
> >> @@ -769,17 +770,17 @@ virSecretObjSetValue(virSecretObjPtr secret,
> >>
> >>
> >> size_t
> >> -virSecretObjGetValueSize(virSecretObjPtr secret)
> >> +virSecretObjGetValueSize(virSecretObjPtr obj)
> >> {
> >> - return secret->value_size;
> >> + return obj->value_size;
> >> }
> >>
> >>
> >> void
> >> -virSecretObjSetValueSize(virSecretObjPtr secret,
> >> +virSecretObjSetValueSize(virSecretObjPtr obj,
> >> size_t value_size)
> >> {
> >> - secret->value_size = value_size;
> >> + obj->value_size = value_size;
> >> }
> >>
> >>
> >> @@ -803,33 +804,33 @@ virSecretLoadValidateUUID(virSecretDefPtr def,
> >>
> >>
> >> static int
> >> -virSecretLoadValue(virSecretObjPtr secret)
> >> +virSecretLoadValue(virSecretObjPtr obj)
> >> {
> >> int ret = -1, fd = -1;
> >> struct stat st;
> >> char *contents = NULL, *value = NULL;
> >> size_t value_size;
> >>
> >> - if ((fd = open(secret->base64File, O_RDONLY)) == -1) {
> >> + if ((fd = open(obj->base64File, O_RDONLY)) == -1) {
> >> if (errno == ENOENT) {
> >> ret = 0;
> >> goto cleanup;
> >> }
> >> virReportSystemError(errno, _("cannot open '%s'"),
> >> - secret->base64File);
> >> + obj->base64File);
> >> goto cleanup;
> >> }
> >>
> >> if (fstat(fd, &st) < 0) {
> >> virReportSystemError(errno, _("cannot stat '%s'"),
> >> - secret->base64File);
> >> + obj->base64File);
> >> goto cleanup;
> >> }
> >>
> >> if ((size_t)st.st_size != st.st_size) {
> >> virReportError(VIR_ERR_INTERNAL_ERROR,
> >> _("'%s' file does not fit in memory"),
> >> - secret->base64File);
> >> + obj->base64File);
> >> goto cleanup;
> >> }
> >>
> >> @@ -838,7 +839,7 @@ virSecretLoadValue(virSecretObjPtr secret)
> >>
> >> if (saferead(fd, contents, st.st_size) != st.st_size) {
> >> virReportSystemError(errno, _("cannot read '%s'"),
> >> - secret->base64File);
> >> + obj->base64File);
> >> goto cleanup;
> >> }
> >>
> >> @@ -847,15 +848,15 @@ virSecretLoadValue(virSecretObjPtr secret)
> >> if (!base64_decode_alloc(contents, st.st_size, &value, &value_size)) {
> >> virReportError(VIR_ERR_INTERNAL_ERROR,
> >> _("invalid base64 in '%s'"),
> >> - secret->base64File);
> >> + obj->base64File);
> >> goto cleanup;
> >> }
> >> if (value == NULL)
> >> goto cleanup;
> >>
> >> - secret->value = (unsigned char *)value;
> >> + obj->value = (unsigned char *)value;
> >> value = NULL;
> >> - secret->value_size = value_size;
> >> + obj->value_size = value_size;
> >>
> >> ret = 0;
> >>
> >> @@ -880,7 +881,8 @@ virSecretLoad(virSecretObjListPtr secrets,
> >> const char *configDir)
> >> {
> >> virSecretDefPtr def = NULL;
> >> - virSecretObjPtr secret = NULL, ret = NULL;
> >> + virSecretObjPtr obj = NULL;
> >> + virSecretObjPtr ret = NULL;
> >>
> >> if (!(def = virSecretDefParseFile(path)))
> >> goto cleanup;
> >> @@ -888,19 +890,18 @@ virSecretLoad(virSecretObjListPtr secrets,
> >> if (virSecretLoadValidateUUID(def, file) < 0)
> >> goto cleanup;
> >>
> >> - if (!(secret = virSecretObjListAdd(secrets, def, configDir, NULL)))
> >> + if (!(obj = virSecretObjListAdd(secrets, def, configDir, NULL)))
> >> goto cleanup;
> >> def = NULL;
> >>
> >> - if (virSecretLoadValue(secret) < 0)
> >> + if (virSecretLoadValue(obj) < 0)
> >> goto cleanup;
> >>
> >> - ret = secret;
> >> - secret = NULL;
> >> + ret = obj;
> >> + obj = NULL;
> >>
> >> cleanup:
> >> - if (secret)
> >> - virSecretObjListRemove(secrets, secret);
> >> + virSecretObjListRemove(secrets, obj);
> >> virSecretDefFree(def);
> >> return ret;
> >> }
> >> @@ -921,7 +922,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
> >> * loop (if any). It's better to keep the secrets we managed to find. */
> >> while (virDirRead(dir, &de, NULL) > 0) {
> >> char *path;
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >>
> >> if (!virFileHasSuffix(de->d_name, ".xml"))
> >> continue;
> >> @@ -929,7 +930,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
> >> if (!(path = virFileBuildPath(configDir, de->d_name, NULL)))
> >> continue;
> >>
> >> - if (!(secret = virSecretLoad(secrets, de->d_name, path, configDir))) {
> >> + if (!(obj = virSecretLoad(secrets, de->d_name, path, configDir))) {
> >> VIR_ERROR(_("Error reading secret: %s"),
> >> virGetLastErrorMessage());
> >> VIR_FREE(path);
> >> @@ -937,7 +938,7 @@ virSecretLoadAllConfigs(virSecretObjListPtr secrets,
> >> }
> >>
> >> VIR_FREE(path);
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >> }
> >>
> >> VIR_DIR_CLOSE(dir);
> >> diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
> >> index 9638b69..8038faa 100644
> >> --- a/src/conf/virsecretobj.h
> >> +++ b/src/conf/virsecretobj.h
> >> @@ -30,7 +30,7 @@ typedef struct _virSecretObj virSecretObj;
> >> typedef virSecretObj *virSecretObjPtr;
> >>
> >> void
> >> -virSecretObjEndAPI(virSecretObjPtr *secret);
> >> +virSecretObjEndAPI(virSecretObjPtr *obj);
> >>
> >> typedef struct _virSecretObjList virSecretObjList;
> >> typedef virSecretObjList *virSecretObjListPtr;
> >> @@ -49,11 +49,11 @@ virSecretObjListFindByUsage(virSecretObjListPtr secrets,
> >>
> >> void
> >> virSecretObjListRemove(virSecretObjListPtr secrets,
> >> - virSecretObjPtr secret);
> >> + virSecretObjPtr obj);
> >>
> >> virSecretObjPtr
> >> virSecretObjListAdd(virSecretObjListPtr secrets,
> >> - virSecretDefPtr def,
> >> + virSecretDefPtr newdef,
> >> const char *configDir,
> >> virSecretDefPtr *oldDef);
> >>
> >> @@ -81,37 +81,37 @@ virSecretObjListGetUUIDs(virSecretObjListPtr secrets,
> >> virConnectPtr conn);
> >>
> >> int
> >> -virSecretObjDeleteConfig(virSecretObjPtr secret);
> >> +virSecretObjDeleteConfig(virSecretObjPtr obj);
> >>
> >> void
> >> -virSecretObjDeleteData(virSecretObjPtr secret);
> >> +virSecretObjDeleteData(virSecretObjPtr obj);
> >>
> >> int
> >> -virSecretObjSaveConfig(virSecretObjPtr secret);
> >> +virSecretObjSaveConfig(virSecretObjPtr obj);
> >>
> >> int
> >> -virSecretObjSaveData(virSecretObjPtr secret);
> >> +virSecretObjSaveData(virSecretObjPtr obj);
> >>
> >> virSecretDefPtr
> >> -virSecretObjGetDef(virSecretObjPtr secret);
> >> +virSecretObjGetDef(virSecretObjPtr obj);
> >>
> >> void
> >> -virSecretObjSetDef(virSecretObjPtr secret,
> >> +virSecretObjSetDef(virSecretObjPtr obj,
> >> virSecretDefPtr def);
> >>
> >> unsigned char *
> >> -virSecretObjGetValue(virSecretObjPtr secret);
> >> +virSecretObjGetValue(virSecretObjPtr obj);
> >>
> >> int
> >> -virSecretObjSetValue(virSecretObjPtr secret,
> >> +virSecretObjSetValue(virSecretObjPtr obj,
> >> const unsigned char *value,
> >> size_t value_size);
> >>
> >> size_t
> >> -virSecretObjGetValueSize(virSecretObjPtr secret);
> >> +virSecretObjGetValueSize(virSecretObjPtr obj);
> >>
> >> void
> >> -virSecretObjSetValueSize(virSecretObjPtr secret,
> >> +virSecretObjSetValueSize(virSecretObjPtr obj,
> >> size_t value_size);
> >>
> >> int
> >> diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c
> >> index 2a371b6..cc050ff 100644
> >> --- a/src/secret/secret_driver.c
> >> +++ b/src/secret/secret_driver.c
> >> @@ -72,6 +72,7 @@ secretDriverLock(void)
> >> virMutexLock(&driver->lock);
> >> }
> >>
> >> +
> >> static void
> >> secretDriverUnlock(void)
> >> {
> >> @@ -79,7 +80,6 @@ secretDriverUnlock(void)
> >> }
> >>
> >>
> >> -
> >> static virSecretObjPtr
> >> secretObjFromSecret(virSecretPtr secret)
> >> {
> >> @@ -120,6 +120,7 @@ secretConnectNumOfSecrets(virConnectPtr conn)
> >> conn);
> >> }
> >>
> >> +
> >> static int
> >> secretConnectListSecrets(virConnectPtr conn,
> >> char **uuids,
> >> @@ -156,10 +157,10 @@ secretLookupByUUID(virConnectPtr conn,
> >> const unsigned char *uuid)
> >> {
> >> virSecretPtr ret = NULL;
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >> virSecretDefPtr def;
> >>
> >> - if (!(secret = virSecretObjListFindByUUID(driver->secrets, uuid))) {
> >> + if (!(obj = virSecretObjListFindByUUID(driver->secrets, uuid))) {
> >> char uuidstr[VIR_UUID_STRING_BUFLEN];
> >> virUUIDFormat(uuid, uuidstr);
> >> virReportError(VIR_ERR_NO_SECRET,
> >> @@ -167,7 +168,7 @@ secretLookupByUUID(virConnectPtr conn,
> >> goto cleanup;
> >> }
> >>
> >> - def = virSecretObjGetDef(secret);
> >> + def = virSecretObjGetDef(obj);
> >> if (virSecretLookupByUUIDEnsureACL(conn, def) < 0)
> >> goto cleanup;
> >>
> >> @@ -177,7 +178,7 @@ secretLookupByUUID(virConnectPtr conn,
> >> def->usage_id);
> >>
> >> cleanup:
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >> return ret;
> >> }
> >>
> >> @@ -188,17 +189,17 @@ secretLookupByUsage(virConnectPtr conn,
> >> const char *usageID)
> >> {
> >> virSecretPtr ret = NULL;
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >> virSecretDefPtr def;
> >>
> >> - if (!(secret = virSecretObjListFindByUsage(driver->secrets,
> >> - usageType, usageID))) {
> >> + if (!(obj = virSecretObjListFindByUsage(driver->secrets,
> >> + usageType, usageID))) {
> >> virReportError(VIR_ERR_NO_SECRET,
> >> _("no secret with matching usage '%s'"), usageID);
> >> goto cleanup;
> >> }
> >>
> >> - def = virSecretObjGetDef(secret);
> >> + def = virSecretObjGetDef(obj);
> >> if (virSecretLookupByUsageEnsureACL(conn, def) < 0)
> >> goto cleanup;
> >>
> >> @@ -208,7 +209,7 @@ secretLookupByUsage(virConnectPtr conn,
> >> def->usage_id);
> >>
> >> cleanup:
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >> return ret;
> >> }
> >>
> >> @@ -219,129 +220,131 @@ secretDefineXML(virConnectPtr conn,
> >> unsigned int flags)
> >> {
> >> virSecretPtr ret = NULL;
> >> - virSecretObjPtr secret = NULL;
> >> + virSecretObjPtr obj = NULL;
> >> virSecretDefPtr backup = NULL;
> >> - virSecretDefPtr new_attrs;
> >> + virSecretDefPtr def;
> >> virObjectEventPtr event = NULL;
> >>
> >> virCheckFlags(0, NULL);
> >>
> >> - if (!(new_attrs = virSecretDefParseString(xml)))
> >> + if (!(def = virSecretDefParseString(xml)))
> >> return NULL;
> >>
> >> - if (virSecretDefineXMLEnsureACL(conn, new_attrs) < 0)
> >> + if (virSecretDefineXMLEnsureACL(conn, def) < 0)
> >> goto cleanup;
> >>
> >> - if (!(secret = virSecretObjListAdd(driver->secrets, new_attrs,
> >> - driver->configDir, &backup)))
> >> + if (!(obj = virSecretObjListAdd(driver->secrets, def,
> >> + driver->configDir, &backup)))
> >> goto cleanup;
> >>
> >> - if (!new_attrs->isephemeral) {
> >> + if (!def->isephemeral) {
> >> if (secretEnsureDirectory() < 0)
> >> goto cleanup;
> >>
> >> if (backup && backup->isephemeral) {
> >> - if (virSecretObjSaveData(secret) < 0)
> >> + if (virSecretObjSaveData(obj) < 0)
> >> goto restore_backup;
> >> }
> >>
> >> - if (virSecretObjSaveConfig(secret) < 0) {
> >> + if (virSecretObjSaveConfig(obj) < 0) {
> >> if (backup && backup->isephemeral) {
> >> /* Undo the virSecretObjSaveData() above; ignore errors */
> >> - virSecretObjDeleteData(secret);
> >> + virSecretObjDeleteData(obj);
> >> }
> >> goto restore_backup;
> >> }
> >> } else if (backup && !backup->isephemeral) {
> >> - if (virSecretObjDeleteConfig(secret) < 0)
> >> + if (virSecretObjDeleteConfig(obj) < 0)
> >> goto restore_backup;
> >>
> >> - virSecretObjDeleteData(secret);
> >> + virSecretObjDeleteData(obj);
> >> }
> >> /* Saved successfully - drop old values */
> >> virSecretDefFree(backup);
> >>
> >> - event = virSecretEventLifecycleNew(new_attrs->uuid,
> >> - new_attrs->usage_type,
> >> - new_attrs->usage_id,
> >> + event = virSecretEventLifecycleNew(def->uuid,
> >> + def->usage_type,
> >> + def->usage_id,
> >> VIR_SECRET_EVENT_DEFINED,
> >> 0);
> >>
> >> ret = virGetSecret(conn,
> >> - new_attrs->uuid,
> >> - new_attrs->usage_type,
> >> - new_attrs->usage_id);
> >> - new_attrs = NULL;
> >> + def->uuid,
> >> + def->usage_type,
> >> + def->usage_id);
> >> + def = NULL;
> >> goto cleanup;
> >>
> >> restore_backup:
> >> /* If we have a backup, then secret was defined before, so just restore
> >> - * the backup. The current (new_attrs) will be handled below.
> >> + * the backup. The current def will be handled below.
> >> * Otherwise, this is a new secret, thus remove it.
> >> */
> >> if (backup)
> >> - virSecretObjSetDef(secret, backup);
> >> + virSecretObjSetDef(obj, backup);
> >> else
> >> - virSecretObjListRemove(driver->secrets, secret);
> >> + virSecretObjListRemove(driver->secrets, obj);
> >>
> >> cleanup:
> >> - virSecretDefFree(new_attrs);
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretDefFree(def);
> >> + virSecretObjEndAPI(&obj);
> >> if (event)
> >> virObjectEventStateQueue(driver->secretEventState, event);
> >>
> >> return ret;
> >> }
> >>
> >> +
> >> static char *
> >> -secretGetXMLDesc(virSecretPtr obj,
> >> +secretGetXMLDesc(virSecretPtr secret,
> >> unsigned int flags)
> >> {
> >> char *ret = NULL;
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >> virSecretDefPtr def;
> >>
> >> virCheckFlags(0, NULL);
> >>
> >> - if (!(secret = secretObjFromSecret(obj)))
> >> + if (!(obj = secretObjFromSecret(secret)))
> >> goto cleanup;
> >>
> >> - def = virSecretObjGetDef(secret);
> >> - if (virSecretGetXMLDescEnsureACL(obj->conn, def) < 0)
> >> + def = virSecretObjGetDef(obj);
> >> + if (virSecretGetXMLDescEnsureACL(secret->conn, def) < 0)
> >> goto cleanup;
> >>
> >> ret = virSecretDefFormat(def);
> >>
> >> cleanup:
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >>
> >> return ret;
> >> }
> >>
> >> +
> >> static int
> >> -secretSetValue(virSecretPtr obj,
> >> +secretSetValue(virSecretPtr secret,
> >> const unsigned char *value,
> >> size_t value_size,
> >> unsigned int flags)
> >> {
> >> int ret = -1;
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >> virSecretDefPtr def;
> >> virObjectEventPtr event = NULL;
> >>
> >> virCheckFlags(0, -1);
> >>
> >> - if (!(secret = secretObjFromSecret(obj)))
> >> + if (!(obj = secretObjFromSecret(secret)))
> >> goto cleanup;
> >>
> >> - def = virSecretObjGetDef(secret);
> >> - if (virSecretSetValueEnsureACL(obj->conn, def) < 0)
> >> + def = virSecretObjGetDef(obj);
> >> + if (virSecretSetValueEnsureACL(secret->conn, def) < 0)
> >> goto cleanup;
> >>
> >> if (secretEnsureDirectory() < 0)
> >> goto cleanup;
> >>
> >> - if (virSecretObjSetValue(secret, value, value_size) < 0)
> >> + if (virSecretObjSetValue(obj, value, value_size) < 0)
> >> goto cleanup;
> >>
> >> event = virSecretEventValueChangedNew(def->uuid,
> >> @@ -350,30 +353,31 @@ secretSetValue(virSecretPtr obj,
> >> ret = 0;
> >>
> >> cleanup:
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >> if (event)
> >> virObjectEventStateQueue(driver->secretEventState, event);
> >>
> >> return ret;
> >> }
> >>
> >> +
> >> static unsigned char *
> >> -secretGetValue(virSecretPtr obj,
> >> +secretGetValue(virSecretPtr secret,
> >> size_t *value_size,
> >> unsigned int flags,
> >> unsigned int internalFlags)
> >> {
> >> unsigned char *ret = NULL;
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >> virSecretDefPtr def;
> >>
> >> virCheckFlags(0, NULL);
> >>
> >> - if (!(secret = secretObjFromSecret(obj)))
> >> + if (!(obj = secretObjFromSecret(secret)))
> >> goto cleanup;
> >>
> >> - def = virSecretObjGetDef(secret);
> >> - if (virSecretGetValueEnsureACL(obj->conn, def) < 0)
> >> + def = virSecretObjGetDef(obj);
> >> + if (virSecretGetValueEnsureACL(secret->conn, def) < 0)
> >> goto cleanup;
> >>
> >> if ((internalFlags & VIR_SECRET_GET_VALUE_INTERNAL_CALL) == 0 &&
> >> @@ -383,33 +387,34 @@ secretGetValue(virSecretPtr obj,
> >> goto cleanup;
> >> }
> >>
> >> - if (!(ret = virSecretObjGetValue(secret)))
> >> + if (!(ret = virSecretObjGetValue(obj)))
> >> goto cleanup;
> >>
> >> - *value_size = virSecretObjGetValueSize(secret);
> >> + *value_size = virSecretObjGetValueSize(obj);
> >>
> >> cleanup:
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >>
> >> return ret;
> >> }
> >>
> >> +
> >> static int
> >> -secretUndefine(virSecretPtr obj)
> >> +secretUndefine(virSecretPtr secret)
> >> {
> >> int ret = -1;
> >> - virSecretObjPtr secret;
> >> + virSecretObjPtr obj;
> >> virSecretDefPtr def;
> >> virObjectEventPtr event = NULL;
> >>
> >> - if (!(secret = secretObjFromSecret(obj)))
> >> + if (!(obj = secretObjFromSecret(secret)))
> >> goto cleanup;
> >>
> >> - def = virSecretObjGetDef(secret);
> >> - if (virSecretUndefineEnsureACL(obj->conn, def) < 0)
> >> + def = virSecretObjGetDef(obj);
> >> + if (virSecretUndefineEnsureACL(secret->conn, def) < 0)
> >> goto cleanup;
> >>
> >> - if (virSecretObjDeleteConfig(secret) < 0)
> >> + if (virSecretObjDeleteConfig(obj) < 0)
> >> goto cleanup;
> >>
> >> event = virSecretEventLifecycleNew(def->uuid,
> >> @@ -418,20 +423,21 @@ secretUndefine(virSecretPtr obj)
> >> VIR_SECRET_EVENT_UNDEFINED,
> >> 0);
> >>
> >> - virSecretObjDeleteData(secret);
> >> + virSecretObjDeleteData(obj);
> >>
> >> - virSecretObjListRemove(driver->secrets, secret);
> >> + virSecretObjListRemove(driver->secrets, obj);
> >>
> >> ret = 0;
> >>
> >> cleanup:
> >> - virSecretObjEndAPI(&secret);
> >> + virSecretObjEndAPI(&obj);
> >> if (event)
> >> virObjectEventStateQueue(driver->secretEventState, event);
> >>
> >> return ret;
> >> }
> >>
> >> +
> >> static int
> >> secretStateCleanup(void)
> >> {
> >> @@ -452,6 +458,7 @@ secretStateCleanup(void)
> >> return 0;
> >> }
> >>
> >> +
> >> static int
> >> secretStateInitialize(bool privileged,
> >> virStateInhibitCallback callback ATTRIBUTE_UNUSED,
> >> @@ -497,6 +504,7 @@ secretStateInitialize(bool privileged,
> >> return -1;
> >> }
> >>
> >> +
> >> static int
> >> secretStateReload(void)
> >> {
> >> @@ -511,6 +519,7 @@ secretStateReload(void)
> >> return 0;
> >> }
> >>
> >> +
> >> static int
> >> secretConnectSecretEventRegisterAny(virConnectPtr conn,
> >> virSecretPtr secret,
> >> @@ -532,6 +541,7 @@ secretConnectSecretEventRegisterAny(virConnectPtr conn,
> >> return callbackID;
> >> }
> >>
> >> +
> >> static int
> >> secretConnectSecretEventDeregisterAny(virConnectPtr conn,
> >> int callbackID)
> >> @@ -576,6 +586,7 @@ static virStateDriver stateDriver = {
> >> .stateReload = secretStateReload,
> >> };
> >>
> >> +
> >> int
> >> secretRegister(void)
> >> {
> >> --
> >> 2.9.3
> >>
> >> --
> >> libvir-list mailing list
> >> libvir-list@xxxxxxxxxx
> >> https://www.redhat.com/mailman/listinfo/libvir-list
>
> --
> 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
