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

Re: [libvirt] [PATCH v2 7/8] util: Introduce virObjectPoolableDef



On Fri, Jun 02, 2017 at 06:17:21 -0400, John Ferlan wrote:
> Add a new virObjectPoolableHashElement child which will be used to provide
> the basis for driver def/newDef elements.
> 
> Each virObjectPoolableDef has:
> 
>   1. A required @primaryKey to be used to uniquely identity the object
>      by some string value.
>   2. An optional @secondaryKey to be used as a secondary means of search
>      for the object by some string value.
>   3. A required @def and @defFreeFunc. The @def will be consumed by the
>      object and when disposed the free function will be called.
> 
> The _virObjectPoolableDef has an additional @newDef element to store
> the "next" boot configuration for consumers that support the functionality.
> 
> Signed-off-by: John Ferlan <jferlan redhat com>
> ---
>  src/libvirt_private.syms |  2 ++
>  src/util/virobject.c     | 83 +++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virobject.h     | 24 ++++++++++++++
>  3 files changed, 108 insertions(+), 1 deletion(-)

[...]

> @@ -69,10 +72,22 @@ struct _virObjectPoolableHashElement {
>      char *secondaryKey;
>  };
>  
> +struct _virObjectPoolableDef {
> +    virObjectPoolableHashElement parent;
> +
> +    /* 'def' is the current config definition.
> +     * 'newDef' is the next boot configuration.

'boot' does not really apply to anything besides VMs.

> +     */
> +    void *def;
> +    void *newDef;
> +    virFreeCallback defFreeFunc;
> +};

Okay, this is another sign that this abstraction has gone too far. Using
void pointers here for the definition pointers really does not help
stuff. You need wrappers to do compile time type safety checks here, so
I don't really see the value to wrap it into a object with such
opaqueness.

Attachment: signature.asc
Description: PGP signature


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