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.
* 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.
_______________________________________________
pve-devel mailing list
pve-devel@pve.proxmox.com
https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
