On 10/30/19 10:08 AM, Stefan Reiter wrote: > On 10/30/19 7:45 AM, Thomas Lamprecht wrote: >> On 10/22/19 2:48 PM, Dominik Csapak wrote: >>> otherwise, having multiple ipconfigX entries, can lead to different >>> instance-ids on different startups, which is not desired >>> >>> Signed-off-by: Dominik Csapak <d.csa...@proxmox.com> >>> --- >>> 2 issues i have with this: >>> * we have a cyclic dependency between PVE::QemuServer and >>> PVE::QemuServer::Cloudinit, and this patch increases that >>> we could fix this by refactoring all relevant code out of >>> QemuServer.pm, but this seems like much work, since the that code >>> depends on many parts of QemuServer, not sure how to proceed here... >> >> Stefan, the move to QemuSchema for such things could avoid that, >> or? Maybe we want to defer that change until that went through? >> > > PVE::QemuServer::Cloudinit has a lot of different references to > PVE::QemuServer, none of which are touched by my changes AFAICT (I see > network stuff, drive stuff, "windows_version", etc...) > > It's certainly possible (and would probably a good idea) to move all relevant > code to different modules as well, but my current series doesn't touch that.
No, i did not meant that at all. I meant if the QEMU MAX_NET limit would/could/should be moved to QemuSchema, so that this increase of tightening the cyclic dependency could be avoided? > >>> * i am not sure if using a getter for MAX_NETS is the right way here, >>> or if we should use constants (or something else?)... >> >> works good for me, a constant or "our" scoped variable isn't inherently >> better, IMO. >> >> So in general, OK, but I'll wait what Stefan thinks regarding the >> cyclic dependency. >> > > I'd say apply it now, when we (I?) get to improving that part of the code, > one reference to "get_max_nets" isn't going to make the difference. It's always just one here, and one there :D I'll see, thanks! _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
