[libvirt] [PATCH v2 00/14] Convert secret driver to use hashed object

Cole Robinson crobinso at redhat.com
Sat Apr 23 16:38:41 UTC 2016


On 04/20/2016 07:40 AM, John Ferlan wrote:
> v1: http://www.redhat.com/archives/libvir-list/2016-March/msg00099.html
> 
> Differences high level:
>  - Use virsecretobj.{c,h} instead of secret_conf.{c,h}
>  - Work in v1 review comments
> 
> Difference details for those that care to know:
> 
> Note: Former patch2 has already been pushed.
> 
> Patch 1: Is combination of v1's patch1 and patch3 w/ adjustments:
> 
>   - Create virsecretobj.c and virsecretobj.h and move sources there.
>   - Add virsecretobj.{c,h} to src/Makefile.am
>   - Use order as suggested by eblake review of patch3 for forward
>     static decls, removed the != NULL from virSecretObjDispose, and
>     add comment regarding why memset() is being done
> 
> Patch 2: Former patch4
>   - Move code to virsecretobj.{c,h} instead of secret_conf.{c,h}
>     - Change the virSecretObjFindByUUIDLocked to use the construct
>       "return virObjectRef(virHashLookup(...));" as suggested by eblake
> 
> Patch3: Former patch5 (split)
>   - Split the format patch5 to have virSecretUsageIDForDef changes in one
>     patch and the remainder in the followup patch.
> 
> Patch4: Former patch5 (split)
>   - The rest of former patch5
>   - Removed the extraneous "VIR_FREE(configFile);" as noted by eblake.
>   - NOTE: eblake queried about the condition:
> 
>       if (secret->def->private && !def->private)
> 
>     in virSecretObjListAddLocked. This same check came from the former
>     secretDefineXML in the else of the "secretFindByUUID(new_attrs->uuid)".
>     IOW: Redefining a secret. The code didn't allow a "new" secret to
>     be not private if the currently defined one was private. I left it as is.
> 
> Patch5->Patch9: Former patch6->patch10:
>   - All that's different is using virsecretobj instead of secret_conf
> 
> Patch10: Former patch11
>   - Using virsecretobj instead of secret_conf
> 
> Patch11: Former patch10
>   - Adjusted and moved the comment from virSecretObjDeleteConfig to
>     virSecretObjDeleteData
>   - Added a virReportSystemError for the DeleteConfig error
> 
> Patch12: Former patch13:
>   - All that's different is using virsecretobj instead of secret_conf
> 
> Patch13: Former patch14
>   - Delete the extra space in the "return  0" as noted by Cole's review.
> 
> Patch14: Former patch15:
>   - All that's different is using virsecretobj instead of secret_conf
> 
> 
> After all is said done, I did a sdiff between the v1 secret_conf.h
> and v2 virsecretobj.h as well as the v1 secret_conf.c and v2 virsecretobj.c
> and found only the differences as noted above, plus removed a duplicated
> virSecretUsageIDForDef prototype I found in secret_conf.h.
> 
> I also went through the painful process of make check syntax-check at each
> step of the way and built using Coverity.
> 
> John Ferlan (14):
>   secret: Create virsecretobj.c and virsecretconf.h
>   secret: Introduce virSecretObjListFindBy{UUID|Usage} support
>   secret: Introduce virSecretUsageIDForDef
>   secret: Introduce virSecretObjListAdd* and virSecretObjListRemove
>   secret: Introduce virSecretObjListNumOfSecrets
>   secret: Introduce virSecretObjListExport
>   secret: Introduce virSecretObjListGetUUIDs
>   secret: Use the hashed virSecretObjList
>   secret: Move and rename secretLoadAllConfigs
>   secret: Introduce virSecretObjDelete{Config|Data}
>   secret: Introduce virSecretObjSave{Config|Data}
>   secret: Introduce virSecretObj{Get|Set}Def
>   secret: Introduce virSecretObjGetValue and virSecretObjGetValueSize
>   secret: Change virSecretDef variable names
> 
>  po/POTFILES.in                |    1 +
>  src/Makefile.am               |    3 +-
>  src/conf/secret_conf.c        |   37 +-
>  src/conf/secret_conf.h        |    9 +-
>  src/conf/virsecretobj.c       | 1011 +++++++++++++++++++++++++++++++++++++++++
>  src/conf/virsecretobj.h       |  110 +++++
>  src/libvirt_private.syms      |   24 +
>  src/secret/secret_driver.c    |  823 ++++-----------------------------
>  src/storage/storage_backend.c |    4 +-
>  9 files changed, 1279 insertions(+), 743 deletions(-)
>  create mode 100644 src/conf/virsecretobj.c
>  create mode 100644 src/conf/virsecretobj.h
> 

ACK (I reviewed the changes you mention above, but just skimmed the rest,
though i reviewed majority of v1)

- Cole




More information about the libvir-list mailing list