6:55 a.m.
On 04/25/2017 04:12 AM, Pavel Hrdina wrote:
On Mon, Apr 24, 2017 at 02:00:12PM -0400, John Ferlan wrote:
> Make various virSecretObjList*Locked functions static and make
> virSecretObjNew static since they're only called within virtsecretobj.c
>
> Signed-off-by: John Ferlan <jferlan(a)redhat.com>
> ---
> src/conf/virsecretobj.c | 33 +++++++--------------------------
> src/conf/virsecretobj.h | 18 ------------------
> 2 files changed, 7 insertions(+), 44 deletions(-)
>
> diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c
> index cc18459..064e66c 100644
> --- a/src/conf/virsecretobj.c
> +++ b/src/conf/virsecretobj.c
> @@ -86,7 +86,7 @@ virSecretObjOnceInit(void)
>
> VIR_ONCE_GLOBAL_INIT(virSecretObj)
>
> -virSecretObjPtr
> +static virSecretObjPtr
> virSecretObjNew(void)
> {
> virSecretObjPtr secret;
> @@ -158,16 +158,7 @@ virSecretObjListDispose(void *obj)
> }
>
>
> -/**
> - * virSecretObjFindByUUIDLocked:
> - * @secrets: list of secret objects
> - * @uuid: secret uuid to find
> - *
> - * This functions requires @secrets to be locked already!
> - *
> - * Returns: not locked, but ref'd secret object.
> - */
I don't think that we need to remove the documentation, it is also useful for
static function.
Not that it matters, but it's essentially the same as the non-Locked
version except for the piece noted below which I also adjusted to
combine them. I guess I'm following other advice received in other
reviews to reduce the extraneous comments. Since the Locked version is
no longer accessible to anything other than a local consumer that's why
I removed it.
Of course if you see the patches I have in my branch - it may not matter
because the eventual goal is to have FindByUUID and FindByUsage
essentially match and thus be combined into FindByObject type function
where the FindBy is based on a parameter telling the function to look in
a primary hash table or a secondary hash table.
Long term the goal is to have ObjListPtr be a pointer to an Object that
keeps two hash tables - one a primary (UUID for secrets) and one an
optional secondary (Usage for secrets). An object can be in both hash
tables (see how the domain code does this) and lookup goes entirely
through the virHash* functions rather than potentially slow linked list
traversal.
Consider some recent issues with "large numbers" of objects that are in
a forward linked list. When there's 10-50 elements perhaps the lookup
times aren't so bad, but if there's 200, 500, 1000 elements in the list,
then it becomes exponentially slower for every lookup that's at the end
of the list. For some things like "node device" - those could be the
ones that are referenced more frequently too.
If we alter all those drivers to use hash tables lookups and the bulk of
the code is in the form of common/shared object, then not only is lookup
quicker, but we don't have multiple different mechanisms to do the same
thing and we share a lot more code.
Unfortunately the journey to get there is going to be a bit painful with
the volume of patches, but the end result is a lot of common code and
common objects.
> -virSecretObjPtr
> +static virSecretObjPtr
> virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
> const unsigned char *uuid)
> {
> @@ -187,7 +178,7 @@ virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
> * This function locks @secrets and finds the secret object which
> * corresponds to @uuid.
> *
> - * Returns: locked and ref'd secret object.
> + * Returns: locked and ref'd secret object on success, NULL on failure.
Unrelated change.
wellll... it's the combination of the two
I can restore the *Locked comments, but they will disappear later on
perhaps making differences look awful.
Let me know either way...
John
> */
> virSecretObjPtr
> virSecretObjListFindByUUID(virSecretObjListPtr secrets,
> @@ -228,17 +219,7 @@ virSecretObjSearchName(const void *payload,
> }
>
>
> -/**
> - * virSecretObjFindByUsageLocked:
> - * @secrets: list of secret objects
> - * @usageType: secret usageType to find
> - * @usageID: secret usage string
> - *
> - * This functions requires @secrets to be locked already!
> - *
> - * Returns: not locked, but ref'd secret object.
> - */
Same here, we can keep the documentation.
> -virSecretObjPtr
> +static virSecretObjPtr
> virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
> int usageType,
> const char *usageID)
> @@ -263,7 +244,7 @@ virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
> * This function locks @secrets and finds the secret object which
> * corresponds to @usageID of @usageType.
> *
> - * Returns: locked and ref'd secret object.
> + * Returns: locked and ref'd secret object on success, NULL on failure.
Unrelated change.
> */
> virSecretObjPtr
> virSecretObjListFindByUsage(virSecretObjListPtr secrets,
> @@ -320,9 +301,9 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
> *
> * This functions requires @secrets to be locked already!
> *
> - * Returns pointer to secret or NULL if failure to add
> + * Returns: locked secret or NULL if failure to add
Unrelated change.
> */
> -virSecretObjPtr
> +static virSecretObjPtr
> virSecretObjListAddLocked(virSecretObjListPtr secrets,
> virSecretDefPtr def,
> const char *configDir,
> diff --git a/src/conf/virsecretobj.h b/src/conf/virsecretobj.h
> index b26061a..9638b69 100644
> --- a/src/conf/virsecretobj.h
> +++ b/src/conf/virsecretobj.h
> @@ -29,9 +29,6 @@
> typedef struct _virSecretObj virSecretObj;
> typedef virSecretObj *virSecretObjPtr;
>
> -virSecretObjPtr
> -virSecretObjNew(void);
> -
> void
> virSecretObjEndAPI(virSecretObjPtr *secret);
>
> @@ -42,19 +39,10 @@ virSecretObjListPtr
> virSecretObjListNew(void);
>
> virSecretObjPtr
> -virSecretObjListFindByUUIDLocked(virSecretObjListPtr secrets,
> - const unsigned char *uuid);
> -
> -virSecretObjPtr
> virSecretObjListFindByUUID(virSecretObjListPtr secrets,
> const unsigned char *uuid);
>
> virSecretObjPtr
> -virSecretObjListFindByUsageLocked(virSecretObjListPtr secrets,
> - int usageType,
> - const char *usageID);
> -
> -virSecretObjPtr
> virSecretObjListFindByUsage(virSecretObjListPtr secrets,
> int usageType,
> const char *usageID);
> @@ -64,12 +52,6 @@ virSecretObjListRemove(virSecretObjListPtr secrets,
> virSecretObjPtr secret);
>
> virSecretObjPtr
> -virSecretObjListAddLocked(virSecretObjListPtr secrets,
> - virSecretDefPtr def,
> - const char *configDir,
> - virSecretDefPtr *oldDef);
> -
> -virSecretObjPtr
> virSecretObjListAdd(virSecretObjListPtr secrets,
> virSecretDefPtr def,
> const char *configDir,
> --
> 2.9.3
>
> --
> libvir-list mailing list
> libvir-list(a)redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list