On 4/25/24 12:33 PM, Daniel P. Berrangé wrote:
On Thu, Apr 25, 2024 at 01:38:26AM -0400, Laine Stump wrote:
This virFirewall object will store the list of actions required to
remove the firewall that was added for the currently active instance
of the network, so it has been named "fwRemoval" (and when parsed into
XML, the <firewall> element will have the name "fwRemoval").
There are no uses of the fwRemoval object in the virNetworkObj yet,
but everything is in place to add it to the XML when formatted, parse
it from the XML when reading network status, and free the virFirewall
object when the virNetworkObj is freed.
Signed-off-by: Laine Stump <laine@xxxxxxxxxx>
Reviewed-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
---
Change from V2:
* add name='fwRemoval' to the network status <firewall> so it
can be positively identified in the XML
src/conf/virnetworkobj.c | 46 ++++++++++++++++++++++++++++++++++++++++
src/conf/virnetworkobj.h | 11 ++++++++++
src/libvirt_private.syms | 3 +++
3 files changed, 60 insertions(+)
diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c
index d5aa121e20..fef4c69004 100644
--- a/src/conf/virnetworkobj.c
+++ b/src/conf/virnetworkobj.c
@@ -55,6 +55,11 @@ struct _virNetworkObj {
unsigned int taint;
+ /* fwRemoval contains all commands needed to remove the firewall
+ * that was added for this network.
+ */
+ virFirewall *fwRemoval;
+
/* Immutable pointer, self locking APIs */
virMacMap *macmap;
@@ -239,6 +244,30 @@ virNetworkObjSetFloorSum(virNetworkObj *obj,
}
+virFirewall **
+virNetworkObjGetFwRemovalPtr(virNetworkObj *obj)
+{
+ return &obj->fwRemoval;
+}
+
+
+virFirewall *
+virNetworkObjGetFwRemoval(virNetworkObj *obj)
+{
+ return obj->fwRemoval;
+}
+
+
+void
+virNetworkObjSetFwRemoval(virNetworkObj *obj,
+ virFirewall *fwRemoval)
+{
+ obj->fwRemoval = fwRemoval;
+ /* give it a name so it's identifiable in the XML */
+ virFirewallSetName(fwRemoval, "fwRemoval");
+}
This call is segv'ing because 'fwRemoval' is NULL if you're
starting virnetworkd from an existing host which is currently
running old libvirt, as the FW rules won't be in the status
XML file. I guess this pristine upgrade scenario is not present
on your own machine.
Just putting an 'if (fwRemoval)' around the SetName call
seems like the right action.
Yeah, I found that this morning and fixed it as well (exactly as you
suggest). The danger of staying up late to finish something is that your
brain *thinks* you retested everything after that last fix, but often
you didn't :-/
Unless you want to immediately synthesize a virFirewall
object with historically accurate rollback rules when
loading status XML. Might make later code simpler if
you can rely on this being non-NULL ?
Well, having that pointer NULL is significant, as that's how I tell the
difference between "there was no fwRemoval in the status" (i.e. "use
the old hardcoded method of removing an iptables firewall") and "there
was an fwRemoval, but it was empty (i.e. "do nothing", which I'm
assuming will be the case when I add in an update of egarver's support
for native firewalld backend).
Although I suppose that empty <firewall> would still have <firewall
backend='firewalld'>, so maybe I don't need to differentiate.
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
