8:27 p.m.
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
8:27 p.m.
New subject: [libvirt] [PATCH v2 2/3] xenconfig: add support for multiple IP addresses
---
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
3:19 p.m.
New subject: [libvirt] [PATCH v2 2/3] xenconfig: add support for multiple IP addresses
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(a)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;
3:39 p.m.
New subject: [libvirt] [PATCH v2 2/3] xenconfig: add support for multiple IP addresses
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
7:02 a.m.
New subject: [libvirt] [PATCH v2 2/3] xenconfig: add support for multiple IP addresses
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;
8:27 p.m.
New subject: [libvirt] [PATCH v2 3/3] tests: add test for multiple IPs for libxl and xenconfig driver
---
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
3:15 p.m.
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(a)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
6:58 a.m.
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
7:37 a.m.
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?
7:55 a.m.
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
8:35 a.m.
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?
2573
days inactive
2577
days old
10 comments
3 participants
participants (3)
-
Jim Fehlig -
John Ferlan -
Marek Marczykowski-Górecki