[pve-devel] [PATCH qemu-server] fix #2390: use fixed order for cloudinits net config

[Stefan Reiter]

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.csapak at 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.