[Libvirt-cim] [PATCH] Add default network card existence checking

[John Ferlan]

On 07/31/2013 09:27 PM, Xu Wang wrote:
> From: Xu Wang <cngesaint at gmail.com>
> 
> The default network card (used as forward device in network pool) name
> was set as "eth0" in Virt_SettingDefineCapabilities.c. This patch added
> check if there is such a network card exists in the network info list.
> If it exists, use it. If not, the default network card would be changed
> into the first available one except lo.

s/except/accept/


As an aside, generally speaking going through a list of output could be
a lengthy process and does have side affects.  I would think there'd be
a way using perhaps socket/bind/getsockname in order to determine the
NIC in use on the host. It just seems there has to be a better way than
the loop and compare.

> 
> Signed-off-by: Xu Wang <gesaint at linux.vnet.ibm.com>
> ---
>  libxkutil/misc_util.c                 |   38 +++++++++++++++++++++++++++++++++
>  libxkutil/misc_util.h                 |    2 +-
>  src/Virt_SettingsDefineCapabilities.c |    8 ++++++-
>  3 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c
> index 9e7e0d5..ada664c 100644
> --- a/libxkutil/misc_util.c
> +++ b/libxkutil/misc_util.c
> @@ -900,6 +900,44 @@ int virt_set_status(const CMPIBroker *broker,
>          return ret;
>  }
>  
> +char *get_avai_net(char *def_name)

s/avai/avail/ ?? I think this would change throughout the function

s/char *def_name/const char* def_name/

> +{
> +    char buf[512];
> +    FILE *pp;
> +    char *delims = ": ";
> +    char *sub_str = NULL;
> +    char *avai_net = NULL;
> +    bool avai_found = false;
> +
> +    pp = popen("ip addr", "r");
> +    if (!pp) {
> +        CU_DEBUG("popen() error.\n");
> +        return NULL;
> +    }
> +
> +    while (fgets(buf, sizeof(buf), pp)) {
> +        sub_str = strtok(buf, delims);
> +        while (sub_str != NULL) {
> +            if (!strncmp(sub_str, "eth", 3) || !strncmp(sub_str, "em", 2)) {
> +                if (!strcmp(def_name, sub_str)) {

if (avai_found) free(avai_net);

> +                    avai_net = strdup(sub_str);
> +                    goto out;
> +                }
> +
> +                if (!avai_found) {
> +                    avai_net = strdup(sub_str);
> +                    avai_found = true;
> +                }
> +            }
> +            sub_str = strtok(NULL, delims);
> +        }
> +    }
> +    pclose(pp);

This should move to after out: ; otherwise, we leak pp when we goto out
above.

> +
> +out:
> +    return avai_net;
> +}
> +
>  
>  /*
>   * Local Variables:
> diff --git a/libxkutil/misc_util.h b/libxkutil/misc_util.h
> index fd4f191..fc63000 100644
> --- a/libxkutil/misc_util.h
> +++ b/libxkutil/misc_util.h
> @@ -157,7 +157,7 @@ const char *get_mig_ssh_tmp_key(void);
>  bool get_disable_kvm(void);
>  const char *get_lldptool_query_options(void);
>  const char *get_vsi_support_key_string(void);
> -
> +char *get_avai_net(char *def_name);

s/char *def_name/const char *def_name)

>  /*
>   * Local Variables:
>   * mode: C
> diff --git a/src/Virt_SettingsDefineCapabilities.c b/src/Virt_SettingsDefineCapabilities.c
> index 78c128c..09ae49f 100644
> --- a/src/Virt_SettingsDefineCapabilities.c
> +++ b/src/Virt_SettingsDefineCapabilities.c
> @@ -777,6 +777,7 @@ static CMPIStatus set_net_pool_props(const CMPIObjectPath *ref,
>          int dev_count;
>          int i;
>          char *tmp_str = NULL;
> +        char *forward_device = "eth0";

s/"eth0"/NULL/

>  
>          /* Isolated network pools don't have a forward device */
>          if (pool_type == NETPOOL_FORWARD_NONE)
> @@ -836,8 +837,13 @@ static CMPIStatus set_net_pool_props(const CMPIObjectPath *ref,
>                                (CMPIValue *)&pool_type, CMPI_uint16);
>  
>                  if (i == 1) {
> +                        forward_device = get_avai_net(forward_device);
s/(forward_device)/("eth0")
> +                        if (!forward_device) {
> +                            goto out;
> +                        }
> +
>                          CMSetProperty(inst, "ForwardDevice",
> -                                      (CMPIValue *)"eth0", CMPI_chars);
> +                                      (CMPIValue *)forward_device, CMPI_chars);

I think you should free(forward_device) here

>                  }
>  
>                  inst_list_add(list, inst);
>