[libvirt] [PATCH] conf: Don't try formating non-existing addresses
Commit a6f9af8292b6 added checking for address colisions between starting and ending addresses of forwarding addresses, but forgot that there might be no addresses set at all. Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/network_conf.c | 22 +++++++++++++++++++--- ....xml => nat-network-forward-nat-no-address.xml} | 1 - ....xml => nat-network-forward-nat-no-address.xml} | 1 - tests/networkxml2xmltest.c | 1 + 4 files changed, 20 insertions(+), 5 deletions(-) copy tests/networkxml2xmlin/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%) copy tests/networkxml2xmlout/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 374d723788e1..f9d894b12046 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1731,9 +1731,25 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; } - /* verify that start <= end */ - if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) - goto cleanup; + if (addrStart && addrEnd) { + /* verify that start <= end */ + if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) + goto cleanup; + } else { + if (addrStart) { + virReportError(VIR_ERR_XML_ERROR, + _("Only start address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrStart, networkName); + goto cleanup; + } + if (addrEnd) { + virReportError(VIR_ERR_XML_ERROR, + _("Only end address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrEnd, networkName); + } + } /* ports for SNAT and MASQUERADE */ nNatPorts = virXPathNodeSet("./port", ctxt, &natPortNodes); diff --git a/tests/networkxml2xmlin/nat-network-forward-nat-address.xml b/tests/networkxml2xmlin/nat-network-forward-nat-no-address.xml similarity index 93% copy from tests/networkxml2xmlin/nat-network-forward-nat-address.xml copy to tests/networkxml2xmlin/nat-network-forward-nat-no-address.xml index 403d058330ea..97a64526e94f 100644 --- a/tests/networkxml2xmlin/nat-network-forward-nat-address.xml +++ b/tests/networkxml2xmlin/nat-network-forward-nat-no-address.xml @@ -4,7 +4,6 @@ <bridge name="virbr0"/> <forward mode="nat" dev="eth1"> <nat> - <address start='10.20.30.40' end='10.20.30.44'/> <port start='60000' end='65432'/> </nat> </forward> diff --git a/tests/networkxml2xmlout/nat-network-forward-nat-address.xml b/tests/networkxml2xmlout/nat-network-forward-nat-no-address.xml similarity index 93% copy from tests/networkxml2xmlout/nat-network-forward-nat-address.xml copy to tests/networkxml2xmlout/nat-network-forward-nat-no-address.xml index faeba2470661..f19e34deecec 100644 --- a/tests/networkxml2xmlout/nat-network-forward-nat-address.xml +++ b/tests/networkxml2xmlout/nat-network-forward-nat-no-address.xml @@ -3,7 +3,6 @@ <uuid>81ff0d90-c91e-6742-64da-4a736edb9a9b</uuid> <forward dev='eth1' mode='nat'> <nat> - <address start='10.20.30.40' end='10.20.30.44'/> <port start='60000' end='65432'/> </nat> <interface dev='eth1'/> diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 290336edbefd..8d60aa8fc665 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -99,6 +99,7 @@ mymain(void) DO_TEST("nat-network-dns-forward-plain"); DO_TEST("nat-network-dns-forwarders"); DO_TEST("nat-network-forward-nat-address"); + DO_TEST("nat-network-forward-nat-no-address"); DO_TEST("8021Qbh-net"); DO_TEST("direct-net"); DO_TEST("host-bridge-net"); -- 2.5.0
On Tue, Aug 11, 2015 at 03:16:16PM +0200, Martin Kletzander wrote:
Commit a6f9af8292b6 added checking for address colisions between starting and ending addresses of forwarding addresses, but forgot that there might be no addresses set at all.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/network_conf.c | 22 +++++++++++++++++++--- ....xml => nat-network-forward-nat-no-address.xml} | 1 - ....xml => nat-network-forward-nat-no-address.xml} | 1 - tests/networkxml2xmltest.c | 1 + 4 files changed, 20 insertions(+), 5 deletions(-) copy tests/networkxml2xmlin/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%) copy tests/networkxml2xmlout/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 374d723788e1..f9d894b12046 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1731,9 +1731,25 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; }
- /* verify that start <= end */ - if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) - goto cleanup; + if (addrStart && addrEnd) { + /* verify that start <= end */ + if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) + goto cleanup; + } else { + if (addrStart) { + virReportError(VIR_ERR_XML_ERROR, + _("Only start address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrStart, networkName); + goto cleanup; + } + if (addrEnd) { + virReportError(VIR_ERR_XML_ERROR, + _("Only end address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrEnd, networkName);
I see the missing 'goto cleanup;' now, consider this squashed in as it already is in my tree: diff --git c/src/conf/network_conf.c w/src/conf/network_conf.c index f9d894b12046..c1cbd76c6807 100644 --- c/src/conf/network_conf.c +++ w/src/conf/network_conf.c @@ -1748,6 +1748,7 @@ virNetworkForwardNatDefParseXML(const char *networkName, _("Only end address '%s' specified in <nat> in " "<forward> in network '%s'"), addrEnd, networkName); + goto cleanup; } } -- Martin
On 08/11/2015 09:21 AM, Martin Kletzander wrote:
On Tue, Aug 11, 2015 at 03:16:16PM +0200, Martin Kletzander wrote:
Commit a6f9af8292b6 added checking for address colisions between starting and ending addresses of forwarding addresses, but forgot that there might be no addresses set at all.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/network_conf.c | 22 +++++++++++++++++++--- ....xml => nat-network-forward-nat-no-address.xml} | 1 - ....xml => nat-network-forward-nat-no-address.xml} | 1 - tests/networkxml2xmltest.c | 1 + 4 files changed, 20 insertions(+), 5 deletions(-) copy tests/networkxml2xmlin/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%) copy tests/networkxml2xmlout/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 374d723788e1..f9d894b12046 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1731,9 +1731,25 @@ virNetworkForwardNatDefParseXML(const char *networkName, goto cleanup; }
- /* verify that start <= end */ - if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) - goto cleanup; + if (addrStart && addrEnd) { + /* verify that start <= end */ + if (virSocketAddrGetRange(&def->addr.start, &def->addr.end, NULL, 0) < 0) + goto cleanup; + } else { + if (addrStart) { + virReportError(VIR_ERR_XML_ERROR, + _("Only start address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrStart, networkName); + goto cleanup; + } + if (addrEnd) { + virReportError(VIR_ERR_XML_ERROR, + _("Only end address '%s' specified in <nat> in " + "<forward> in network '%s'"), + addrEnd, networkName);
I see the missing 'goto cleanup;' now, consider this squashed in as it already is in my tree:
diff --git c/src/conf/network_conf.c w/src/conf/network_conf.c index f9d894b12046..c1cbd76c6807 100644 --- c/src/conf/network_conf.c +++ w/src/conf/network_conf.c @@ -1748,6 +1748,7 @@ virNetworkForwardNatDefParseXML(const char *networkName, _("Only end address '%s' specified in <nat> in " "<forward> in network '%s'"), addrEnd, networkName); + goto cleanup; } }
ACK to the patch with that squashed in. Thanks for cleaning up after me :-)
On 11.08.2015 15:16, Martin Kletzander wrote:
Commit a6f9af8292b6 added checking for address colisions between starting and ending addresses of forwarding addresses, but forgot that there might be no addresses set at all.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com> --- src/conf/network_conf.c | 22 +++++++++++++++++++--- ....xml => nat-network-forward-nat-no-address.xml} | 1 - ....xml => nat-network-forward-nat-no-address.xml} | 1 - tests/networkxml2xmltest.c | 1 + 4 files changed, 20 insertions(+), 5 deletions(-) copy tests/networkxml2xmlin/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%) copy tests/networkxml2xmlout/{nat-network-forward-nat-address.xml => nat-network-forward-nat-no-address.xml} (93%)
ACK with that diff you've posted applied. Michal
participants (3)
-
Laine Stump -
Martin Kletzander -
Michal Privoznik