[libvirt] [PATCH] network: don't allow multiple portgroups with the same name in a network
When defining and creating networks, we have been checking to make sure there is only a single "default" portgroup, but haven't verified that no two portgroups have the same name. We *do* check for multiple definitions when updating the portgroups in an existing network though. This patch adds a check to networkValidate(), which is called when a network is defined or created, to disallow duplicate names. It would actually make sense to do this in the network XML parser (since it's not really "something that might make sense but isn't supported by this driver", but is instead "something that should never be allowed"), but doing that carries the danger of causing errors when rereading the config of existing networks when libvirtd is restarted after an upgrade, and that would result in networks disappearing from libvirt's list. (I'm thinking I should change the error to "XML_ERROR" instead of "UNSUPPORTED", even though that's not the type of error that networkValidate is intended for) This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1115858 --- src/network/bridge_driver.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index c56e8f2..d549ca5 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -1,7 +1,7 @@ /* * bridge_driver.c: core driver methods for managing network * - * Copyright (C) 2006-2014 Red Hat, Inc. + * Copyright (C) 2006-2015 Red Hat, Inc. * Copyright (C) 2006 Daniel P. Berrange * * This library is free software; you can redistribute it and/or @@ -2728,7 +2728,7 @@ static int networkValidate(virNetworkDefPtr def, bool check_active) { - size_t i; + size_t i, j; bool vlanUsed, vlanAllowed, badVlanUse = false; virPortGroupDefPtr defaultPortGroup = NULL; virNetworkIpDefPtr ipdef; @@ -2874,7 +2874,15 @@ networkValidate(virNetworkDefPtr def, } defaultPortGroup = &def->portGroups[i]; } - + for (j = i+1; j < def->nPortGroups; j++) { + if (STREQ(def->portGroups[i].name, def->portGroups[j].name)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("multiple <portgroup> elements with the " + "same name (%s) in network '%s'"), + def->portGroups[i].name, def->name); + return -1; + } + } if (def->portGroups[i].bandwidth && !bandwidthAllowed) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Unsupported <bandwidth> element in network '%s' " -- 2.1.0
On 02/05/2015 12:29 PM, Laine Stump wrote:
When defining and creating networks, we have been checking to make sure there is only a single "default" portgroup, but haven't verified that no two portgroups have the same name. We *do* check for multiple definitions when updating the portgroups in an existing network though.
This patch adds a check to networkValidate(), which is called when a network is defined or created, to disallow duplicate names. It would actually make sense to do this in the network XML parser (since it's not really "something that might make sense but isn't supported by this driver", but is instead "something that should never be allowed"), but doing that carries the danger of causing errors when rereading the config of existing networks when libvirtd is restarted after an upgrade, and that would result in networks disappearing from libvirt's list. (I'm thinking I should change the error to "XML_ERROR" instead of "UNSUPPORTED", even though that's not the type of error that networkValidate is intended for)
This resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1115858 --- src/network/bridge_driver.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-)
+ for (j = i+1; j < def->nPortGroups; j++) {
Wonder why our syntax-checker didn't flag the lack of spaces around '+'? ACK -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake -
Laine Stump