All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eduardo Habkost <ehabkost@redhat.com>
To: Markus Armbruster <armbru@redhat.com>
Cc: Daniel Henrique Barboza <danielhb@linux.ibm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	libvir-list@redhat.com, mdroth@linux.vnet.ibm.com,
	qemu-devel@nongnu.org, Anthony Perard <anthony.perard@citrix.com>,
	Igor Mammedov <imammedo@redhat.com>,
	dgilbert@redhat.com
Subject: Re: [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target
Date: Wed, 23 May 2018 09:27:49 -0300	[thread overview]
Message-ID: <20180523122749.GC8988@localhost.localdomain> (raw)
In-Reply-To: <874liyivcs.fsf@dusky.pond.sub.org>

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

  reply	other threads:[~2018-05-23 12:28 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-17 19:23 [Qemu-devel] [PATCH v7 0/3] wakeup-from-suspend and system_wakeup changes Daniel Henrique Barboza
2018-05-17 19:23 ` [Qemu-devel] [PATCH v7 1/3] qmp: adding 'wakeup-suspend-support' in query-target Daniel Henrique Barboza
2018-05-18  8:48   ` Markus Armbruster
2018-05-21 18:14     ` Eduardo Habkost
2018-05-21 19:46       ` Daniel Henrique Barboza
2018-05-21 20:26         ` Eduardo Habkost
2018-05-23  9:17           ` Markus Armbruster
2018-05-23 12:27             ` Eduardo Habkost [this message]
2018-05-23 14:11               ` Daniel Henrique Barboza
2018-05-23 15:53               ` Markus Armbruster
2018-05-24 18:57                 ` Eduardo Habkost
2018-05-25  6:30                   ` Markus Armbruster
2018-05-25 20:30                     ` Eduardo Habkost
2018-05-28  7:23                       ` Markus Armbruster
2018-05-29 14:55                         ` Eduardo Habkost
2018-06-19 20:29                           ` Daniel Henrique Barboza
2018-06-20  7:09                             ` Markus Armbruster
2018-05-17 19:23 ` [Qemu-devel] [PATCH v7 2/3] qga: update guest-suspend-ram and guest-suspend-hybrid descriptions Daniel Henrique Barboza
2018-05-17 19:23 ` [Qemu-devel] [PATCH v7 3/3] qmp.c: system_wakeup: runstate and wake-up support check Daniel Henrique Barboza
2018-05-18  8:48   ` Markus Armbruster
2018-05-18 12:52     ` Daniel Henrique Barboza
2018-05-18 15:00       ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180523122749.GC8988@localhost.localdomain \
    --to=ehabkost@redhat.com \
    --cc=anthony.perard@citrix.com \
    --cc=armbru@redhat.com \
    --cc=danielhb@linux.ibm.com \
    --cc=dgilbert@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=libvir-list@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=mst@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=sstabellini@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.