lxc / incus

Powerful system container and virtual machine manager
https://linuxcontainers.org/incus
Apache License 2.0
2.8k stars 224 forks source link

Profile inheritance and raw QEMU configuration keys #1400

Open bensmrs opened 6 days ago

bensmrs commented 6 days ago

raw.qemu* configuration keys override the ones defined in previous profiles. Sometimes, I’d like to write a profile that defines several keys in raw.qemu.conf, adds some things in raw.qemu and does additional processing in raw.qemu.qmp.scriptlet. And sometimes I’d also like to further modify these QEMU options in my VM settings, without having to repeat everything the profile says. Could we imagine a mechanism allowing, rather than replacing existing data, appending to them for the raw.qemu, raw.qemu.conf and raw.qemu.qmp.{early,{post,pre}-start} keys? (For raw.qemu.qmp.scriptlet, it gets trickier and I’m not sure what to propose…)

stgraber commented 6 days ago

We've so far stayed away from trying to do anything like this, despite it being equally useful for things like cloud-init. Though in the cloud-init case you at least do have the benefit of having both cloud-init.vendor-data and cloud-init.user-data with cloud-init itself merging the two together.

We obviously don't want to change the general behavior of config key overriding through profiles and local config, even just for a few select keys as we really want consistency there.

What we could do in theory is provide something like raw.qemu.conf.NAME and then have those be processed in alphabetical order with suitable merging/overriding. But we've also stayed away from doing that for cloud-init in the past as it adds quite a bit of complexity, makes it a fair bit harder for a user to get a good overview of their configuration and could be a bit of a slippery slope where the moment we do it for a few keys, we'll have request to do that for basically every single key in the API. This approach also has the downside of effectively nuking the namespace. So if we allow say raw.qemu.conf.XYZ, we won't be able to define raw.qemu.conf.disks or whatever for ourselves as a new config key as now everything under raw.qemu.conf is effectively reserved.

For the scriptlet case, my recommended solution would be to change the scriptlet so it's being fed the instance definition, then a single scriptlet could get through varying behavior based on the instance, its devices or config keys (including user.XYZ config keys). I think that may allow many cases to still have the one scriptlet but have it get sufficiently customized per instance.

Obviously that wouldn't really help you with raw.qemu and raw.qemu.conf, but it would at least cover the QMP case.

bensmrs commented 6 days ago

We obviously don't want to change the general behavior of config key overriding through profiles and local config, even just for a few select keys as we really want consistency there.

Yeah obviously

What we could do in theory is provide something like raw.qemu.conf.NAME and then have those be processed in alphabetical order with suitable merging/overriding. But we've also stayed away from doing that for cloud-init in the past as it adds quite a bit of complexity

Oh I’m not a fan of that AT ALL :)

I have two suggestions, both of them having the same pros and cons:

Pros: doesn’t pollute the namespace, processes properties in definition order Cons:

the moment we do it for a few keys, we'll have request to do that for basically every single key in the API.

But… is it that bad?

For the +: notation, we would only mark “extensible” keys, and as soon as there’s an actual need, define an “extension handler” doing what’s the most appropriate for the given key (e.g. concatenate with a space for raw.qemu, concatenate with \n\n for raw.qemu.conf, unmarshal > concatenate list > marshal for raw.qemu.qmp.{early,{post,pre}-start}…)

For the templating engine, this would open plenty of possibilities, such as renaming (single) network cards lan.{{ instance_name[:12] }}, and come with a one-time cost; if a key is a string, add a call to a template parser with a context (but the context is not trivial to define, as we’ll want some keys to be available before others).

To be honest, I kinda like both solutions, and they are not incompatible. The second one is pretty hard though (e.g. which template parameters do we allow, and for which keys?).

makes it a fair bit harder for a user to get a good overview of their configuration

That’s what --expanded is here for :)