[libvirt] [PATCH v3 0/5] Add support for graphics to get a IPv6 address
Update to Luyao's series - figured I'd repost just to make it easier rather than continued replies... v2 here: http://www.redhat.com/archives/libvir-list/2015-February/msg01228.html Changes since v2: * Split patch 1 into two parts - one to create the IPAddress shim that just calls the existing (and now static) IPv4 code. * Adjust remainder of patches according to my review comments (hopefully and thus why I'm reposting) John Ferlan (1): util: Replace virNetDevGetIPv4Address with virNetDevGetIPAddress Luyao Huang (4): util: Update virNetDevGetIPAddress to add IPv6 boolean conf: Add 'family' attribute to <graphics> 'listen' element network: Allow networkGetNetworkAddress to return IPv6 address qemu: Remove unnecessary virReportError on networkGetNetworkAddress return docs/formatdomain.html.in | 14 ++- docs/schemas/domaincommon.rng | 8 ++ src/conf/domain_conf.c | 21 ++++ src/conf/domain_conf.h | 10 ++ src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 17 +-- src/network/bridge_driver.h | 6 +- src/qemu/qemu_command.c | 22 ++-- src/util/virnetdev.c | 121 +++++++++++++++++++-- src/util/virnetdev.h | 7 +- .../qemuxml2argv-graphics-listen-network-ipv4.xml | 35 ++++++ .../qemuxml2argv-graphics-listen-network-ipv6.xml | 35 ++++++ tests/qemuxml2xmltest.c | 2 + 13 files changed, 263 insertions(+), 37 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml -- 2.1.0
Create a shim virNetDevGetIPAddress API to just call the virNetDevGetIPv4Address with the future being to add static API's to get the IPv6 or IPv4 address Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 2 +- src/network/bridge_driver.c | 2 +- src/util/virnetdev.c | 26 ++++++++++++++++++++++---- src/util/virnetdev.h | 2 +- 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e0d5459..fcaf729 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1676,7 +1676,7 @@ virNetDevDelMulti; virNetDevExists; virNetDevGetFeatures; virNetDevGetIndex; -virNetDevGetIPv4Address; +virNetDevGetIPAddress; virNetDevGetLinkInfo; virNetDevGetMAC; virNetDevGetMTU; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9637371..281612a 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4569,7 +4569,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) } if (dev_name) { - if (virNetDevGetIPv4Address(dev_name, &addr) < 0) + if (virNetDevGetIPAddress(dev_name, &addr) < 0) goto cleanup; addrptr = &addr; } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index 971db43..fd480e9 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -1395,8 +1395,9 @@ int virNetDevClearIPAddress(const char *ifname, * Returns 0 on success, -errno on failure. */ #if defined(SIOCGIFADDR) && defined(HAVE_STRUCT_IFREQ) -int virNetDevGetIPv4Address(const char *ifname, - virSocketAddrPtr addr) +static int +virNetDevGetIPv4Address(const char *ifname, + virSocketAddrPtr addr) { int fd = -1; int ret = -1; @@ -1426,8 +1427,9 @@ int virNetDevGetIPv4Address(const char *ifname, #else /* ! SIOCGIFADDR */ -int virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, - virSocketAddrPtr addr ATTRIBUTE_UNUSED) +static int +virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, + virSocketAddrPtr addr ATTRIBUTE_UNUSED) { virReportSystemError(ENOSYS, "%s", _("Unable to get IPv4 address on this platform")); @@ -1436,6 +1438,22 @@ int virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, #endif /* ! SIOCGIFADDR */ +/** + * virNetDevGetIPAddress: + * @ifname: name of the interface whose IP address we want + * @addr: filled with the IPv4 address + * + * This function gets the IPv4 address for the interface @ifname + * and stores it in @addr + * + * Returns 0 on success, -errno on failure. + */ +int +virNetDevGetIPAddress(const char *ifname, + virSocketAddrPtr addr) +{ + return virNetDevGetIPv4Address(ifname, addr); +} /** * virNetDevValidateConfig: diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index 856127b..fb0fd46 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -103,7 +103,7 @@ int virNetDevClearIPAddress(const char *ifname, virSocketAddr *addr, unsigned int prefix) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -int virNetDevGetIPv4Address(const char *ifname, virSocketAddrPtr addr) +int virNetDevGetIPAddress(const char *ifname, virSocketAddrPtr addr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -- 2.1.0
From: Luyao Huang <lhuang@redhat.com> Add static virNetDevGetIfaddrsAddress to attempt to get interface IPv6 or IPv4 address. If that fails, if an IPv4 address is desired, then fall back to try the virNetDevGetIPv4Address to get the IP address. Signed-off-by: Luyao Huang <lhuang@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/network/bridge_driver.c | 2 +- src/util/virnetdev.c | 97 +++++++++++++++++++++++++++++++++++++++++---- src/util/virnetdev.h | 7 ++-- 3 files changed, 95 insertions(+), 11 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 281612a..5186be8 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4569,7 +4569,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) } if (dev_name) { - if (virNetDevGetIPAddress(dev_name, &addr) < 0) + if (virNetDevGetIPAddress(dev_name, false, &addr) < 0) goto cleanup; addrptr = &addr; } diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index fd480e9..50f6f0f 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -33,6 +33,10 @@ #include "virstring.h" #include "virutil.h" +#if defined(HAVE_GETIFADDRS) +# include <ifaddrs.h> +#endif + #include <sys/ioctl.h> #include <net/if.h> #include <fcntl.h> @@ -1403,9 +1407,6 @@ virNetDevGetIPv4Address(const char *ifname, int ret = -1; struct ifreq ifr; - memset(addr, 0, sizeof(*addr)); - addr->data.stor.ss_family = AF_UNSPEC; - if ((fd = virNetDevSetupControl(ifname, &ifr)) < 0) return -1; @@ -1431,16 +1432,79 @@ static int virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, virSocketAddrPtr addr ATTRIBUTE_UNUSED) { - virReportSystemError(ENOSYS, "%s", - _("Unable to get IPv4 address on this platform")); - return -1; + return -2; } #endif /* ! SIOCGIFADDR */ /** + * virNetDevGetIfaddrsAddress: + * @ifname: name of the interface whose IP address we want + * @want_ipv6: get IPv4 or IPv6 address + * @addr: filled with the IP address + * + * This function gets the IP address for the interface @ifname + * and stores it in @addr + * + * Returns 0 on success, -1 on failure, -2 on unsupported. + */ +#if defined(HAVE_GETIFADDRS) +static int +virNetDevGetIfaddrsAddress(const char *ifname, + bool want_ipv6, + virSocketAddrPtr addr) +{ + struct ifaddrs *ifap, *ifa; + int ret = -1; + + if (getifaddrs(&ifap) < 0) { + virReportSystemError(errno, + _("Could not get interface list for '%s'"), + ifname); + return -1; + } + + for (ifa = ifap; ifa; ifa = ifa->ifa_next) { + if (STRNEQ_NULLABLE(ifa->ifa_name, ifname)) + continue; + if (ifa->ifa_addr->sa_family == (want_ipv6 ? AF_INET6 : AF_INET)) { + if (want_ipv6) { + addr->len = sizeof(addr->data.inet6); + memcpy(&addr->data.inet6, ifa->ifa_addr, addr->len); + } else { + addr->len = sizeof(addr->data.inet4); + memcpy(&addr->data.inet4, ifa->ifa_addr, addr->len); + } + addr->data.stor.ss_family = ifa->ifa_addr->sa_family; + ret = 0; + goto cleanup; + } + } + + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to find %s address for interface '%s'"), + want_ipv6 ? "IPv6" : "IPv4", ifname); + cleanup: + freeifaddrs(ifap); + return ret; +} + +#else /* ! defined(HAVE_GETIFADDRS) */ + +static int +virNetDevGetIfaddrsAddress(const char *ifname ATTRIBUTE_UNUSED, + bool want_ipv6 ATTRIBUTE_UNUSED, + virSocketAddrPtr addr ATTRIBUTE_UNUSED) +{ + return -2; +} + +#endif + +/** * virNetDevGetIPAddress: * @ifname: name of the interface whose IP address we want + * @want_ipv6: get IPv4 or IPv6 address * @addr: filled with the IPv4 address * * This function gets the IPv4 address for the interface @ifname @@ -1450,9 +1514,28 @@ virNetDevGetIPv4Address(const char *ifname ATTRIBUTE_UNUSED, */ int virNetDevGetIPAddress(const char *ifname, + bool want_ipv6, virSocketAddrPtr addr) { - return virNetDevGetIPv4Address(ifname, addr); + int ret; + + memset(addr, 0, sizeof(*addr)); + addr->data.stor.ss_family = AF_UNSPEC; + + ret = virNetDevGetIfaddrsAddress(ifname, want_ipv6, addr); + if (ret != -2) + return ret; + + if (!want_ipv6) + ret = virNetDevGetIPv4Address(ifname, addr); + + if (ret != -2) + return ret; + + virReportSystemError(ENOSYS, + _("Unable to get %s address on this platform"), + want_ipv6 ? "IPv6" : "IPv4"); + return -1; } /** diff --git a/src/util/virnetdev.h b/src/util/virnetdev.h index fb0fd46..620db50 100644 --- a/src/util/virnetdev.h +++ b/src/util/virnetdev.h @@ -103,9 +103,10 @@ int virNetDevClearIPAddress(const char *ifname, virSocketAddr *addr, unsigned int prefix) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; -int virNetDevGetIPAddress(const char *ifname, virSocketAddrPtr addr) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK; - +int virNetDevGetIPAddress(const char *ifname, + bool want_ipv6, + virSocketAddrPtr addr) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK; int virNetDevSetMAC(const char *ifname, const virMacAddr *macaddr) -- 2.1.0
From: Luyao Huang <lhuang@redhat.com> If an interface or network has both ipv6 and ipv4 addresses which can be used, we do not know which to use as a listen address. This patch introduces the 'family' attribute to allow the XML to determine whether the desire is to use IPv6 instead of IPv4 as the listen family to use. The default will remain IPv4. The graphics XML may look like this after this commit: <graphics type='spice' port='5900' autoport='yes'> <listen type='network' address='192.168.0.1' network='vepa-net' family='ipv4'/> </graphics> The address used to listen will be the first address of the family type found for the network. Added two new tests with the new attribute in place. Signed-off-by: Luyao Huang <lhuang@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 14 ++++++++- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 21 +++++++++++++ src/conf/domain_conf.h | 10 +++++++ .../qemuxml2argv-graphics-listen-network-ipv4.xml | 35 ++++++++++++++++++++++ .../qemuxml2argv-graphics-listen-network-ipv6.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 335763f..262c576 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4553,7 +4553,7 @@ qemu-kvm -net nic,model=? /dev/null <graphics type='rdp' autoport='yes' multiUser='yes' /> <graphics type='desktop' fullscreen='yes'/> <graphics type='spice'> - <listen type='network' network='rednet'/> + <listen type='network' network='rednet' family='ipv4'/> </graphics> </devices> ...</pre> @@ -4793,6 +4793,18 @@ qemu-kvm -net nic,model=? /dev/null the first forward dev will be used. </dd> </dl> + <dl> + <dt><code>family</code></dt> + <dd>if <code>type='network'</code>, the <code>family</code> + attribute may contain the IP family. The <code>family</code> + can be set to either <code>ipv4</code> or <code>ipv6</code>. + This advises the graphics device which IP address family + to use as listen address for the network. The listen address + used will be the first found address of the <code>family</code> + type defined for the host. + <span class="since">Since 1.2.14</span> + </dd> + </dl> <h4><a name="elementsVideo">Video devices</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 56ea6a4..87582cc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2926,6 +2926,14 @@ <ref name="addrIPorName"/> </attribute> </optional> + <optional> + <attribute name="family"> + <choice> + <value>ipv4</value> + <value>ipv6</value> + </choice> + </attribute> + </optional> </group> </choice> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc8616b..70a3525 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -522,6 +522,12 @@ VIR_ENUM_IMPL(virDomainGraphicsListen, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST, "address", "network") +VIR_ENUM_IMPL(virDomainGraphicsListenFamily, + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST, + "none", + "ipv4", + "ipv6") + VIR_ENUM_IMPL(virDomainGraphicsAuthConnected, VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST, "default", @@ -9547,6 +9553,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, char *address = virXMLPropString(node, "address"); char *network = virXMLPropString(node, "network"); char *fromConfig = virXMLPropString(node, "fromConfig"); + char *family = virXMLPropString(node, "family"); int tmp; if (!type) { @@ -9584,6 +9591,15 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, network = NULL; } + if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK && family) { + if ((def->family = + virDomainGraphicsListenFamilyTypeFromString(family)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown graphics listen IP family '%s'"), family); + goto error; + } + } + if (fromConfig && flags & VIR_DOMAIN_DEF_PARSE_STATUS) { if (virStrToLong_i(fromConfig, NULL, 10, &tmp) < 0) { @@ -9603,6 +9619,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, VIR_FREE(address); VIR_FREE(network); VIR_FREE(fromConfig); + VIR_FREE(family); return ret; } @@ -19203,6 +19220,10 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, if (def->network && (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) { virBufferEscapeString(buf, " network='%s'", def->network); + + if (def->family != VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_NONE) + virBufferAsprintf(buf, " family='%s'", + virDomainGraphicsListenFamilyTypeToString(def->family)); } if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 36bb418..8a6df7c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1442,6 +1442,14 @@ typedef enum { } virDomainGraphicsListenType; typedef enum { + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_NONE = 0, + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV4, + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV6, + + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST +} virDomainGraphicsListenFamily; + +typedef enum { VIR_DOMAIN_HUB_TYPE_USB, VIR_DOMAIN_HUB_TYPE_LAST @@ -1454,6 +1462,7 @@ struct _virDomainGraphicsListenDef { char *address; char *network; bool fromConfig; /* true if the @address is config file originated */ + int family; /*enum virDomainGraphicsListenFamily*/ }; struct _virDomainGraphicsDef { @@ -2858,6 +2867,7 @@ VIR_ENUM_DECL(virDomainInput) VIR_ENUM_DECL(virDomainInputBus) VIR_ENUM_DECL(virDomainGraphics) VIR_ENUM_DECL(virDomainGraphicsListen) +VIR_ENUM_DECL(virDomainGraphicsListenFamily) VIR_ENUM_DECL(virDomainGraphicsAuthConnected) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelName) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelMode) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml new file mode 100644 index 0000000..3b5c2de --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='5903' autoport='no'> + <listen type='network' network='Bobsnetwork' family='ipv4'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml new file mode 100644 index 0000000..6cce7a8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='5903' autoport='no'> + <listen type='network' network='Bobsnetwork' family='ipv6'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 8e12e84..e49510c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -247,6 +247,8 @@ mymain(void) DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE); DO_TEST_FULL("disk-active-commit", false, WHEN_ACTIVE); DO_TEST("graphics-listen-network"); + DO_TEST("graphics-listen-network-ipv4"); + DO_TEST("graphics-listen-network-ipv6"); DO_TEST("graphics-vnc"); DO_TEST("graphics-vnc-websocket"); DO_TEST("graphics-vnc-sasl"); -- 2.1.0
On Mon, Mar 09, 2015 at 08:04:59PM -0400, John Ferlan wrote:
From: Luyao Huang <lhuang@redhat.com>
If an interface or network has both ipv6 and ipv4 addresses which can be used, we do not know which to use as a listen address. This patch introduces the 'family' attribute to allow the XML to determine whether the desire is to use IPv6 instead of IPv4 as the listen family to use. The default will remain IPv4.
As Laine mentioned in his reply to v1: https://www.redhat.com/archives/libvir-list/2015-February/msg01080.html This is intended to be run only on networks with one address. With more addresses, you cannot control which one to use. If you want to listen on IPv6, don't configure an IPv4 address on the network and vice versa. This attribute does not seem that useful to me. The original bug https://bugzilla.redhat.com/show_bug.cgi?id=1192318 complained about 'no usable address' I think the bug here is not treating an ipv6 address as an IP address, not that we cannot choose the attribute by family. Jan
The graphics XML may look like this after this commit:
<graphics type='spice' port='5900' autoport='yes'> <listen type='network' address='192.168.0.1' network='vepa-net' family='ipv4'/> </graphics>
The address used to listen will be the first address of the family type found for the network.
Added two new tests with the new attribute in place.
Signed-off-by: Luyao Huang <lhuang@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 14 ++++++++- docs/schemas/domaincommon.rng | 8 +++++ src/conf/domain_conf.c | 21 +++++++++++++ src/conf/domain_conf.h | 10 +++++++ .../qemuxml2argv-graphics-listen-network-ipv4.xml | 35 ++++++++++++++++++++++ .../qemuxml2argv-graphics-listen-network-ipv6.xml | 35 ++++++++++++++++++++++ tests/qemuxml2xmltest.c | 2 ++ 7 files changed, 124 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 335763f..262c576 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -4553,7 +4553,7 @@ qemu-kvm -net nic,model=? /dev/null <graphics type='rdp' autoport='yes' multiUser='yes' /> <graphics type='desktop' fullscreen='yes'/> <graphics type='spice'> - <listen type='network' network='rednet'/> + <listen type='network' network='rednet' family='ipv4'/> </graphics> </devices> ...</pre> @@ -4793,6 +4793,18 @@ qemu-kvm -net nic,model=? /dev/null the first forward dev will be used. </dd> </dl> + <dl> + <dt><code>family</code></dt> + <dd>if <code>type='network'</code>, the <code>family</code> + attribute may contain the IP family. The <code>family</code> + can be set to either <code>ipv4</code> or <code>ipv6</code>. + This advises the graphics device which IP address family + to use as listen address for the network. The listen address + used will be the first found address of the <code>family</code> + type defined for the host. + <span class="since">Since 1.2.14</span> + </dd> + </dl>
<h4><a name="elementsVideo">Video devices</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 56ea6a4..87582cc 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -2926,6 +2926,14 @@ <ref name="addrIPorName"/> </attribute> </optional> + <optional> + <attribute name="family"> + <choice> + <value>ipv4</value> + <value>ipv6</value> + </choice> + </attribute> + </optional> </group> </choice> </element> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index cc8616b..70a3525 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -522,6 +522,12 @@ VIR_ENUM_IMPL(virDomainGraphicsListen, VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_LAST, "address", "network")
+VIR_ENUM_IMPL(virDomainGraphicsListenFamily, + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST, + "none", + "ipv4", + "ipv6") + VIR_ENUM_IMPL(virDomainGraphicsAuthConnected, VIR_DOMAIN_GRAPHICS_AUTH_CONNECTED_LAST, "default", @@ -9547,6 +9553,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, char *address = virXMLPropString(node, "address"); char *network = virXMLPropString(node, "network"); char *fromConfig = virXMLPropString(node, "fromConfig"); + char *family = virXMLPropString(node, "family"); int tmp;
if (!type) { @@ -9584,6 +9591,15 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, network = NULL; }
+ if (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK && family) { + if ((def->family = + virDomainGraphicsListenFamilyTypeFromString(family)) < 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("unknown graphics listen IP family '%s'"), family); + goto error; + } + } + if (fromConfig && flags & VIR_DOMAIN_DEF_PARSE_STATUS) { if (virStrToLong_i(fromConfig, NULL, 10, &tmp) < 0) { @@ -9603,6 +9619,7 @@ virDomainGraphicsListenDefParseXML(virDomainGraphicsListenDefPtr def, VIR_FREE(address); VIR_FREE(network); VIR_FREE(fromConfig); + VIR_FREE(family); return ret; }
@@ -19203,6 +19220,10 @@ virDomainGraphicsListenDefFormat(virBufferPtr buf, if (def->network && (def->type == VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_NETWORK)) { virBufferEscapeString(buf, " network='%s'", def->network); + + if (def->family != VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_NONE) + virBufferAsprintf(buf, " family='%s'", + virDomainGraphicsListenFamilyTypeToString(def->family)); }
if (flags & VIR_DOMAIN_DEF_FORMAT_STATUS) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 36bb418..8a6df7c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1442,6 +1442,14 @@ typedef enum { } virDomainGraphicsListenType;
typedef enum { + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_NONE = 0, + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV4, + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV6, + + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_LAST +} virDomainGraphicsListenFamily; + +typedef enum { VIR_DOMAIN_HUB_TYPE_USB,
VIR_DOMAIN_HUB_TYPE_LAST @@ -1454,6 +1462,7 @@ struct _virDomainGraphicsListenDef { char *address; char *network; bool fromConfig; /* true if the @address is config file originated */ + int family; /*enum virDomainGraphicsListenFamily*/ };
struct _virDomainGraphicsDef { @@ -2858,6 +2867,7 @@ VIR_ENUM_DECL(virDomainInput) VIR_ENUM_DECL(virDomainInputBus) VIR_ENUM_DECL(virDomainGraphics) VIR_ENUM_DECL(virDomainGraphicsListen) +VIR_ENUM_DECL(virDomainGraphicsListenFamily) VIR_ENUM_DECL(virDomainGraphicsAuthConnected) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelName) VIR_ENUM_DECL(virDomainGraphicsSpiceChannelMode) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml new file mode 100644 index 0000000..3b5c2de --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv4.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='5903' autoport='no'> + <listen type='network' network='Bobsnetwork' family='ipv4'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml new file mode 100644 index 0000000..6cce7a8 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-graphics-listen-network-ipv6.xml @@ -0,0 +1,35 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='i686' machine='pc'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='ide'/> + <address type='drive' controller='0' bus='0' target='0' unit='0'/> + </disk> + <controller type='usb' index='0'/> + <controller type='ide' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <graphics type='vnc' port='5903' autoport='no'> + <listen type='network' network='Bobsnetwork' family='ipv6'/> + </graphics> + <video> + <model type='cirrus' vram='16384' heads='1'/> + </video> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 8e12e84..e49510c 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -247,6 +247,8 @@ mymain(void) DO_TEST_FULL("disk-mirror", true, WHEN_INACTIVE); DO_TEST_FULL("disk-active-commit", false, WHEN_ACTIVE); DO_TEST("graphics-listen-network"); + DO_TEST("graphics-listen-network-ipv4"); + DO_TEST("graphics-listen-network-ipv6"); DO_TEST("graphics-vnc"); DO_TEST("graphics-vnc-websocket"); DO_TEST("graphics-vnc-sasl"); -- 2.1.0
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 03/10/2015 12:58 PM, Ján Tomko wrote:
On Mon, Mar 09, 2015 at 08:04:59PM -0400, John Ferlan wrote:
From: Luyao Huang <lhuang@redhat.com>
If an interface or network has both ipv6 and ipv4 addresses which can be used, we do not know which to use as a listen address. This patch introduces the 'family' attribute to allow the XML to determine whether the desire is to use IPv6 instead of IPv4 as the listen family to use. The default will remain IPv4.
As Laine mentioned in his reply to v1: https://www.redhat.com/archives/libvir-list/2015-February/msg01080.html This is intended to be run only on networks with one address. With more addresses, you cannot control which one to use.
It's been a while since I've had to "think" whether getifaddrs() would return the same IPv4 address in the "first" entry as would be return from the ioctl(SIOCGIFADDR)... IIRC, IPv4 addresses can be aliased to the same device, but how getifaddrs handles returning addresses I just don't have recent exposure to..
If you want to listen on IPv6, don't configure an IPv4 address on the network and vice versa. This attribute does not seem that useful to me.
The original bug https://bugzilla.redhat.com/show_bug.cgi?id=1192318 complained about 'no usable address'
I think the bug here is not treating an ipv6 address as an IP address, not that we cannot choose the attribute by family.
Jan
hmmm... true we're really just looking to get "an" address and shouldn't care what style it is. However, what if someone has both configured and wants to force usage of one over the other? Maybe they've separate their IPv4 and IPv6 addresses to connect to different places. Perhaps forcing certain protocols over IPv6 rather than IPv4? Since both can be defined in one <network> object - it's possible - how or why it would be done I haven't given too much thought to. John
The graphics XML may look like this after this commit:
<graphics type='spice' port='5900' autoport='yes'> <listen type='network' address='192.168.0.1' network='vepa-net' family='ipv4'/> </graphics>
On Tue, Mar 10, 2015 at 07:43:16PM -0400, John Ferlan wrote:
On 03/10/2015 12:58 PM, Ján Tomko wrote:
On Mon, Mar 09, 2015 at 08:04:59PM -0400, John Ferlan wrote:
From: Luyao Huang <lhuang@redhat.com>
If an interface or network has both ipv6 and ipv4 addresses which can be used, we do not know which to use as a listen address. This patch introduces the 'family' attribute to allow the XML to determine whether the desire is to use IPv6 instead of IPv4 as the listen family to use. The default will remain IPv4.
As Laine mentioned in his reply to v1: https://www.redhat.com/archives/libvir-list/2015-February/msg01080.html This is intended to be run only on networks with one address. With more addresses, you cannot control which one to use.
It's been a while since I've had to "think" whether getifaddrs() would return the same IPv4 address in the "first" entry as would be return from the ioctl(SIOCGIFADDR)... IIRC, IPv4 addresses can be aliased to the same device, but how getifaddrs handles returning addresses I just don't have recent exposure to..
If you want to listen on IPv6, don't configure an IPv4 address on the network and vice versa. This attribute does not seem that useful to me.
The original bug https://bugzilla.redhat.com/show_bug.cgi?id=1192318 complained about 'no usable address'
I think the bug here is not treating an ipv6 address as an IP address, not that we cannot choose the attribute by family.
Jan
hmmm... true we're really just looking to get "an" address and shouldn't care what style it is. However, what if someone has both configured and wants to force usage of one over the other? Maybe they've separate their IPv4 and IPv6 addresses to connect to different places. Perhaps forcing certain protocols over IPv6 rather than IPv4? Since both can be defined in one <network> object - it's possible - how or why it would be done I haven't given too much thought to.
They can configure another separate network, or create a hook that fills in the proper <listen type='address'>. If we allow filtering by family, should we also introduce filtering by a netmask? Jan
From: Luyao Huang <lhuang@redhat.com> Export the required helpers and rework networkGetNetworkAddress to make it can get IPv6 address. Signed-off-by: Luyao Huang <lhuang@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/network/bridge_driver.c | 17 +++++++++-------- src/network/bridge_driver.h | 6 ++++-- src/qemu/qemu_command.c | 8 ++++++-- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 5186be8..c00c656 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4493,8 +4493,9 @@ networkReleaseActualDevice(virDomainDefPtr dom, * networkGetNetworkAddress: * @netname: the name of a network * @netaddr: string representation of IP address for that network. + * @want_ipv6: if true will get IPv6 address, false for IPv4. * - * Attempt to return an IP (v4) address associated with the named + * Attempt to return an IP (v4 or v6) address associated with the named * network. If a libvirt virtual network, that will be provided in the * configuration. For host bridge and direct (macvtap) networks, we * must do an ioctl to learn the address. @@ -4509,12 +4510,12 @@ networkReleaseActualDevice(virDomainDefPtr dom, * completely unsupported. */ int -networkGetNetworkAddress(const char *netname, char **netaddr) +networkGetNetworkAddress(const char *netname, char **netaddr, bool want_ipv6) { int ret = -1; virNetworkObjPtr network; virNetworkDefPtr netdef; - virNetworkIpDefPtr ipdef; + virNetworkIpDefPtr ipdef = NULL; virSocketAddr addr; virSocketAddrPtr addrptr = NULL; char *dev_name = NULL; @@ -4535,12 +4536,12 @@ networkGetNetworkAddress(const char *netname, char **netaddr) case VIR_NETWORK_FORWARD_NONE: case VIR_NETWORK_FORWARD_NAT: case VIR_NETWORK_FORWARD_ROUTE: - /* if there's an ipv4def, get it's address */ - ipdef = virNetworkDefGetIpByIndex(netdef, AF_INET, 0); + /* if there's an ipdef, get it's IPv4 or IPv6 address */ + ipdef = virNetworkDefGetIpByIndex(netdef, want_ipv6 ? AF_INET6 : AF_INET, 0); if (!ipdef) { virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have an IPv4 address"), - netdef->name); + _("network '%s' doesn't have an '%s' address"), + netdef->name, want_ipv6 ? "IPv6" : "IPv4"); goto cleanup; } addrptr = &ipdef->address; @@ -4569,7 +4570,7 @@ networkGetNetworkAddress(const char *netname, char **netaddr) } if (dev_name) { - if (virNetDevGetIPAddress(dev_name, false, &addr) < 0) + if (virNetDevGetIPAddress(dev_name, want_ipv6, &addr) < 0) goto cleanup; addrptr = &addr; } diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 2f801ee..465ab18 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -44,7 +44,9 @@ int networkReleaseActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); -int networkGetNetworkAddress(const char *netname, char **netaddr) +int networkGetNetworkAddress(const char *netname, + char **netaddr, + bool want_ipv6) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int networkDnsmasqConfContents(virNetworkObjPtr network, @@ -57,7 +59,7 @@ int networkDnsmasqConfContents(virNetworkObjPtr network, # define networkAllocateActualDevice(dom, iface) 0 # define networkNotifyActualDevice(dom, iface) (dom=dom, iface=iface, 0) # define networkReleaseActualDevice(dom, iface) (dom=dom, iface=iface, 0) -# define networkGetNetworkAddress(netname, netaddr) (-2) +# define networkGetNetworkAddress(netname, netaddr, want_ipv6) (-2) # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 # endif diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1510797..466c0cf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7381,7 +7381,9 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); if (!listenNetwork) break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); + ret = networkGetNetworkAddress(listenNetwork, &netAddr, + graphics->listens->family == + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV6); if (ret <= -2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("network-based listen not possible, " @@ -7545,7 +7547,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, listenNetwork = virDomainGraphicsListenGetNetwork(graphics, 0); if (!listenNetwork) break; - ret = networkGetNetworkAddress(listenNetwork, &netAddr); + ret = networkGetNetworkAddress(listenNetwork, &netAddr, + graphics->listens->family == + VIR_DOMAIN_GRAPHICS_LISTEN_FAMILY_IPV6); if (ret <= -2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("network-based listen not possible, " -- 2.1.0
From: Luyao Huang <lhuang@redhat.com> Error messages are already set in all code paths returning -1 from networkGetNetworkAddress, so we don't want to overwrite them. Signed-off-by: Luyao Huang <lhuang@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 466c0cf..495ed20 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -7390,12 +7390,9 @@ qemuBuildGraphicsVNCCommandLine(virQEMUDriverConfigPtr cfg, "network driver not present")); goto error; } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); + if (ret < 0) goto error; - } + listenAddr = netAddr; /* store the address we found in the <graphics> element so it will * show up in status. */ @@ -7556,12 +7553,9 @@ qemuBuildGraphicsSPICECommandLine(virQEMUDriverConfigPtr cfg, "network driver not present")); goto error; } - if (ret < 0) { - virReportError(VIR_ERR_XML_ERROR, - _("listen network '%s' had no usable address"), - listenNetwork); + if (ret < 0) goto error; - } + listenAddr = netAddr; /* store the address we found in the <graphics> element so it will * show up in status. */ -- 2.1.0
On Mon, Mar 09, 2015 at 08:05:01PM -0400, John Ferlan wrote:
From: Luyao Huang <lhuang@redhat.com>
Error messages are already set in all code paths returning -1 from networkGetNetworkAddress, so we don't want to overwrite them.
Signed-off-by: Luyao Huang <lhuang@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
ACK, this one does not depend on the rest of the series. Jan
On 03/10/2015 10:58 AM, Ján Tomko wrote:
On Mon, Mar 09, 2015 at 08:05:01PM -0400, John Ferlan wrote:
From: Luyao Huang <lhuang@redhat.com>
Error messages are already set in all code paths returning -1 from networkGetNetworkAddress, so we don't want to overwrite them.
Signed-off-by: Luyao Huang <lhuang@redhat.com> Signed-off-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-)
ACK, this one does not depend on the rest of the series.
Jan
Tks - this one patch is pushed now. I believe this will cause a change to the message in: https://bugzilla.redhat.com/show_bug.cgi?id=1192318 from "error: XML error: listen network 'ipv6' had no usable address" to: "...Unable to get IPv4 address on this platform" But I have no empirical evidence. John
participants (2)
-
John Ferlan -
Ján Tomko