On Wed, May 23, 2018 at 05:53:34PM +0200, Markus Armbruster wrote:
Eduardo Habkost <ehabkost(a)redhat.com> writes:
> On Wed, May 23, 2018 at 11:17:55AM +0200, Markus Armbruster wrote:
>> Eduardo Habkost <ehabkost(a)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.
Would you like to take care of it?
Adding to my TODO-list, I hope I will be able to do it before the
next release.
[...]
> >
> > 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.
> Then the best time to address it is now, provided we can
:)
I'm not sure this is the best time. If libvirt only needs the
runtime value and don't need any info at query-machines time, I
think support for this on query-machines will be left unused and
they will only use the query-current-machine value.
Just giving libvirt the runtime data it wants
(query-current-machine) seems way better than requiring libvirt
to interpret a set of rules and independently calculate something
QEMU already knows.
> >> 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?
> This can support arbitrary configuration dependencies,
unlike my
> proposal. However, I fear combinatorial explosion would make querying
> anything but "default configuration" and "current configuration"
> impractical, and "default configuration" would be basically useless, as
> you can't predict how arguments will affect the value query-machines.
> Whether this is an issue depends on how management
software wants to use
> query-machines.
> Whether the ability to support arbitrary configuration
dependencies is a
> useful feature or an invitation to stupid stunts is another open
> question :)
> Here's a synthesis of the two proposals: have
query-machines spell out
> which of its results are determinate, and which configuration bits need
> to be supplied to resolve the indeterminate ones. For machine type
> "pc-q35-*", wakeup-suspend-support would always yield true, but for
> "pc-i440fx-*" it would return true when passed an acpi: true argument,
> false when passed an acpi: false argument, and an encoding of
> "indeterminate, you need to pass an acpi argument to learn more" when
> passed no acpi argument.
I like this proposal for other query-machines fields (like bus
information), but I think doing this for wakeup-suspend-support
is overkill, based on Daniel's description of its intended usage.
> I'm not saying this synthesis makes sense, I'm
just exploring the design
> space.
> We need input from libvirt guys.
I have the impression we need real use cases so they can evaluate
the proposal. wakeup-suspend-support doesn't seem like a use
case that really needs support on query-machines (because we
can simply provide the data at runtime).
--
Eduardo