qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>,
	qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Roman Bolshakov <r.bolshakov@yadro.com>,
	Paolo Bonzini <pbonzini@redhat.com>, John Snow <jsnow@redhat.com>
Subject: Re: [PATCH for-6.0 1/6] qapi: Add query-accel command
Date: Wed, 18 Nov 2020 11:56:12 +0000	[thread overview]
Message-ID: <20201118115612.GD229461@redhat.com> (raw)
In-Reply-To: <20201118112845.GA11988@merkur.fritz.box>

On Wed, Nov 18, 2020 at 12:28:45PM +0100, Kevin Wolf wrote:
> Am 18.11.2020 um 09:36 hat Markus Armbruster geschrieben:
> > >> >>                                       [...]  Even better would be
> > >> >> returning an array of KvmInfo with information on all supported
> > >> >> accelerators at once, rather than making the user call this command once
> > >> >> per name.
> > >> >
> > >> > Maybe.  It would save us the work of answering the question
> > >> > above, but is this (querying information for all accelerators at
> > >> > once) going to be a common use case?
> > >> 
> > >> I recommend to scratch the query-accel parameter, and return information
> > >> on all accelerators in this build of QEMU.  Simple, and consistent with
> > >> similar ad hoc queries.  If a client is interested in just one, fishing
> > >> it out of the list is easy enough.
> > >> 
> > >
> > > Works for me. I'll then leave KvmInfo as is and will introduce AccelInfo
> > > with two fields: name and enabled. enabled will be true only for
> > > currently active accelerator.
> > 
> > Please document that at most on result can have 'enabled': true.
> 
> This doesn't feel right.
> 
> If I understand right, the proposal is to get a result like this:
> 
>     [ { 'name': 'kvm', 'enabled': true },
>       { 'name': 'tcg', 'enabled': false },
>       { 'name': 'xen', 'enabled': false } ]
> 
> The condition that only one field can be enabled would only be in the
> documentation instead of being enforced.
> 
> Instead, consider a response like this:
> 
>     { 'available': [ 'kvm', 'tcg', 'xen' ],
>       'active': 'kvm' }
> 
> This is not only shorter, but also makes sure that only one accelerator
> can be reported as active.

I guess this can be extended with a union to report extra props for the
accelerator, discriminated on the 'active' field eg

     { 'available': [ 'kvm', 'tcg', 'xen' ],
       'active': 'kvm',
       'data': {
           "allow-nested": true,
       }
    }


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



  reply	other threads:[~2020-11-18 11:57 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-16 13:10 [PATCH for-6.0 0/6] Add HMP/QMP commands to query accelerator Roman Bolshakov
2020-11-16 13:10 ` [PATCH for-6.0 1/6] qapi: Add query-accel command Roman Bolshakov
2020-11-16 16:20   ` Eric Blake
2020-11-16 18:56     ` Roman Bolshakov
2020-11-16 21:13     ` Eduardo Habkost
2020-11-17  8:51       ` Markus Armbruster
2020-11-18  1:19         ` Roman Bolshakov
2020-11-18  8:36           ` Markus Armbruster
2020-11-18  9:21             ` Paolo Bonzini
2020-11-18 13:08               ` Markus Armbruster
2020-11-18 13:46                 ` Paolo Bonzini
2020-11-18 14:45                   ` Markus Armbruster
2020-11-18 14:54                     ` Paolo Bonzini
2020-11-18 14:00                 ` Roman Bolshakov
2020-11-18 11:28             ` Kevin Wolf
2020-11-18 11:56               ` Daniel P. Berrangé [this message]
2020-11-18 13:53                 ` Markus Armbruster
2020-11-18 15:45                   ` Eduardo Habkost
2020-11-18 15:56                     ` Eric Blake
2020-11-18 16:23                       ` Eduardo Habkost
2020-11-19 13:17                         ` Markus Armbruster
2020-11-30 17:05   ` Philippe Mathieu-Daudé
2020-11-16 13:10 ` [PATCH for-6.0 2/6] qapi: Rename KvmInfo to AccelInfo Roman Bolshakov
2020-11-27 10:40   ` Dr. David Alan Gilbert
2020-11-27 12:08     ` Markus Armbruster
2020-11-16 13:10 ` [PATCH for-6.0 3/6] qapi: Use qmp_query_accel() in qmp_query_kvm() Roman Bolshakov
2020-11-16 13:10 ` [PATCH for-6.0 4/6] softmmu: Remove kvm_available() Roman Bolshakov
2020-11-16 13:10 ` [PATCH for-6.0 5/6] hmp: Add 'info accel' command Roman Bolshakov
2020-11-27 10:39   ` Dr. David Alan Gilbert
2020-11-16 13:10 ` [PATCH for-6.0 6/6] qapi: Deprecate 'query-kvm' Roman Bolshakov
2020-11-27 10:50   ` Daniel P. Berrangé
2020-11-27 11:21     ` Peter Krempa
2020-11-27 11:45       ` Roman Bolshakov
2020-11-27 12:18         ` Peter Krempa
2020-11-27 15:44           ` Markus Armbruster
2020-11-27 16:30             ` Peter Krempa
2020-11-30  9:21               ` Markus Armbruster
2020-11-30 10:09                 ` Peter Krempa
2020-11-30 16:03                   ` Markus Armbruster
2020-11-30 15:30               ` Eric Blake
2020-11-27 15:53           ` Daniel P. Berrangé
2020-11-27 16:35             ` Peter Krempa
2020-11-19 14:41 ` [PATCH for-6.0 0/6] Add HMP/QMP commands to query accelerator Claudio Fontana
2020-11-19 15:46   ` Roman Bolshakov
2020-11-19 15:54     ` Claudio Fontana

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=20201118115612.GD229461@redhat.com \
    --to=berrange@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=r.bolshakov@yadro.com \
    /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).