On Wed, May 23, 2018 at 11:17:55AM +0200, Markus Armbruster wrote:
Eduardo Habkost <ehabkost@redhat.com> writes:
On Mon, May 21, 2018 at 04:46:36PM -0300, Daniel Henrique Barboza wrote: [...]
Since no objection was made back then, this logic was put into query-target starting in v2. Still, I don't have any favorites though: query-target looks ok, query-machine looks ok and a new API looks ok too. It's all about what makes (more) sense in the management level, I think.
I understand the original objection from Eric: having to add a new command for every runtime flag we want to expose to the user looks wrong to me.
Agreed.
However, extending query-machines and query-target looks wrong too, however. query-target looks wrong because this not a property of the target. query-machines is wrong because this is not a static property of the machine-type, but of the running machine instance.
Of the two, query-machines looks less wrong.
Arguably, -no-acpi should not exist. It's an ad hoc flag that sneakily splits a few machine types into two variants, with and without ACPI. It's silently ignored for other machine types, even APCI-capable ones.
If the machine type variants with and without ACPI were separate types, wakeup-suspend-support would be a static property of the machine type.
However, "separate types" probably doesn't scale: I'm afraid we'd end up with an undesirable number of machine types. Avoiding that is exactly why we have machine types with configurable options. I suspect that's how ACPI should be configured (if at all).
So, should we make -no-acpi sugar for a machine type parameter? And then deprecate -no-acpi for good measure?
I think we should.
Can we have a new query command that could be an obvious container for simple machine capabilities that are not static? A name like "query-machine" would be generic enough for that, I guess.
Having command names differ only in a single letter is awkward, but let's focus on things other than naming now, and use query-current-machine like a working title.
query-machines is wrong because wakeup-suspend-support isn't static for some machine types.
query-current-machine is also kind of wrong because wakeup-suspend-support *is* static for most machine types.
The most appropriate solution depends a lot on how/when management software needs to query this. If they only need to query it at runtime for a running VM, there's no reason for us to go of our way and add complexity just to make it look like static data in query-machines. On the other hand, if they really need to query it before configuring/starting a VM, it won't be useful at all to make it available only at runtime. Daniel, when/how exactly software would need to query the new flag?
Worse, a machine type property that is static for all machine types now could conceivably become dynamic when we add a machine type configuration knob.
This isn't the first time a machine capability that seems static actually depends on other configuration arguments. We will probably need to address this eventually.
Would a way to tie the property to the configuration knob help? Something like wakeup-suspend-support taking values true (supported), false (not supported), and "acpi" (supported if machine type configuration knob "acpi" is switched on).
I would prefer a more generic mechanism. Maybe make 'query-machines' accept a 'machine-options' argument? -- Eduardo