[Libvirt-cim] [PATCH V2] Add default network card existence checking
John Ferlan
jferlan at redhat.com
Thu Aug 8 19:30:17 UTC 2013
On 08/07/2013 04:46 AM, Xu Wang wrote:
> 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 one in the network devices list.
>
> Signed-off-by: Xu Wang <gesaint at linux.vnet.ibm.com>
Something is not quite right - I got the following result
when using this code and the RASD test
--------------------------------------------------------------------
RASD - 06_parent_net_pool.py: FAIL
ERROR - Exception details: Mismatching Mode and device values, Got [('None', 0L), ('None', 1L), ('tun0', 1L), ('None', 2L), ('tun0', 2L)], Expected [('None', 0L), ('None', 1L), ('lo', 1L), ('None', 2L), ('lo', 2L)]
--------------------------------------------------------------------
I seem to have had some other failures afterwards which caused
libvirtd to crash, but that could be because I'm running with top
of tree and other changes...
If I cut out your code and run it in it's own program passing "eth0"
as a parameter here's what I get (although em1 is what I'd be
expecting to return):
interface num = 4
network device name: tun0
network device name: virbr0
network device name: em1
network device name: lo
forward_device = lo
I have Fedora 19.. and here's some ip addr results:
1: lo: <LOOPBACK,UP,LOWER_UP> mtu 65536 qdisc noqueue state UNKNOWN
2: em1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UP qlen 1000
3: wlp3s0: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN qlen 1000
4: virbr0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP
5: vnet0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master virbr0 state UNKNOWN qlen 500
6: tun0: <POINTOPOINT,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast state UNKNOWN qlen 100
8: vnet1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master virbr0 state UNKNOWN qlen 500
and ifconfig:
em1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
lo: flags=73<UP,LOOPBACK,RUNNING> mtu 65536
tun0: flags=4305<UP,POINTOPOINT,RUNNING,NOARP,MULTICAST> mtu 1500
virbr0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
vnet0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
vnet1: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
Specific comments below
> ---
> libxkutil/misc_util.c | 39 +++++++++++++++++++++++++++++++++
> libxkutil/misc_util.h | 2 +-
> src/Virt_SettingsDefineCapabilities.c | 6 ++++-
> 3 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/libxkutil/misc_util.c b/libxkutil/misc_util.c
> index 9e7e0d5..7541aa3 100644
> --- a/libxkutil/misc_util.c
> +++ b/libxkutil/misc_util.c
> @@ -33,6 +33,8 @@
> #include <errno.h>
> #include <libvirt/libvirt.h>
> #include <libvirt/virterror.h>
> +#include <net/if.h>
> +#include <sys/ioctl.h>
>
> #include "cmpidt.h"
> #include "cmpift.h"
> @@ -900,6 +902,43 @@ int virt_set_status(const CMPIBroker *broker,
> return ret;
> }
>
> +char *get_avail_net(const char *def_name)
> +{
> + int fd;
> + int interfaceNum = 0;
> + struct ifreq buf[16];
> + struct ifconf ifc;
> + char *avail_name = NULL;
> +
^
'git am' complains about trailing whitespace
> + if ((fd = socket(AF_INET, SOCK_DGRAM, 0)) < 0) {
> + CU_DEBUG("socket() failed.");
> + goto out;
Need to add extra space before each line
> + }
> +
^
'git am' complains about trailing whitespace
> + ifc.ifc_len = sizeof(buf);
> + ifc.ifc_buf = (caddr_t)buf;
If you're going to go this route SIOCGIFCOUNT will get you
the interface count so you don't have use a constant 16 value
which could be too limiting.
> + if (!ioctl(fd, SIOCGIFCONF, (char *)&ifc)) {
> + interfaceNum = ifc.ifc_len / sizeof(struct ifreq);
> + CU_DEBUG("interface num = %d", interfaceNum);
Unfortunately you know nothing "about" the interface - just that it
exists and that's your sole criteria for selecting. If you don't find
the one that's passed in you select the last one in the list - which
happens to be loopback (lo) which certainly isn't what you want.
You can look at libvirt source code src/util/virnetdev.c for some sample
processing - get the names, get their flags, get other state/date info
about them.
I think the method I had asked for before was to use bind to INADDR_ANY
and socket/bind. It's been a while since I've had to do this so I'm
not sure of the best mechanism to use to solve the problem to find a
network interface card that can/should be used for this.
One would think there'd be an easier way to get what is wanted here!
> + while (interfaceNum-- > 0) {
> + CU_DEBUG("network device name: %s", buf[interfaceNum].ifr_name);
> + if (!strcmp(def_name, buf[interfaceNum].ifr_name)) {
> + avail_name = strdup(buf[interfaceNum].ifr_name);
> + goto out;
> + }
> + }
> +
> + avail_name = strdup(buf[0].ifr_name);
> + } else {
> + CU_DEBUG("ioctl: %s [%s:%d]\n", strerror(errno), __FILE__, __LINE__);
> + goto out;
Need to remove extra space before each of the lines above
> + }
> +
^
'git am' complains about trailing whitespace
> +out:
> + close(fd);
if (fd >= 0)
close(fd);
> + return avail_name;
> +}
> +
>
> /*
> * Local Variables:
> diff --git a/libxkutil/misc_util.h b/libxkutil/misc_util.h
> index fd4f191..9489a18 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_avail_net(const char *def_name);
> /*
> * Local Variables:
> * mode: C
> diff --git a/src/Virt_SettingsDefineCapabilities.c b/src/Virt_SettingsDefineCapabilities.c
> index 78c128c..817980c 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 = NULL;
>
> /* Isolated network pools don't have a forward device */
> if (pool_type == NETPOOL_FORWARD_NONE)
> @@ -836,14 +837,17 @@ static CMPIStatus set_net_pool_props(const CMPIObjectPath *ref,
> (CMPIValue *)&pool_type, CMPI_uint16);
>
> if (i == 1) {
> + forward_device = get_avail_net("eth0");
> +
> CMSetProperty(inst, "ForwardDevice",
> - (CMPIValue *)"eth0", CMPI_chars);
> + (CMPIValue *)forward_device, CMPI_chars);
> }
>
> inst_list_add(list, inst);
> }
>
> out:
> + free(forward_device);
> return s;
> }
>
>
More information about the Libvirt-cim
mailing list