public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Thomas Lamprecht <t.lamprecht@proxmox.com>
To: "DERUMIER, Alexandre" <Alexandre.DERUMIER@groupe-cyllene.com>,
	"pve-devel@lists.proxmox.com" <pve-devel@lists.proxmox.com>,
	"aderumier@odiso.com" <aderumier@odiso.com>
Subject: Re: [pve-devel] [PATCH pve-common 1/1] schema: add pve-targetstorage (moved from qemu-server)
Date: Thu, 30 Jun 2022 09:51:30 +0200	[thread overview]
Message-ID: <22fe96b6-9fd5-51f2-6ef9-e42a1f6cd3be@proxmox.com> (raw)
In-Reply-To: <cdf7aa016a849680a4dee7656127756d4e4d9d36.camel@groupe-cyllene.com>

On 30/06/2022 09:30, DERUMIER, Alexandre wrote:
>> this and "qemu-server: remove json schema pve-targetstorage (moved to
>> pve-common)"
>> seems rather unrelated from the firewall series.
>>
> I haved added it, because if you remove "use Firewall.pm" from lxc,
> 
> it's doesn't find the schema anymore (because QemuServer is require in
> Firewall.pm)
> 

Ah, ok, so an implicit, "invisible" dependency from pve-container to
qemu-server, yuck. Then I understand the relation (could be more obviously
noted in a commit message though :-) - anyhow

> 
> But yes, it could be done firstly in another patch series. (and It seem
> than fabian has already send a patch some months ago)
> 
> 

I can take a look a the ones from Fabian so we can get that out of the way
first.

> BTW, Do you known why we need  in pve-firewall :> > # dynamically include PVE::QemuServer and PVE::LXC> # to avoid dependency problems> my $have_qemu_server;> eval {>     require PVE::QemuServer;>     require PVE::QemuConfig;>     $have_qemu_server = 1;> };> > my $have_lxc;> eval {>     require PVE::LXC;>     $have_lxc = 1;> };> > > > ?> > is it because PVE::Qemuserver && PVE::LXC are also using use> PVE::Firewall ?> 
Yes. It's mostly to allow solving the circular dependency issue for bootstrapping
PVE in a easy way, e.g., for a new Debian release like Bookworm next year.

I mean, we only uses it to parse the networks of VM/CTs, maybe we could reorganise
that - from top of my head: either write a simple standalone parser in firewall that
gets out the relevant info or split out the config schema stuff in separate packages
for each VM and CTs, which could be a bit more work but maybe fit in our project to
convert more of the schema from perl to rust (for datacenter manager).
Anyhow, that is probably not as important now and can stay as is with the eval'd
requires.




  reply	other threads:[~2022-06-30  7:52 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-29  9:08 [pve-devel] [PATCH SERIES common/firewall/qemu-server/container 0/1] Cleanup use of PVE::Firewall Alexandre Derumier
2022-06-29  9:08 ` [pve-devel] [PATCH qemu-server 1/2] api2: use firewall helpers Alexandre Derumier
2022-06-29  9:08 ` [pve-devel] [PATCH pve-firewall 1/1] move clone_vmfw_conf && remove_vmfw_conf to a Helpers Alexandre Derumier
2022-11-16 17:09   ` [pve-devel] applied: " Thomas Lamprecht
2022-06-29  9:08 ` [pve-devel] [PATCH pve-container 1/2] remove unused use PVE::Firewall Alexandre Derumier
2022-06-29  9:08 ` [pve-devel] [PATCH pve-common 1/1] schema: add pve-targetstorage (moved from qemu-server) Alexandre Derumier
2022-06-29  9:34   ` Thomas Lamprecht
2022-06-29  9:46     ` Fabian Ebner
2022-06-29  9:48       ` Thomas Lamprecht
2022-06-30  7:30     ` DERUMIER, Alexandre
2022-06-30  7:51       ` Thomas Lamprecht [this message]
2022-06-29  9:08 ` [pve-devel] [PATCH pve-container 2/2] api2 : use firewall helpers Alexandre Derumier
2022-06-29  9:08 ` [pve-devel] [PATCH qemu-server 2/2] qemu-server: remove json schema pve-targetstorage (moved to pve-common) Alexandre Derumier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=22fe96b6-9fd5-51f2-6ef9-e42a1f6cd3be@proxmox.com \
    --to=t.lamprecht@proxmox.com \
    --cc=Alexandre.DERUMIER@groupe-cyllene.com \
    --cc=aderumier@odiso.com \
    --cc=pve-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal