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

Re: [libvirt] [PATCH 2/8] domain: conf: Export virDomainDefPostParseDevices



$SUBJ:  "domain: Introduce virDomainDefPostParseDevices"

On 03/08/2016 11:36 AM, Cole Robinson wrote:
> We will use this in upcoming patches

Export the virDomainDefPostParseDevices rather than having it only be
callable during virDomainDefPostParse. Future patches will be able to
make use of the optimization.

> ---
>  src/conf/domain_conf.c   | 33 +++++++++++++++++++++++----------
>  src/conf/domain_conf.h   |  5 +++++
>  src/libvirt_private.syms |  1 +
>  3 files changed, 29 insertions(+), 10 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 40b1929..bc4e369 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -4191,12 +4191,11 @@ virDomainDefPostParseDeviceIterator(virDomainDefPtr def ATTRIBUTE_UNUSED,
>                                         data->parseFlags, data->xmlopt);
>  }
>  
> -
>  int
> -virDomainDefPostParse(virDomainDefPtr def,
> -                      virCapsPtr caps,
> -                      unsigned int parseFlags,
> -                      virDomainXMLOptionPtr xmlopt)
> +virDomainDefPostParseDevices(virDomainDefPtr def,
> +                             virCapsPtr caps,
> +                             unsigned int parseFlags,
> +                             virDomainXMLOptionPtr xmlopt)
>  {
>      int ret;
>      struct virDomainDefPostParseDeviceIteratorData data = {
> @@ -4206,6 +4205,23 @@ virDomainDefPostParse(virDomainDefPtr def,
>          .parseFlags = parseFlags,
>      };
>  
> +    if ((ret = virDomainDeviceInfoIterateInternal(def,
> +                                                  virDomainDefPostParseDeviceIterator,
> +                                                  true,
> +                                                  &data)) < 0)
> +        return ret;
> +
> +    return 0;

I think you could just "return virDomainDeviceInfoIterateInternal(...);"

It only returns 0 or -1 anyway.

I'm not sure this is necessary - it works, but do we really want to open
up just device post processing? Beyond your usage in patch 3, is there
another need for it? My concern being missing some subsequent domain
check such as virDomainDefPostParseInternal (or worse needing to add
code somewhere that eventually adds a call here).  The rest of my
thoughts are in patch 3...

John

> +}
> +
> +int
> +virDomainDefPostParse(virDomainDefPtr def,
> +                      virCapsPtr caps,
> +                      unsigned int parseFlags,
> +                      virDomainXMLOptionPtr xmlopt)
> +{
> +    int ret;
> +
>      /* call the domain config callback */
>      if (xmlopt->config.domainPostParseCallback) {
>          ret = xmlopt->config.domainPostParseCallback(def, caps, parseFlags,
> @@ -4215,13 +4231,10 @@ virDomainDefPostParse(virDomainDefPtr def,
>      }
>  
>      /* iterate the devices */
> -    if ((ret = virDomainDeviceInfoIterateInternal(def,
> -                                                  virDomainDefPostParseDeviceIterator,
> -                                                  true,
> -                                                  &data)) < 0)
> +    if ((ret = virDomainDefPostParseDevices(def, caps,
> +                                            parseFlags, xmlopt)) < 0)
>          return ret;
>  
> -
>      if ((ret = virDomainDefPostParseInternal(def, caps, parseFlags)) < 0)
>          return ret;
>  
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 6f044d2..aba53a2 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2476,6 +2476,11 @@ virDomainDefPostParse(virDomainDefPtr def,
>                        virCapsPtr caps,
>                        unsigned int parseFlags,
>                        virDomainXMLOptionPtr xmlopt);
> +int
> +virDomainDefPostParseDevices(virDomainDefPtr def,
> +                             virCapsPtr caps,
> +                             unsigned int parseFlags,
> +                             virDomainXMLOptionPtr xmlopt);
>  
>  static inline bool
>  virDomainObjIsActive(virDomainObjPtr dom)
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index 5399117..51d2721 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -235,6 +235,7 @@ virDomainDefParseFile;
>  virDomainDefParseNode;
>  virDomainDefParseString;
>  virDomainDefPostParse;
> +virDomainDefPostParseDevices;
>  virDomainDefSetMemoryInitial;
>  virDomainDefSetMemoryTotal;
>  virDomainDefSetVcpus;
> 


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