[pve-devel] [PATCH pve-firewall 1/2] Manually construct guest config path

[Thomas Lamprecht]

Am 10/11/2023 um 14:26 schrieb Stefan Lendl:
> Thomas Lamprecht <t.lamprecht at proxmox.com> writes:
>> This won't work as now cfs_read_file only works by luck, if at all, as the
>> cfs_read_file needs the cfs_register_file that happens in PVE::QemuServer
>> so that the parser is actually available...
>>
>> I'd much rather see Firewall be split-up than doing broken hacks and
>> switching from one of our saner interfaces to manual assembly.
> 
> The issue arises because firewall depends on qemu-server but qemu-server
> depends on SDN. So if I try to include firewall from SDN, it will not work.

Yes, I knew that and read it from your first mail ;-)
My point was, the result to fix that should work and not introduce more
mess to the system.

> 
> I have looked at Firewall for quite some time now. Some functions in
> Firewall.pm depend on QemuServer mainly for the parse_net function. I
> tried to extract the functions that depend on QemuServer to another
> package but I couldn't get the tests to pass.

another package or another module?

> Firewall.pm is using several global variables and I couldn't identify
> what I missed.

Maybe post/push the WIP somewhere so others can help or pick it up (later).

> Another option would be to split the SDN module to allow QemuServer to
> depend only on a certain part of SDN to notify SDN about nic added to a
> VM and VM start. I have not analyzed if it's possible to can split the
> dependency cycle.

A rather different idea would be to avoid that SDN calls into firewall at
all, but like I wrote to my reply to the other patch, rather that the
firewall daemon automatically adds the aliases from the SDN config, after
all such an alias would be useable before the firewall daemon starts the
next round anyway, so from that POV it would be as good as this and avoid
doing direct API calls from some uncontrolled context (can mess with
various (file) permissions).

Ideally those would live in their own namespace, as said in the other
reply, as then one would also avoid odd effects, especially with existing
aliases.

> I don't see a clear path to implement this at this point and I will

Did you tried above option as POC, i.e., the one from my reply from last
Wednesday?

> focus on supporting Stefan Hanreich next week to finalize other aspects
> of SDN for a successful release.
> 

Yeah, I'm *still* missing any patches to the docs, those would be very
high priority as of now, and should be doable on the side, especially
as you tried the various existing features since a few weeks.

ps. as I asked you in the past, *please* avoid top-posting!