[PATCH v2 16/27] network: turn on auto-rollback for the rules added for virtual networks

Laine Stump posted 27 patches 1 year, 9 months ago
There is a newer version of this series
[PATCH v2 16/27] network: turn on auto-rollback for the rules added for virtual networks
Posted by Laine Stump 1 year, 9 months ago
So far this will only affect what happens if there is some failure
while applying the firewall rules; the rollback rules aren't yet
persistent beyond that time. More work is needed to remember the
rollback rules while the network is active, and use those rules to
remove the firewall for the network when it is destroyed.

Signed-off-by: Laine Stump <laine@redhat.com>
---
 src/network/network_iptables.c  | 15 +++------------
 tests/networkxml2firewalltest.c |  9 ++++++++-
 2 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c
index db35a4c5a0..467d43c1e9 100644
--- a/src/network/network_iptables.c
+++ b/src/network/network_iptables.c
@@ -1599,7 +1599,7 @@ iptablesAddFirewallRules(virNetworkDef *def)
     virNetworkIPDef *ipdef;
     g_autoptr(virFirewall) fw = virFirewallNew(VIR_FIREWALL_BACKEND_IPTABLES);
 
-    virFirewallStartTransaction(fw, 0);
+    virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK);
 
     iptablesAddGeneralFirewallRules(fw, def);
 
@@ -1610,17 +1610,8 @@ iptablesAddFirewallRules(virNetworkDef *def)
             return -1;
     }
 
-    virFirewallStartRollback(fw, 0);
-
-    for (i = 0;
-         (ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i));
-         i++) {
-        if (iptablesRemoveIPSpecificFirewallRules(fw, def, ipdef) < 0)
-            return -1;
-    }
-    iptablesRemoveGeneralFirewallRules(fw, def);
-
-    virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
+    virFirewallStartTransaction(fw, (VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS |
+                                     VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK));
     iptablesAddChecksumFirewallRules(fw, def);
 
     return virFirewallApply(fw);
diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
index 3a9f409e2a..e61787daec 100644
--- a/tests/networkxml2firewalltest.c
+++ b/tests/networkxml2firewalltest.c
@@ -79,7 +79,14 @@ testCommandDryRun(const char *const*args G_GNUC_UNUSED,
                   void *opaque G_GNUC_UNUSED)
 {
     *status = 0;
-    *output = g_strdup("");
+    /* if arg[1] is -ae then this is an nft command,
+     * and the caller requested to get the handle
+     * of the newly added object in stdout
+     */
+    if (STREQ_NULLABLE(args[1], "-ae"))
+        *output = g_strdup("# handle 5309");
+    else
+        *output = g_strdup("");
     *error = g_strdup("");
 }
 
-- 
2.44.0
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2 16/27] network: turn on auto-rollback for the rules added for virtual networks
Posted by Daniel P. Berrangé 1 year, 9 months ago
On Sun, Apr 21, 2024 at 10:53:24PM -0400, Laine Stump wrote:
> So far this will only affect what happens if there is some failure
> while applying the firewall rules; the rollback rules aren't yet
> persistent beyond that time. More work is needed to remember the
> rollback rules while the network is active, and use those rules to
> remove the firewall for the network when it is destroyed.
> 
> Signed-off-by: Laine Stump <laine@redhat.com>
> ---
>  src/network/network_iptables.c  | 15 +++------------
>  tests/networkxml2firewalltest.c |  9 ++++++++-
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/src/network/network_iptables.c b/src/network/network_iptables.c
> index db35a4c5a0..467d43c1e9 100644
> --- a/src/network/network_iptables.c
> +++ b/src/network/network_iptables.c
> @@ -1599,7 +1599,7 @@ iptablesAddFirewallRules(virNetworkDef *def)
>      virNetworkIPDef *ipdef;
>      g_autoptr(virFirewall) fw = virFirewallNew(VIR_FIREWALL_BACKEND_IPTABLES);
>  
> -    virFirewallStartTransaction(fw, 0);
> +    virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK);
>  
>      iptablesAddGeneralFirewallRules(fw, def);
>  
> @@ -1610,17 +1610,8 @@ iptablesAddFirewallRules(virNetworkDef *def)
>              return -1;
>      }
>  
> -    virFirewallStartRollback(fw, 0);
> -
> -    for (i = 0;
> -         (ipdef = virNetworkDefGetIPByIndex(def, AF_UNSPEC, i));
> -         i++) {
> -        if (iptablesRemoveIPSpecificFirewallRules(fw, def, ipdef) < 0)
> -            return -1;
> -    }
> -    iptablesRemoveGeneralFirewallRules(fw, def);
> -
> -    virFirewallStartTransaction(fw, VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS);
> +    virFirewallStartTransaction(fw, (VIR_FIREWALL_TRANSACTION_IGNORE_ERRORS |
> +                                     VIR_FIREWALL_TRANSACTION_AUTO_ROLLBACK));
>      iptablesAddChecksumFirewallRules(fw, def);
>  
>      return virFirewallApply(fw);

To this point

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


> diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
> index 3a9f409e2a..e61787daec 100644
> --- a/tests/networkxml2firewalltest.c
> +++ b/tests/networkxml2firewalltest.c
> @@ -79,7 +79,14 @@ testCommandDryRun(const char *const*args G_GNUC_UNUSED,
>                    void *opaque G_GNUC_UNUSED)
>  {
>      *status = 0;
> -    *output = g_strdup("");
> +    /* if arg[1] is -ae then this is an nft command,
> +     * and the caller requested to get the handle
> +     * of the newly added object in stdout
> +     */
> +    if (STREQ_NULLABLE(args[1], "-ae"))
> +        *output = g_strdup("# handle 5309");
> +    else
> +        *output = g_strdup("");

Belongs in the later nft tests patch


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org
Re: [PATCH v2 16/27] network: turn on auto-rollback for the rules added for virtual networks
Posted by Laine Stump 1 year, 9 months ago
On 4/23/24 6:53 AM, Daniel P. Berrangé wrote:
> On Sun, Apr 21, 2024 at 10:53:24PM -0400, Laine Stump wrote:
> 
>> diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c
>> index 3a9f409e2a..e61787daec 100644
>> --- a/tests/networkxml2firewalltest.c
>> +++ b/tests/networkxml2firewalltest.c
>> @@ -79,7 +79,14 @@ testCommandDryRun(const char *const*args G_GNUC_UNUSED,
>>                     void *opaque G_GNUC_UNUSED)
>>   {
>>       *status = 0;
>> -    *output = g_strdup("");
>> +    /* if arg[1] is -ae then this is an nft command,
>> +     * and the caller requested to get the handle
>> +     * of the newly added object in stdout
>> +     */
>> +    if (STREQ_NULLABLE(args[1], "-ae"))
>> +        *output = g_strdup("# handle 5309");
>> +    else
>> +        *output = g_strdup("");
> 
> Belongs in the later nft tests patch

Ah, yep! Another byproduct of reordering patches (and 
splitting/recombining/rewriting others).
_______________________________________________
Devel mailing list -- devel@lists.libvirt.org
To unsubscribe send an email to devel-leave@lists.libvirt.org