On 09/21/12 21:46, Laine Stump wrote:
<interface> elements are location inside the <forward> element of a
network. There is only one <forward> element in any network, but it
might have many <interface> elements. This element only contains a
single attribute, "dev", which is the name of a network device
(e.g. "eth0").
Since there is only a single attribute, the modify operation isn't
supported for this "section", only add-first, add-last, and
delete. Also, note that it's not permitted to delete an interface from
the list while any guest is using it. We may later decide this is safe
(because removing it from the list really only excludes it from
consideration in future guest allocations of interfaces, but doesn't
affect any guests currently connected), but for now this limitation
seems prudent (of course when changing the persistent config, this
limitation doesn't apply, because the persistent config doesn't
support the concept of "in used").
s/used/use/
Another limitation - it is also possible for the interfraces in this
s/interfraces/interfaces/
list to be described by PCI address rather than netdev name. However,
I noticed while writing this function that we currently don't support
defining interfaces that way in config - the only method of getting
interfaces specified as <adress type='pci' ..../> instead of
s/adress/address/
<interface dev='xx'/> is to provide a <pf dev='yy'/> element under
forward, and let the entries in the interface list be automatically
populated with the virtual functions (VF) of the physical function
device given in <pg>.
As with the other virNetworkUpdate section backends, support for this
section is completely contained within a single static function, no
other changes were required, and only functions already called from
elsewhere within the same file are used in the new content for this
existing function (i.e., adding this code should not cause a new build
problem on any platform).
---
src/conf/network_conf.c | 96 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 94 insertions(+), 2 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index a2d82d4..4f853e3 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -2620,8 +2620,100 @@ virNetworkDefUpdateForwardInterface(virNetworkDefPtr def,
/* virNetworkUpdateFlags */
unsigned int fflags ATTRIBUTE_UNUSED)
In the function header there are multiple arguments marked as unused,
but the new code uses them. You should unmark "command" and "ctxt" as
unused.
{
- virNetworkDefUpdateNoSupport(def, "forward interface");
- return -1;
+ int ii, ret = -1;
+ virNetworkForwardIfDef iface;
+
+ memset(&iface, 0, sizeof(iface));
+
+ if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "interface") < 0)
+ goto cleanup;
+
+ if (command == VIR_NETWORK_UPDATE_COMMAND_MODIFY) {
+ virReportError(VIR_ERR_NO_SUPPORT, "%s",
+ _("forward interface entries cannot be modified, "
+ "only added or deleted"));
+ goto cleanup;
+ }
+
+ /* parsing this is so simple that it doesn't have its own function */
+ iface.type = VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV;
+ if (!(iface.device.dev = virXMLPropString(ctxt->node, "dev"))) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing dev attribute in <interface> element"));
goto cleanup; missing?
+ }
+
+ /* check if an <interface> with same dev name already exists */
+ for (ii = 0; ii < def->nForwardIfs; ii++) {
+ if (def->forwardIfs[ii].type
+ == VIR_NETWORK_FORWARD_HOSTDEV_DEVICE_NETDEV &&
I'd rather have a >80 cols line than break expressions in half. But this
isn't really relevant to discuss in context of this patch.
+ STREQ(iface.device.dev, def->forwardIfs[ii].device.dev))
+ break;
+ }
+
+ if ((command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) ||
+ (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST)) {
+
+ if (ii < def->nForwardIfs) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("there is an existing interface entry "
+ "in network '%s' that matches "
+ "\"<interface dev='%s'>\""),
+ def->name, iface.device.dev);
+ goto cleanup;
+ }
+
+ /* add to beginning/end of list */
+ if (VIR_REALLOC_N(def->forwardIfs, def->nForwardIfs +1) < 0) {
Add space after +.
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST) {
+ def->forwardIfs[def->nForwardIfs] = iface;
+ } else { /* implied (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST) */
+ memmove(def->forwardIfs + 1, def->forwardIfs,
+ sizeof(*def->forwardIfs) * def->nForwardIfs);
+ def->forwardIfs[0] = iface;
+ }
+ def->nForwardIfs++;
+ memset(&iface, 0, sizeof(iface));
+
+ } else if (command == VIR_NETWORK_UPDATE_COMMAND_DELETE) {
+
+ if (ii == def->nForwardIfs) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("couldn't find an interface entry "
+ "in network '%s' matching <interface dev='%s'>"),
+ def->name, iface.device.dev);
+ goto cleanup;
+ }
+
+ /* fail if the interface is being used */
+ if (def->forwardIfs[ii].connections > 0) {
+ virReportError(VIR_ERR_OPERATION_INVALID,
+ _("unable to delete interface '%s' "
+ "in network '%s'. It is currently being used "
+ " by %d domains."),
+ iface.device.dev, def->name,
+ def->forwardIfs[ii].connections);
+ goto cleanup;
+ }
+
+ /* remove it */
+ virNetworkForwardIfDefClear(&def->forwardIfs[ii]);
+ memmove(def->forwardIfs + ii, def->forwardIfs + ii + 1,
+ sizeof(*def->forwardIfs) * (def->nForwardIfs - ii - 1));
+ def->nForwardIfs--;
+ ignore_value(VIR_REALLOC_N(def->forwardIfs, def->nForwardIfs));
+ } else {
+ virNetworkDefUpdateUnknownCommand(command);
+ goto cleanup;
+ }
+
+ ret = 0;
+cleanup:
+ virNetworkForwardIfDefClear(&iface);
+ return ret;
}
static int
ACK with the nits fixed.
Peter
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list