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

[Thomas Lamprecht]

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

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!