[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 5/9] conf: support abstracted interface info in network XML



On 07/20/2011 12:21 AM, Laine Stump wrote:
The network XML is updated in the following ways:

1) The<forward>  element can now contain a list of forward interfaces:

      <forward ....>
        <interface dev='eth10'/>
        <interface dev='eth11'/>
        <interface dev='eth12'/>
        <interface dev='eth13'/>
      </forward>

    The first of these takes the place of the dev attribute that is
    normally in<forward>  - on when defining a network you can specify
    either one, and on output both will be present. If you specify
    both, they must match.


Note that I now have text cases for both this and the domain XML
changes, and documentation for the domain XML changes. I am still
missing documentation for the network XML, and am working on that
while you're reviewing this code.

And you know I'll bug you for it if it hasn't shown up by next week :)

+        case VIR_NETWORK_FORWARD_PASSTHROUGH:
+            if (def->bridge) {
+                virNetworkReportError(VIR_ERR_XML_ERROR,
+                                      _("bridge name not allowed in %s mode (network '%s'"),
+                                      virNetworkForwardTypeToString(def->forwardType),
+                                      def->name);
+                goto error;
+            }
+            /* fall through to next case */
+        case VIR_NETWORK_FORWARD_BRIDGE:

We'll soon find out whether Coverity understands this comment. Hopefully yes.

+++ b/src/libvirt_private.syms
@@ -748,6 +748,7 @@ virNetworkRemoveInactive;
  virNetworkSaveConfig;
  virNetworkSetBridgeMacAddr;
  virNetworkSetBridgeName;
+virPortGroupFindByName;

  # node_device_conf.h

Simple merge conflict here if you applied my squash changes from patch 4/9.

@@ -959,11 +960,12 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
      if (iptablesAddForwardMasquerade(driver->iptables,
                                       &ipdef->address,
                                       prefix,
-                                     network->def->forwardDev,
+                                     forwardIf,
                                       NULL)<  0) {
          networkReportError(VIR_ERR_SYSTEM_ERROR,
-                           _("failed to add iptables rule to enable masquerading to '%s'"),
-                           network->def->forwardDev ? network->def->forwardDev : NULL);
+                           _("failed to add iptables rule to enable masquerading%s%s"),
+                           forwardIf ? " to " : "",
+                           forwardIf ? forwardIf : "");

Translators don't like this. Not to mention that " to " is not marked for translation, so you'd get a mixed-language result.

Better to spell out two possible sentences.

          goto masqerr3;
      }

@@ -971,11 +973,12 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
      if (iptablesAddForwardMasquerade(driver->iptables,
                                       &ipdef->address,
                                       prefix,
-                                     network->def->forwardDev,
+                                     forwardIf,
                                       "udp")<  0) {
          networkReportError(VIR_ERR_SYSTEM_ERROR,
-                           _("failed to add iptables rule to enable UDP masquerading to '%s'"),
-                           network->def->forwardDev ? network->def->forwardDev : NULL);
+                           _("failed to add iptables rule to enable UDP masquerading%s%s"),
+                           forwardIf ? " to " : "",
+                           forwardIf ? forwardIf : "");

Likewise.

ACK with this squashed in (or something similar, if you don't like my abuse of the fact that printf ignores an unused argument on one of the two strings):

diff --git i/src/network/bridge_driver.c w/src/network/bridge_driver.c
index 4459146..402f447 100644
--- i/src/network/bridge_driver.c
+++ w/src/network/bridge_driver.c
@@ -963,9 +963,10 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
                                      forwardIf,
                                      NULL) < 0) {
         networkReportError(VIR_ERR_SYSTEM_ERROR,
- _("failed to add iptables rule to enable masquerading%s%s"),
-                           forwardIf ? " to " : "",
-                           forwardIf ? forwardIf : "");
+                           forwardIf ?
+ _("failed to add iptables rule to enable masquerading to %s") : + _("failed to add iptables rule to enable masquerading"),
+                           forwardIf);
         goto masqerr3;
     }

@@ -976,9 +977,10 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
                                      forwardIf,
                                      "udp") < 0) {
         networkReportError(VIR_ERR_SYSTEM_ERROR,
- _("failed to add iptables rule to enable UDP masquerading%s%s"),
-                           forwardIf ? " to " : "",
-                           forwardIf ? forwardIf : "");
+                           forwardIf ?
+ _("failed to add iptables rule to enable UDP masquerading to %s") : + _("failed to add iptables rule to enable UDP masquerading"),
+                           forwardIf);
         goto masqerr4;
     }

@@ -989,9 +991,10 @@ networkAddMasqueradingIptablesRules(struct network_driver *driver,
                                      forwardIf,
                                      "tcp") < 0) {
         networkReportError(VIR_ERR_SYSTEM_ERROR,
- _("failed to add iptables rule to enable TCP masquerading%s%s"),
-                           forwardIf ? " to " : "",
-                           forwardIf ? forwardIf : "");
+                           forwardIf ?
+ _("failed to add iptables rule to enable TCP masquerading to %s") : + _("failed to add iptables rule to enable TCP masquerading"),
+                           forwardIf);
         goto masqerr5;
     }



--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]