Re: [PATCH v2 5/8] util: Introduce virObjectPoolableHashElement

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

 



On Fri, Jun 02, 2017 at 06:17:19 -0400, John Ferlan wrote:
> Add a new virObjectLockable child which will be used to more generically
> describe driver objects. Eventually these objects will be placed into a
> more generic hash table object which will take care of object mgmt functions.
> 
> Each virObjectPoolableHashElement will have a primaryKey (required) and a
> secondaryKey (optional) which can be used to insert the same object into
> two hash tables for faster lookups.
> 
> Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/virobject.c     | 76 +++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virobject.h     | 17 +++++++++++
>  3 files changed, 94 insertions(+), 1 deletion(-)

[...]

>      VIR_OBJECT_USAGE_PRINT_WARNING(anyobj, virObjectLockableClass);
> diff --git a/src/util/virobject.h b/src/util/virobject.h
> index f4c292b..e29dae7 100644
> --- a/src/util/virobject.h
> +++ b/src/util/virobject.h

[...]

> @@ -59,9 +62,17 @@ struct _virObjectLockable {
>      virMutex lock;
>  };
>  
> +struct _virObjectPoolableHashElement {
> +    virObjectLockable parent;
> +
> +    char *primaryKey;
> +    char *secondaryKey;
> +};

I'm afraid that this abstraction is going too far.

Putting dissimilar objects into a single hash does not really make sense
in any way in libvirt. Without the need to put dissimilar objects into a
single hash I don't really see value in abstracting the identifiers of
the objects into opaque things like 'primaryKey'.

Refering to the objects by these oaque terms will cause confusion, and
thus will still require wrappers to de-confuse readers of the code.

An additional worry is the optionality of the secondary key. This hints
that the objects are so dissimilar that they don't have two names in
common.

Attachment: signature.asc
Description: PGP signature

--
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]

  Powered by Linux