[libvirt] [PATCH v2 1/3] libxl: add support for multiple IP addresses
vif-* scripts support it for a long time, and expect addresses to be separated by spaces. Add appropriate support to libxl driver. --- changed in v2: - moved libxlMakeIPList function to xenconfig/xen_common.c and renamed to xenMakeIPList --- src/libxl/libxl_conf.c | 5 +++-- src/libxl/libxl_domain.c | 12 ------------ src/xenconfig/xen_common.c | 24 ++++++++++++++++++++++++ src/xenconfig/xen_common.h | 1 + 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2a9be69..970cff2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -49,6 +49,7 @@ #include "virstoragefile.h" #include "secret_util.h" #include "cpu/cpu.h" +#include "xen_common.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -1144,7 +1145,7 @@ libxlMakeNic(virDomainDefPtr def, if (VIR_STRDUP(x_nic->script, l_nic->script) < 0) goto cleanup; if (l_nic->guestIP.nips > 0) { - x_nic->ip = virSocketAddrFormat(&l_nic->guestIP.ips[0]->address); + x_nic->ip = xenMakeIPList(&l_nic->guestIP); if (!x_nic->ip) goto cleanup; } @@ -1160,7 +1161,7 @@ libxlMakeNic(virDomainDefPtr def, } if (l_nic->guestIP.nips > 0) { - x_nic->ip = virSocketAddrFormat(&l_nic->guestIP.ips[0]->address); + x_nic->ip = xenMakeIPList(&l_nic->guestIP); if (!x_nic->ip) goto cleanup; } diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index d054b07..395c8a9 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -294,18 +294,6 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, def->os.type != VIR_DOMAIN_OSTYPE_HVM) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN; - if (dev->type == VIR_DOMAIN_DEVICE_NET && - (dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE || - dev->data.net->type == VIR_DOMAIN_NET_TYPE_ETHERNET || - dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK)) { - if (dev->data.net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("multiple IP addresses not supported on device type %s"), - virDomainNetTypeToString(dev->data.net->type)); - return -1; - } - } - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV || (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) { diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index ded0aca..7f838b6 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1166,6 +1166,30 @@ xenFormatSerial(virConfValuePtr list, virDomainChrDefPtr serial) return -1; } +char * +xenMakeIPList(virNetDevIPInfoPtr guestIP) +{ + size_t i; + char **address_array; + char *ret = NULL; + + if (VIR_ALLOC_N(address_array, guestIP->nips + 1) < 0) + return NULL; + + for (i = 0; i < guestIP->nips; i++) { + address_array[i] = virSocketAddrFormat(&guestIP->ips[i]->address); + if (!address_array[i]) + goto cleanup; + } + address_array[guestIP->nips] = NULL; + + ret = virStringListJoin((const char**)address_array, " "); + + cleanup: + while (i > 0) + VIR_FREE(address_array[--i]); + return ret; +} static int xenFormatNet(virConnectPtr conn, diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index 9055692..3b7a5db 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -67,6 +67,7 @@ int xenFormatConfigCommon(virConfPtr conf, virConnectPtr conn, const char *nativeFormat); +char *xenMakeIPList(virNetDevIPInfoPtr guestIP); int xenDomainDefAddImplicitInputDevice(virDomainDefPtr def); base-commit: 9f0ccc717ba9026c30ce38951a354dd66fa12e3b -- git-series 0.9.1
--- new patch in v2 --- src/xenconfig/xen_common.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 7f838b6..40b1483 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -847,7 +847,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) while (list) { char model[10]; char type[10]; - char ip[16]; + char ip[128]; char mac[18]; char bridge[50]; char vifname[50]; @@ -964,8 +964,18 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge) < 0) goto cleanup; } - if (ip[0] && virDomainNetAppendIPAddress(net, ip, AF_INET, 0) < 0) - goto cleanup; + if (ip[0]) { + char **ip_list = virStringSplit(ip, " ", 0); + size_t i; + + for (i = 0; ip_list[i]; i++) { + if (virDomainNetAppendIPAddress(net, ip_list[i], 0, 0) < 0) { + virStringListFree(ip_list); + goto cleanup; + } + } + virStringListFree(ip_list); + } if (script && script[0] && VIR_STRDUP(net->script, script) < 0) @@ -1207,14 +1217,10 @@ xenFormatNet(virConnectPtr conn, switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferAsprintf(&buf, ",bridge=%s", net->data.bridge.brname); - if (net->guestIP.nips == 1) { - char *ipStr = virSocketAddrFormat(&net->guestIP.ips[0]->address); + if (net->guestIP.nips > 0) { + char *ipStr = xenMakeIPList(&net->guestIP); virBufferAsprintf(&buf, ",ip=%s", ipStr); VIR_FREE(ipStr); - } else if (net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Driver does not support setting multiple IP addresses")); - goto cleanup; } virBufferAsprintf(&buf, ",script=%s", DEFAULT_VIF_SCRIPT); break; @@ -1222,14 +1228,10 @@ xenFormatNet(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_ETHERNET: if (net->script) virBufferAsprintf(&buf, ",script=%s", net->script); - if (net->guestIP.nips == 1) { - char *ipStr = virSocketAddrFormat(&net->guestIP.ips[0]->address); + if (net->guestIP.nips > 0) { + char *ipStr = xenMakeIPList(&net->guestIP); virBufferAsprintf(&buf, ",ip=%s", ipStr); VIR_FREE(ipStr); - } else if (net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Driver does not support setting multiple IP addresses")); - goto cleanup; } break; -- git-series 0.9.1
On 12/06/2017 07:27 PM, Marek Marczykowski-Górecki wrote:
--- new patch in v2 ---
Sorry for being neurotic when it comes to empty commit messages. I added some useless text before pushing. Same for 3/3.
src/xenconfig/xen_common.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> I've pushed all 3. Thanks! Regards, Jim
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 7f838b6..40b1483 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -847,7 +847,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) while (list) { char model[10]; char type[10]; - char ip[16]; + char ip[128]; char mac[18]; char bridge[50]; char vifname[50]; @@ -964,8 +964,18 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge) < 0) goto cleanup; } - if (ip[0] && virDomainNetAppendIPAddress(net, ip, AF_INET, 0) < 0) - goto cleanup; + if (ip[0]) { + char **ip_list = virStringSplit(ip, " ", 0); + size_t i; + + for (i = 0; ip_list[i]; i++) { + if (virDomainNetAppendIPAddress(net, ip_list[i], 0, 0) < 0) { + virStringListFree(ip_list); + goto cleanup; + } + } + virStringListFree(ip_list); + }
if (script && script[0] && VIR_STRDUP(net->script, script) < 0) @@ -1207,14 +1217,10 @@ xenFormatNet(virConnectPtr conn, switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferAsprintf(&buf, ",bridge=%s", net->data.bridge.brname); - if (net->guestIP.nips == 1) { - char *ipStr = virSocketAddrFormat(&net->guestIP.ips[0]->address); + if (net->guestIP.nips > 0) { + char *ipStr = xenMakeIPList(&net->guestIP); virBufferAsprintf(&buf, ",ip=%s", ipStr); VIR_FREE(ipStr); - } else if (net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Driver does not support setting multiple IP addresses")); - goto cleanup; } virBufferAsprintf(&buf, ",script=%s", DEFAULT_VIF_SCRIPT); break; @@ -1222,14 +1228,10 @@ xenFormatNet(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_ETHERNET: if (net->script) virBufferAsprintf(&buf, ",script=%s", net->script); - if (net->guestIP.nips == 1) { - char *ipStr = virSocketAddrFormat(&net->guestIP.ips[0]->address); + if (net->guestIP.nips > 0) { + char *ipStr = xenMakeIPList(&net->guestIP); virBufferAsprintf(&buf, ",ip=%s", ipStr); VIR_FREE(ipStr); - } else if (net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Driver does not support setting multiple IP addresses")); - goto cleanup; } break;
On 12/08/2017 02:19 PM, Jim Fehlig wrote:
On 12/06/2017 07:27 PM, Marek Marczykowski-Górecki wrote:
--- new patch in v2 ---
Sorry for being neurotic when it comes to empty commit messages.
The improvement you made in this series should be advertised in News. I struggled, but managed to send a patch containing only a summary :-) https://www.redhat.com/archives/libvir-list/2017-December/msg00302.html Regards, Jim
On 12/06/2017 09:27 PM, Marek Marczykowski-Górecki wrote:
--- new patch in v2 --- src/xenconfig/xen_common.c | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 7f838b6..40b1483 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -847,7 +847,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) while (list) { char model[10]; char type[10]; - char ip[16]; + char ip[128]; char mac[18]; char bridge[50]; char vifname[50]; @@ -964,8 +964,18 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def, const char *vif_typename) if (bridge[0] && VIR_STRDUP(net->data.bridge.brname, bridge) < 0) goto cleanup; } - if (ip[0] && virDomainNetAppendIPAddress(net, ip, AF_INET, 0) < 0) - goto cleanup; + if (ip[0]) { + char **ip_list = virStringSplit(ip, " ", 0); + size_t i; + + for (i = 0; ip_list[i]; i++) {
Coverity notes that virStringSplit could return ip_list == NULL, thus causing a problem for the above loop. An "if (!ip_list) goto cleanup;" should take care of that. John
+ if (virDomainNetAppendIPAddress(net, ip_list[i], 0, 0) < 0) { + virStringListFree(ip_list); + goto cleanup; + } + } + virStringListFree(ip_list); + }
if (script && script[0] && VIR_STRDUP(net->script, script) < 0) @@ -1207,14 +1217,10 @@ xenFormatNet(virConnectPtr conn, switch (net->type) { case VIR_DOMAIN_NET_TYPE_BRIDGE: virBufferAsprintf(&buf, ",bridge=%s", net->data.bridge.brname); - if (net->guestIP.nips == 1) { - char *ipStr = virSocketAddrFormat(&net->guestIP.ips[0]->address); + if (net->guestIP.nips > 0) { + char *ipStr = xenMakeIPList(&net->guestIP); virBufferAsprintf(&buf, ",ip=%s", ipStr); VIR_FREE(ipStr); - } else if (net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Driver does not support setting multiple IP addresses")); - goto cleanup; } virBufferAsprintf(&buf, ",script=%s", DEFAULT_VIF_SCRIPT); break; @@ -1222,14 +1228,10 @@ xenFormatNet(virConnectPtr conn, case VIR_DOMAIN_NET_TYPE_ETHERNET: if (net->script) virBufferAsprintf(&buf, ",script=%s", net->script); - if (net->guestIP.nips == 1) { - char *ipStr = virSocketAddrFormat(&net->guestIP.ips[0]->address); + if (net->guestIP.nips > 0) { + char *ipStr = xenMakeIPList(&net->guestIP); virBufferAsprintf(&buf, ",ip=%s", ipStr); VIR_FREE(ipStr); - } else if (net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Driver does not support setting multiple IP addresses")); - goto cleanup; } break;
--- changed in v2: - add tests for xenconfig driver too --- tests/libxlxml2domconfigdata/multiple-ip.json | 37 +++++++++++++++- tests/libxlxml2domconfigdata/multiple-ip.xml | 22 +++++++++- tests/libxlxml2domconfigtest.c | 1 +- tests/xlconfigdata/test-vif-multi-ip.cfg | 25 ++++++++++- tests/xlconfigdata/test-vif-multi-ip.xml | 48 ++++++++++++++++++++- tests/xlconfigtest.c | 1 +- 6 files changed, 134 insertions(+) create mode 100644 tests/libxlxml2domconfigdata/multiple-ip.json create mode 100644 tests/libxlxml2domconfigdata/multiple-ip.xml create mode 100644 tests/xlconfigdata/test-vif-multi-ip.cfg create mode 100644 tests/xlconfigdata/test-vif-multi-ip.xml diff --git a/tests/libxlxml2domconfigdata/multiple-ip.json b/tests/libxlxml2domconfigdata/multiple-ip.json new file mode 100644 index 0000000..4aff61d --- /dev/null +++ b/tests/libxlxml2domconfigdata/multiple-ip.json @@ -0,0 +1,37 @@ +{ + "c_info": { + "type": "pv", + "name": "test-pv", + "uuid": "039e9ee6-4a84-3055-4c81-8ba426ae2656" + }, + "b_info": { + "max_vcpus": 4, + "avail_vcpus": [ + 0, + 1, + 2, + 3 + ], + "max_memkb": 524288, + "target_memkb": 524288, + "sched_params": { + "weight": 1000 + }, + "type.pv": { + "bootloader": "pygrub" + }, + "arch_arm": { + + } + }, + "nics": [ + { + "devid": 0, + "mac": "00:16:3e:3e:86:60", + "ip": "10.0.0.1 2000:abcd::1", + "script": "/etc/xen/scripts/vif-bridge", + "nictype": "vif" + } + ], + "on_reboot": "restart" +} diff --git a/tests/libxlxml2domconfigdata/multiple-ip.xml b/tests/libxlxml2domconfigdata/multiple-ip.xml new file mode 100644 index 0000000..5188935 --- /dev/null +++ b/tests/libxlxml2domconfigdata/multiple-ip.xml @@ -0,0 +1,22 @@ +<domain type='xen'> + <name>test-pv</name> + <uuid>039e9ee6-4a84-3055-4c81-8ba426ae2656</uuid> + <memory>524288</memory> + <currentMemory>524288</currentMemory> + <vcpu>4</vcpu> + <os> + <type arch='x86_64' machine='xenpv'>linux</type> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <interface type='ethernet'> + <ip address='10.0.0.1' family='ipv4'/> + <ip address='2000:abcd::1' family='ipv6'/> + <mac address='00:16:3e:3e:86:60'/> + <script path='/etc/xen/scripts/vif-bridge'/> + </interface> + </devices> +</domain> diff --git a/tests/libxlxml2domconfigtest.c b/tests/libxlxml2domconfigtest.c index c8379f1..bd4c3af 100644 --- a/tests/libxlxml2domconfigtest.c +++ b/tests/libxlxml2domconfigtest.c @@ -190,6 +190,7 @@ mymain(void) DO_TEST("basic-hvm"); DO_TEST("moredevs-hvm"); DO_TEST("vnuma-hvm"); + DO_TEST("multiple-ip"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } diff --git a/tests/xlconfigdata/test-vif-multi-ip.cfg b/tests/xlconfigdata/test-vif-multi-ip.cfg new file mode 100644 index 0000000..4a5d37f --- /dev/null +++ b/tests/xlconfigdata/test-vif-multi-ip.cfg @@ -0,0 +1,25 @@ +name = "XenGuest2" +uuid = "c7a5fdb2-cdaf-9455-926a-d65c16db1809" +maxmem = 579 +memory = 394 +vcpus = 1 +pae = 1 +acpi = 1 +apic = 1 +viridian = 0 +rtc_timeoffset = 0 +localtime = 0 +on_poweroff = "destroy" +on_reboot = "restart" +on_crash = "restart" +device_model = "/usr/lib/xen/bin/qemu-system-i386" +sdl = 0 +vnc = 1 +vncunused = 1 +vnclisten = "127.0.0.1" +vif = [ "mac=00:16:3e:66:92:9c,bridge=xenbr1,ip=10.0.0.1 10.1.1.1 2000::1,script=vif-bridge,type=vif" ] +parallel = "none" +serial = "none" +builder = "hvm" +boot = "d" +disk = [ "format=raw,vdev=hda,access=rw,backendtype=phy,target=/dev/HostVG/XenGuest2" ] diff --git a/tests/xlconfigdata/test-vif-multi-ip.xml b/tests/xlconfigdata/test-vif-multi-ip.xml new file mode 100644 index 0000000..7e831cf --- /dev/null +++ b/tests/xlconfigdata/test-vif-multi-ip.xml @@ -0,0 +1,48 @@ +<domain type='xen'> + <name>XenGuest2</name> + <uuid>c7a5fdb2-cdaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>592896</memory> + <currentMemory unit='KiB'>403456</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='xenfv'>hvm</type> + <loader type='rom'>/usr/lib/xen/boot/hvmloader</loader> + <boot dev='cdrom'/> + </os> + <features> + <acpi/> + <apic/> + <pae/> + </features> + <clock offset='variable' adjustment='0' basis='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>restart</on_crash> + <devices> + <emulator>/usr/lib/xen/bin/qemu-system-i386</emulator> + <disk type='block' device='disk'> + <driver name='phy' type='raw'/> + <source dev='/dev/HostVG/XenGuest2'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='ide' index='0'/> + <interface type='bridge'> + <mac address='00:16:3e:66:92:9c'/> + <source bridge='xenbr1'/> + <ip address='10.0.0.1' family='ipv4'/> + <ip address='10.1.1.1' family='ipv4'/> + <ip address='2000::1' family='ipv6'/> + <script path='vif-bridge'/> + <model type='netfront'/> + </interface> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='-1' autoport='yes' listen='127.0.0.1'> + <listen type='address' address='127.0.0.1'/> + </graphics> + <video> + <model type='cirrus' vram='8192' heads='1' primary='yes'/> + </video> + </devices> +</domain> diff --git a/tests/xlconfigtest.c b/tests/xlconfigtest.c index e6fdfe8..57a0a67 100644 --- a/tests/xlconfigtest.c +++ b/tests/xlconfigtest.c @@ -298,6 +298,7 @@ mymain(void) DO_TEST_FORMAT("fullvirt-direct-kernel-boot-bogus-extra", false); #endif DO_TEST("vif-typename"); + DO_TEST("vif-multi-ip"); DO_TEST("usb"); DO_TEST("usbctrl"); -- git-series 0.9.1
On 12/06/2017 07:27 PM, Marek Marczykowski-Górecki wrote:
vif-* scripts support it for a long time, and expect addresses to be separated by spaces. Add appropriate support to libxl driver.
--- changed in v2: - moved libxlMakeIPList function to xenconfig/xen_common.c and renamed to xenMakeIPList --- src/libxl/libxl_conf.c | 5 +++-- src/libxl/libxl_domain.c | 12 ------------ src/xenconfig/xen_common.c | 24 ++++++++++++++++++++++++ src/xenconfig/xen_common.h | 1 + 4 files changed, 28 insertions(+), 14 deletions(-)
Reviewed-by: Jim Fehlig <jfehlig@suse.com> Regards, Jim
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2a9be69..970cff2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -49,6 +49,7 @@ #include "virstoragefile.h" #include "secret_util.h" #include "cpu/cpu.h" +#include "xen_common.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL @@ -1144,7 +1145,7 @@ libxlMakeNic(virDomainDefPtr def, if (VIR_STRDUP(x_nic->script, l_nic->script) < 0) goto cleanup; if (l_nic->guestIP.nips > 0) { - x_nic->ip = virSocketAddrFormat(&l_nic->guestIP.ips[0]->address); + x_nic->ip = xenMakeIPList(&l_nic->guestIP); if (!x_nic->ip) goto cleanup; } @@ -1160,7 +1161,7 @@ libxlMakeNic(virDomainDefPtr def, }
if (l_nic->guestIP.nips > 0) { - x_nic->ip = virSocketAddrFormat(&l_nic->guestIP.ips[0]->address); + x_nic->ip = xenMakeIPList(&l_nic->guestIP); if (!x_nic->ip) goto cleanup; } diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index d054b07..395c8a9 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -294,18 +294,6 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, def->os.type != VIR_DOMAIN_OSTYPE_HVM) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
- if (dev->type == VIR_DOMAIN_DEVICE_NET && - (dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE || - dev->data.net->type == VIR_DOMAIN_NET_TYPE_ETHERNET || - dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK)) { - if (dev->data.net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("multiple IP addresses not supported on device type %s"), - virDomainNetTypeToString(dev->data.net->type)); - return -1; - } - } - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV || (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) { diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index ded0aca..7f838b6 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1166,6 +1166,30 @@ xenFormatSerial(virConfValuePtr list, virDomainChrDefPtr serial) return -1; }
+char * +xenMakeIPList(virNetDevIPInfoPtr guestIP) +{ + size_t i; + char **address_array; + char *ret = NULL; + + if (VIR_ALLOC_N(address_array, guestIP->nips + 1) < 0) + return NULL; + + for (i = 0; i < guestIP->nips; i++) { + address_array[i] = virSocketAddrFormat(&guestIP->ips[i]->address); + if (!address_array[i]) + goto cleanup; + } + address_array[guestIP->nips] = NULL; + + ret = virStringListJoin((const char**)address_array, " "); + + cleanup: + while (i > 0) + VIR_FREE(address_array[--i]); + return ret; +}
static int xenFormatNet(virConnectPtr conn, diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index 9055692..3b7a5db 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -67,6 +67,7 @@ int xenFormatConfigCommon(virConfPtr conf, virConnectPtr conn, const char *nativeFormat);
+char *xenMakeIPList(virNetDevIPInfoPtr guestIP);
int xenDomainDefAddImplicitInputDevice(virDomainDefPtr def);
base-commit: 9f0ccc717ba9026c30ce38951a354dd66fa12e3b
On 12/06/2017 09:27 PM, Marek Marczykowski-Górecki wrote:
vif-* scripts support it for a long time, and expect addresses to be separated by spaces. Add appropriate support to libxl driver.
--- changed in v2: - moved libxlMakeIPList function to xenconfig/xen_common.c and renamed to xenMakeIPList --- src/libxl/libxl_conf.c | 5 +++-- src/libxl/libxl_domain.c | 12 ------------ src/xenconfig/xen_common.c | 24 ++++++++++++++++++++++++ src/xenconfig/xen_common.h | 1 + 4 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index 2a9be69..970cff2 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -49,6 +49,7 @@ #include "virstoragefile.h" #include "secret_util.h" #include "cpu/cpu.h" +#include "xen_common.h"
#define VIR_FROM_THIS VIR_FROM_LIBXL @@ -1144,7 +1145,7 @@ libxlMakeNic(virDomainDefPtr def, if (VIR_STRDUP(x_nic->script, l_nic->script) < 0) goto cleanup; if (l_nic->guestIP.nips > 0) { - x_nic->ip = virSocketAddrFormat(&l_nic->guestIP.ips[0]->address); + x_nic->ip = xenMakeIPList(&l_nic->guestIP); if (!x_nic->ip) goto cleanup; } @@ -1160,7 +1161,7 @@ libxlMakeNic(virDomainDefPtr def, }
if (l_nic->guestIP.nips > 0) { - x_nic->ip = virSocketAddrFormat(&l_nic->guestIP.ips[0]->address); + x_nic->ip = xenMakeIPList(&l_nic->guestIP); if (!x_nic->ip) goto cleanup; } diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index d054b07..395c8a9 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -294,18 +294,6 @@ libxlDomainDeviceDefPostParse(virDomainDeviceDefPtr dev, def->os.type != VIR_DOMAIN_OSTYPE_HVM) dev->data.chr->targetType = VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_XEN;
- if (dev->type == VIR_DOMAIN_DEVICE_NET && - (dev->data.net->type == VIR_DOMAIN_NET_TYPE_BRIDGE || - dev->data.net->type == VIR_DOMAIN_NET_TYPE_ETHERNET || - dev->data.net->type == VIR_DOMAIN_NET_TYPE_NETWORK)) { - if (dev->data.net->guestIP.nips > 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("multiple IP addresses not supported on device type %s"), - virDomainNetTypeToString(dev->data.net->type)); - return -1; - } - } - if (dev->type == VIR_DOMAIN_DEVICE_HOSTDEV || (dev->type == VIR_DOMAIN_DEVICE_NET && dev->data.net->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)) { diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index ded0aca..7f838b6 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -1166,6 +1166,30 @@ xenFormatSerial(virConfValuePtr list, virDomainChrDefPtr serial) return -1; }
+char * +xenMakeIPList(virNetDevIPInfoPtr guestIP) +{ + size_t i; + char **address_array; + char *ret = NULL; + + if (VIR_ALLOC_N(address_array, guestIP->nips + 1) < 0) + return NULL; + + for (i = 0; i < guestIP->nips; i++) { + address_array[i] = virSocketAddrFormat(&guestIP->ips[i]->address); + if (!address_array[i]) + goto cleanup; + } + address_array[guestIP->nips] = NULL; + + ret = virStringListJoin((const char**)address_array, " "); + + cleanup: + while (i > 0) + VIR_FREE(address_array[--i]);
Coverity notes that address_array is leaked. May I sugguest "virStringListFree()" on address array? John
+ return ret; +}
static int xenFormatNet(virConnectPtr conn, diff --git a/src/xenconfig/xen_common.h b/src/xenconfig/xen_common.h index 9055692..3b7a5db 100644 --- a/src/xenconfig/xen_common.h +++ b/src/xenconfig/xen_common.h @@ -67,6 +67,7 @@ int xenFormatConfigCommon(virConfPtr conf, virConnectPtr conn, const char *nativeFormat);
+char *xenMakeIPList(virNetDevIPInfoPtr guestIP);
int xenDomainDefAddImplicitInputDevice(virDomainDefPtr def);
base-commit: 9f0ccc717ba9026c30ce38951a354dd66fa12e3b
On Mon, Dec 11, 2017 at 07:58:54AM -0500, John Ferlan wrote:
+char * +xenMakeIPList(virNetDevIPInfoPtr guestIP) +{ + size_t i; + char **address_array; + char *ret = NULL; + + if (VIR_ALLOC_N(address_array, guestIP->nips + 1) < 0) + return NULL; + + for (i = 0; i < guestIP->nips; i++) { + address_array[i] = virSocketAddrFormat(&guestIP->ips[i]->address); + if (!address_array[i]) + goto cleanup; + } + address_array[guestIP->nips] = NULL; + + ret = virStringListJoin((const char**)address_array, " "); + + cleanup: + while (i > 0) + VIR_FREE(address_array[--i]);
Coverity notes that address_array is leaked. May I sugguest "virStringListFree()" on address array?
Then I should initialize each entry to NULL first (which will be overridden a moment later). Is it ok? -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?
On 12/11/2017 08:37 AM, Marek Marczykowski-Górecki wrote:
On Mon, Dec 11, 2017 at 07:58:54AM -0500, John Ferlan wrote:
+char * +xenMakeIPList(virNetDevIPInfoPtr guestIP) +{ + size_t i; + char **address_array; + char *ret = NULL; + + if (VIR_ALLOC_N(address_array, guestIP->nips + 1) < 0) + return NULL; + + for (i = 0; i < guestIP->nips; i++) { + address_array[i] = virSocketAddrFormat(&guestIP->ips[i]->address); + if (!address_array[i]) + goto cleanup; + } + address_array[guestIP->nips] = NULL; + + ret = virStringListJoin((const char**)address_array, " "); + + cleanup: + while (i > 0) + VIR_FREE(address_array[--i]);
Coverity notes that address_array is leaked. May I sugguest "virStringListFree()" on address array?
Then I should initialize each entry to NULL first (which will be overridden a moment later). Is it ok?
Not sure I understand the question as VIR_ALLOC_N allocates address_array with guestIP->nips + 1 NULL 'char *' entries. Then your for loop fills the entries[i].... The "address_array[guestIP->nips] = NULL;" would seem superfluous too I guess. I wasn't initially looking beyond the memory leak. There's plenty of examples using VIR_ALLOC_N in the code that you can see how each array entry is free'd as well as the containing structure. John
On Mon, Dec 11, 2017 at 08:55:50AM -0500, John Ferlan wrote:
On 12/11/2017 08:37 AM, Marek Marczykowski-Górecki wrote:
On Mon, Dec 11, 2017 at 07:58:54AM -0500, John Ferlan wrote:
+char * +xenMakeIPList(virNetDevIPInfoPtr guestIP) +{ + size_t i; + char **address_array; + char *ret = NULL; + + if (VIR_ALLOC_N(address_array, guestIP->nips + 1) < 0) + return NULL; + + for (i = 0; i < guestIP->nips; i++) { + address_array[i] = virSocketAddrFormat(&guestIP->ips[i]->address); + if (!address_array[i]) + goto cleanup; + } + address_array[guestIP->nips] = NULL; + + ret = virStringListJoin((const char**)address_array, " "); + + cleanup: + while (i > 0) + VIR_FREE(address_array[--i]);
Coverity notes that address_array is leaked. May I sugguest "virStringListFree()" on address array?
Then I should initialize each entry to NULL first (which will be overridden a moment later). Is it ok?
Not sure I understand the question as VIR_ALLOC_N allocates address_array with guestIP->nips + 1 NULL 'char *' entries. Then your for loop fills the entries[i].... The "address_array[guestIP->nips] = NULL;" would seem superfluous too I guess. I wasn't initially looking beyond the memory leak. There's plenty of examples using VIR_ALLOC_N in the code that you can see how each array entry is free'd as well as the containing structure.
Ah, I've missed the part that VIR_ALLOC_N initialize memory with zeros. -- Best Regards, Marek Marczykowski-Górecki Invisible Things Lab A: Because it messes up the order in which people normally read text. Q: Why is top-posting such a bad thing?
participants (3)
-
Jim Fehlig -
John Ferlan -
Marek Marczykowski-Górecki