[libvirt] [PATCH 0/3] Three simple bugfixes

Patch 1 was posted several days ago, but may have gotten lost in the stream, so I'm reposting it. (The patches are unrelated other than that they are nominally related to networking, they were all in my queue at the same time, and they're all three very short. I'm just posting them together to make it easier to keep track of them.) Laine Stump (3): netdev: fail when setting up an SRIOV VF if PF is offline interface: allow multiple IPv4 addresses + dhcp on a single interface util: better error message after failure to initialize firewall backend docs/schemas/interface.rng | 28 ++++++++++------------ src/util/virfirewall.c | 10 +++++++- src/util/virnetdev.c | 22 +++++++++++++++++ .../ethernet-dhcp-and-multi-static.xml | 9 +++++++ tests/interfacexml2xmltest.c | 1 + 5 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 tests/interfaceschemadata/ethernet-dhcp-and-multi-static.xml -- 2.1.0

If an SRIOV PF is offline, the kernel won't complain if you set the mac address and vlan tag for a VF via this PF, and it will even let you assign the VF to a guest using PCI device assignment or macvtap passthrough. But in this case (the PF isn't online), the device won't be usable in the guest. Silently setting the PF online would solve the connectivity problem, but as pointed out by Dan Berrange, when an interface is set online with no associated config, the kernel will by default turn on IPv6 autoconf, which could create unexpected security problems for the host. For this reason, this patch instead logs an error and fails the operation. This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=893738 Originally filed against RHEL6, but present in every version of libvirt until today. --- src/util/virnetdev.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c index e14b401..98ce152 100644 --- a/src/util/virnetdev.c +++ b/src/util/virnetdev.c @@ -2258,6 +2258,28 @@ virNetDevReplaceVfConfig(const char *pflinkdev, int vf, char macstr[VIR_MAC_STRING_BUFLEN]; char *fileData = NULL; int ifindex = -1; + bool pfIsOnline; + + /* Assure that PF is online prior to twiddling with the VF. It + * *should* be, but if the PF isn't online the changes made to the + * VF via the PF won't take effect, yet there will be no error + * reported. In the case that it isn't online, fail and report the + * error, since setting an unconfigured interface online + * automatically turns on IPv6 autoconfig, which may not be what + * the admin expects, so we want them to explicitly enable the PF + * in the host system network config. + */ + if (virNetDevGetOnline(pflinkdev, &pfIsOnline) < 0) + goto cleanup; + if (!pfIsOnline) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to configure VF %d of PF '%s' " + "because the PF is not online. Please " + "change host network config to put the " + "PF online."), + vf, pflinkdev); + goto cleanup; + } if (virNetDevGetVfConfig(pflinkdev, vf, &oldmac, &oldvlanid) < 0) goto cleanup; -- 2.1.0

As of netcf-0.2.8, netcf supports configuring multipl IPv4 addresses, as well as simultaneously configuring dhcp and static IPv4 addresses, on a single interface. This patch updates libvirt's interface.rng to allow such configurations. This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1223688 --- docs/schemas/interface.rng | 28 ++++++++++------------ .../ethernet-dhcp-and-multi-static.xml | 9 +++++++ tests/interfacexml2xmltest.c | 1 + 3 files changed, 23 insertions(+), 15 deletions(-) create mode 100644 tests/interfaceschemadata/ethernet-dhcp-and-multi-static.xml diff --git a/docs/schemas/interface.rng b/docs/schemas/interface.rng index 45b40cd..052703c 100644 --- a/docs/schemas/interface.rng +++ b/docs/schemas/interface.rng @@ -323,24 +323,22 @@ <value>ipv4</value> </attribute> <interleave> - <choice> + <optional> <ref name="dhcp-element"/> - <group> - <oneOrMore> - <element name="ip"> - <attribute name="address"><ref name="ipv4Addr"/></attribute> - <optional> - <attribute name="prefix"><ref name="ipv4Prefix"/></attribute> - </optional> - </element> - </oneOrMore> + </optional> + <zeroOrMore> + <element name="ip"> + <attribute name="address"><ref name="ipv4Addr"/></attribute> <optional> - <element name="route"> - <attribute name="gateway"><ref name="ipv4Addr"/></attribute> - </element> + <attribute name="prefix"><ref name="ipv4Prefix"/></attribute> </optional> - </group> - </choice> + </element> + </zeroOrMore> + <optional> + <element name="route"> + <attribute name="gateway"><ref name="ipv4Addr"/></attribute> + </element> + </optional> </interleave> </element> </define> diff --git a/tests/interfaceschemadata/ethernet-dhcp-and-multi-static.xml b/tests/interfaceschemadata/ethernet-dhcp-and-multi-static.xml new file mode 100644 index 0000000..3bef2b6 --- /dev/null +++ b/tests/interfaceschemadata/ethernet-dhcp-and-multi-static.xml @@ -0,0 +1,9 @@ +<interface type='ethernet' name='eth1'> + <start mode='onboot'/> + <protocol family='ipv4'> + <dhcp peerdns='yes'/> + <ip address='192.168.0.5' prefix='24'/> + <ip address='1.2.3.4' prefix='32'/> + <route gateway='192.168.0.1'/> + </protocol> +</interface> diff --git a/tests/interfacexml2xmltest.c b/tests/interfacexml2xmltest.c index 5e11754..65f5167 100644 --- a/tests/interfacexml2xmltest.c +++ b/tests/interfacexml2xmltest.c @@ -75,6 +75,7 @@ mymain(void) ret = -1 DO_TEST("ethernet-dhcp"); + DO_TEST("ethernet-dhcp-and-multi-static"); DO_TEST("ethernet-static"); DO_TEST("ethernet-static-no-prefix"); DO_TEST("bridge"); -- 2.1.0

If the firewalld backend wasn't available and libvirt decides to try setting up a "direct" backend, it checks for the presence of iptables, ip6tables, and ebtables. If they are not found, a message like this is logged: error : virFirewallValidateBackend:193 : direct firewall backend requested, but /usr/sbin/ip6tables is not available: No such file or directory But then at a later time if an attempt is made to use the virFirewall API, failure will be indicated with: error : virFirewallApply:936 : out of memory This patch changes virFirewallApply to first check if a firewall backend hadn't been successfully setup, and logs a slightly more informative message in that case: error : virFirewallApply:940 : internal error: Failed to initialize a valid firewall backend This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1223876 --- src/util/virfirewall.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/util/virfirewall.c b/src/util/virfirewall.c index 2251f97..a972c05 100644 --- a/src/util/virfirewall.c +++ b/src/util/virfirewall.c @@ -1,7 +1,7 @@ /* * virfirewall.c: integration with firewalls * - * Copyright (C) 2013, 2014 Red Hat, Inc. + * Copyright (C) 2013-2015 Red Hat, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -932,6 +932,14 @@ virFirewallApply(virFirewallPtr firewall) virMutexLock(&ruleLock); + if (currentBackend == VIR_FIREWALL_BACKEND_AUTOMATIC) { + /* a specific backend should have been set when the firewall + * object was created. If not, it means none was found. + */ + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Failed to initialize a valid firewall backend")); + goto cleanup; + } if (!firewall || firewall->err == ENOMEM) { virReportOOMError(); goto cleanup; -- 2.1.0

On Thu, May 21, 2015 at 01:48:46PM -0400, Laine Stump wrote:
Patch 1 was posted several days ago, but may have gotten lost in the stream, so I'm reposting it. (The patches are unrelated other than that they are nominally related to networking, they were all in my queue at the same time, and they're all three very short. I'm just posting them together to make it easier to keep track of them.)
Laine Stump (3): netdev: fail when setting up an SRIOV VF if PF is offline interface: allow multiple IPv4 addresses + dhcp on a single interface util: better error message after failure to initialize firewall backend
docs/schemas/interface.rng | 28 ++++++++++------------ src/util/virfirewall.c | 10 +++++++- src/util/virnetdev.c | 22 +++++++++++++++++ .../ethernet-dhcp-and-multi-static.xml | 9 +++++++ tests/interfacexml2xmltest.c | 1 + 5 files changed, 54 insertions(+), 16 deletions(-) create mode 100644 tests/interfaceschemadata/ethernet-dhcp-and-multi-static.xml
ACK Jan

On 05/22/2015 04:16 AM, Ján Tomko wrote:
On Thu, May 21, 2015 at 01:48:46PM -0400, Laine Stump wrote:
Laine Stump (3): netdev: fail when setting up an SRIOV VF if PF is offline interface: allow multiple IPv4 addresses + dhcp on a single interface util: better error message after failure to initialize firewall backend ACK
Pushed. Thanks!
participants (2)
-
Ján Tomko
-
Laine Stump