Hi,

On 05/23/2018 09:27 AM, Eduardo Habkost wrote:
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?

    

    

    The original idea of this series was to provide a way to inform
    management

    when not to execute a pm-suspend* command. This is a command from
    the

    guest agent, thus it's only available when the guest is already
    running. As far

    as I know there is no way to suspend the VM  without using the guest
    agent. 

    

    Thus, unless management wants to store this state to avoid querying
    it multiple

    times during the VM lifetime (I remember from Libvirt code that it
    stores some

    sort of capabilities of the domain in an internal state, although I
    can't recall if

    this info would be eligible for that), there is no need to query
    this until the VM

    is booted. 

    

    

    

    

    

      




      

        
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?