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@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; }