qemu-devel.nongnu.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).