[libvirt] [PATCH V3 01/24] src/xenxs:Refactor code parsing memory config

[Jim Fehlig]

David Kiarie wrote:
> From: Kiarie Kahurani <davidkiarie4 at gmail.com>
>   

Ok, I think we are getting close with all the refactoring work.  Some
nits that apply to all the patches:

1) Please add a space between the component and comment in the commit
summary.  E.g.

src/xenxs: Refactor code parsing memory config

2) I think your gitconfig needs needs some adjustment.  Author is
identified as Kiarie Kahurani but your SOB contains David Kiarie.  Do
you have something like the following in you .gitconfig?

[user]
        name = Kiarie Kahurani
        email = davidkiarie4 at gmail.com
...
[format]
        signoff = true

This should ensure the authorship and SOB are filled in properly.

> introduce function
>   xenParseXMMem(virConfPtr conf,.........);
> which parses memory config instead
>
> signed-off-by: David Kiarie<davidkiarie4 at gmail.com>
>   

git will populate the SOB for you with 'signoff = true', ensuring the
's' is capitalized and there is a space between your name and email address.

> ---
>  src/xenxs/xen_xm.c | 83 +++++++++++++++++++++++++++++-------------------------
>  1 file changed, 45 insertions(+), 38 deletions(-)
>
> diff --git a/src/xenxs/xen_xm.c b/src/xenxs/xen_xm.c
> index 4461654..443e6da 100644
> --- a/src/xenxs/xen_xm.c
> +++ b/src/xenxs/xen_xm.c
> @@ -40,11 +40,9 @@
>  #include "virstoragefile.h"
>  #include "virstring.h"
>  
> -/* Convenience method to grab a long int from the config file object */
> -static int xenXMConfigGetBool(virConfPtr conf,
> -                              const char *name,
> -                              int *value,
> -                              int def)
> +/* Convenience method to grab a int from the config file object */
> +static int
> +xenXMConfigGetBool(virConfPtr conf, const char *name, int *value, int def)
>   

Hehe, I meant to follow the prevailing practice only in new functions
you are adding.  Changing whitespace around existing functions should be
done with a separate cleanup patch later.

>  {
>      virConfValuePtr val;
>  
> @@ -67,11 +65,10 @@ static int xenXMConfigGetBool(virConfPtr conf,
>  }
>  
>  
> -/* Convenience method to grab a int from the config file object */
> -static int xenXMConfigGetULong(virConfPtr conf,
> -                               const char *name,
> -                               unsigned long *value,
> -                               unsigned long def)
> +/* Convenience method to grab a long int from the config file object */
> +static int
> +xenXMConfigGetULong(virConfPtr conf, const char *name, unsigned long *value,
> +                    unsigned long def)
>  {
>      virConfValuePtr val;
>  
> @@ -99,10 +96,9 @@ static int xenXMConfigGetULong(virConfPtr conf,
>  
>  
>  /* Convenience method to grab a int from the config file object */
> -static int xenXMConfigGetULongLong(virConfPtr conf,
> -                                   const char *name,
> -                                   unsigned long long *value,
> -                                   unsigned long long def)
> +static int
> +xenXMConfigGetULongLong(virConfPtr conf, const char *name,
> +                        unsigned long long *value, unsigned long long def)
>  {
>      virConfValuePtr val;
>  
> @@ -130,10 +126,9 @@ static int xenXMConfigGetULongLong(virConfPtr conf,
>  
>  
>  /* Convenience method to grab a string from the config file object */
> -static int xenXMConfigGetString(virConfPtr conf,
> -                                const char *name,
> -                                const char **value,
> -                                const char *def)
> +static int
> +xenXMConfigGetString(virConfPtr conf, const char *name, const char **value,
> +                     const char *def)
>  {
>      virConfValuePtr val;
>  
> @@ -155,10 +150,10 @@ static int xenXMConfigGetString(virConfPtr conf,
>      return 0;
>  }
>  
> -static int xenXMConfigCopyStringInternal(virConfPtr conf,
> -                                         const char *name,
> -                                         char **value,
> -                                         int allowMissing)
> +
> +static int
> +xenXMConfigCopyStringInternal(virConfPtr conf, const char *name, char **value,
> +                              int allowMissing)
>  {
>      virConfValuePtr val;
>  
> @@ -188,15 +183,16 @@ static int xenXMConfigCopyStringInternal(virConfPtr conf,
>  }
>  
>  
> -static int xenXMConfigCopyString(virConfPtr conf,
> -                                 const char *name,
> -                                 char **value) {
> +static int
> +xenXMConfigCopyString(virConfPtr conf, const char *name, char **value)
> +{
>      return xenXMConfigCopyStringInternal(conf, name, value, 0);
>  }
>  
> -static int xenXMConfigCopyStringOpt(virConfPtr conf,
> -                                    const char *name,
> -                                    char **value) {
> +
> +static int
> +xenXMConfigCopyStringOpt(virConfPtr conf, const char *name, char **value)
> +{
>   

Looks good after removing these changes which have nothing to do with
"Refactor code parsing memory config".  Removing these will require some
rebasing of later patches in the series.

Can you post a V4 after fixing the nits and removing the unrelated
changes in this patch?  Thanks!

Regards,
Jim

Oh, btw, no need to cc other libvirt devs on these patches.  eblake,
danpb, etc. all read the list :-).

>      return xenXMConfigCopyStringInternal(conf, name, value, 1);
>  }
>  
> @@ -244,6 +240,25 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char *uuid)
>      return 0;
>  }
>  
> +
> +static int
> +xenParseXMMem(virConfPtr conf, virDomainDefPtr def)
> +{
> +    if (xenXMConfigGetULongLong(conf, "memory", &def->mem.cur_balloon,
> +                                MIN_XEN_GUEST_SIZE * 2) < 0)
> +        return -1;
> +
> +    if (xenXMConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon,
> +                                def->mem.cur_balloon) < 0)
> +        return -1;
> +
> +    def->mem.cur_balloon *= 1024;
> +    def->mem.max_balloon *= 1024;
> +
> +    return 0;
> +}
> +
> +
>  #define MAX_VFB 1024
>  /*
>   * Turn a config record into a lump of XML describing the
> @@ -251,7 +266,7 @@ xenXMConfigGetUUID(virConfPtr conf, const char *name, unsigned char *uuid)
>   */
>  virDomainDefPtr
>  xenParseXM(virConfPtr conf, int xendConfigVersion,
> -                       virCapsPtr caps)
> +           virCapsPtr caps)
>  {
>      const char *str;
>      int hvm = 0;
> @@ -360,17 +375,9 @@ xenParseXM(virConfPtr conf, int xendConfigVersion,
>          }
>      }
>  
> -    if (xenXMConfigGetULongLong(conf, "memory", &def->mem.cur_balloon,
> -                                MIN_XEN_GUEST_SIZE * 2) < 0)
> -        goto cleanup;
> -
> -    if (xenXMConfigGetULongLong(conf, "maxmem", &def->mem.max_balloon,
> -                                def->mem.cur_balloon) < 0)
> +    if (xenParseXMMem(conf, def) < 0)
>          goto cleanup;
>  
> -    def->mem.cur_balloon *= 1024;
> -    def->mem.max_balloon *= 1024;
> -
>      if (xenXMConfigGetULong(conf, "vcpus", &count, 1) < 0 ||
>          MAX_VIRT_CPUS < count)
>          goto cleanup;
>