qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Daniel P. Berrangé" <berrange@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>
Cc: John G Johnson <john.g.johnson@oracle.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	mtsirkin@redhat.com, Yan Zhao <yan.y.zhao@intel.com>,
	quintela@redhat.com, Jason Wang <jasowang@redhat.com>,
	"Zeng, Xin" <xin.zeng@intel.com>,
	qemu-devel@nongnu.org,
	"Dr. David Alan Gilbert" <dgilbert@redhat.com>,
	Kirti Wankhede <kwankhede@nvidia.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Alex Williamson <alex.williamson@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	Felipe Franciosi <felipe@nutanix.com>,
	Christophe de Dinechin <dinechin@redhat.com>,
	Thanos Makatos <thanos.makatos@nutanix.com>
Subject: Re: [RFC v3] VFIO Migration
Date: Wed, 11 Nov 2020 15:48:50 +0000	[thread overview]
Message-ID: <20201111154850.GG906488@redhat.com> (raw)
In-Reply-To: <20201111143615.GA1421166@stefanha-x1.localdomain>

On Wed, Nov 11, 2020 at 02:36:15PM +0000, Stefan Hajnoczi wrote:
> On Tue, Nov 10, 2020 at 12:12:31PM +0100, Paolo Bonzini wrote:
> > On 10/11/20 10:53, Stefan Hajnoczi wrote:
> > > "allowed_values"
> > >    The list all values that the device implementation accepts for this migration
> > >    parameter. Integer ranges can be described using "<min>-<max>" strings.
> > > 
> > >    Examples: ['a', 'b', 'c'], [1, 5, 7], ['0-255', 512, '1024-2048'], [true]
> > > 
> > >    This member is optional. When absent, any value suitable for the type may be
> > >    given but the device implementation may refuse certain values.
> > 
> > I'd rather make this simpler:
> > 
> > - remove allowed_values for strings.  Effect: discourages using strings as
> > enums, leaving them only for free-form values such as vendor name or model
> > name.
> 
> And introduce an enum type?
> 
> > - remove allowed_values for bools.  If off_value is absent the only allowed
> > value is init_value.  If off_value is present, both true and false are
> > allowed (and !off_value is the "on_value", so to speak).
> 
> Makes sense.
> 
> > - change allowed_values into allowed_min and allowed_max for int values.
> > Advantage: avoids having to parse strings as ranges.  Disadvantage: removes
> > expressiveness (cannot say "x must be a power of two"), but I'm not sure
> > it's worth the extra complication.
> 
> Yes, the current syntax supports sparse ranges and multiple ranges.
> 
> The trade-off is that a tool cannot validate inputs beforehand. You need
> to instantiate the device to see if it accepts your inputs. This is not
> great for management tools because they cannot select a destination
> device if they don't know which exact values are supported.
> 
> Daniel Berrange raised this requirement in a previous revision, so I
> wonder what his thoughts are?

In terms of validation I can't help but feel the whole proposal is
really very complicated.

In validating QEMU migration compatibility we merely compare the
versioned machine type.

IIUC, in this proposal, it would be more like exploding the machine
type into all its 100's of properties and then comparing each one
individually.

I really prefer the simpler model of QEMU versioned machine types
where compatibility is a simple string comparison, hiding the
100's of individual config parameters.  

Of course there are scenarios where this will lead a mgmt app to
refuse a migration, when it could in fact have permitted it.

eg  consider   pc-i440fx-4.0  and pc-i440fx-5.0 machine types,
which only differ in the value  "foo=7" and "foo=8" respectively.

Now if the target only supported machine type pc-i440fx-5.0, then
with a basic string comparison of machine type versin, we can't
migrate from a host uing pc-i440fx-4.0

If we exploded the machine type into its params, we could see that
we can migrate from pc-i440fx-4.0 to pc-i440fx-5.0, simply by
overriding the value of "foo".

So, yes, dealing with individual params is more flexible, but it
comes at an enourmous cost in complexity to process all the
parameters. I'm not convinced this is a good tradeoff. 


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-11 15:52 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-10  9:53 [RFC v3] VFIO Migration Stefan Hajnoczi
2020-11-10 11:12 ` Paolo Bonzini
2020-11-11 14:36   ` Stefan Hajnoczi
2020-11-11 15:48     ` Daniel P. Berrangé [this message]
2020-11-12 15:26       ` Cornelia Huck
2020-11-16 10:48       ` Stefan Hajnoczi
2020-11-16 11:15       ` Stefan Hajnoczi
2020-11-16 11:41         ` Daniel P. Berrangé
2020-11-16 12:03           ` Michael S. Tsirkin
2020-11-16 12:05             ` Daniel P. Berrangé
2020-11-16 12:34               ` Michael S. Tsirkin
2020-11-16 12:45                 ` Daniel P. Berrangé
2020-11-16 12:51                   ` Michael S. Tsirkin
2020-11-16 12:48         ` Gerd Hoffmann
2020-11-16 12:54           ` Michael S. Tsirkin
2020-11-16 12:06       ` Michael S. Tsirkin
2020-11-10 20:14 ` Alex Williamson
2020-11-11 11:48   ` Cornelia Huck
2020-11-11 15:14     ` Stefan Hajnoczi
2020-11-11 15:35       ` Cornelia Huck
2020-11-16 11:02         ` Stefan Hajnoczi
2020-11-16 13:52           ` Cornelia Huck
2020-11-16 17:30             ` Alex Williamson
2020-11-24 17:24               ` Dr. David Alan Gilbert
2020-11-11 15:10   ` Stefan Hajnoczi
2020-11-11 15:28     ` Cornelia Huck
2020-11-16 11:36       ` Stefan Hajnoczi
2020-11-11 11:19 ` Cornelia Huck
2020-11-11 15:35   ` Stefan Hajnoczi
2020-11-11 12:56 ` Dr. David Alan Gilbert
2020-11-11 15:34   ` Stefan Hajnoczi
2020-11-11 15:41     ` Dr. David Alan Gilbert
2020-11-16 14:38       ` Stefan Hajnoczi
2020-11-17  9:44         ` Michael S. Tsirkin
2020-12-01 13:17           ` Stefan Hajnoczi
2020-11-11 16:18 ` Thanos Makatos
2020-11-16 15:24   ` Stefan Hajnoczi
2020-11-24 17:29     ` Dr. David Alan Gilbert

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=20201111154850.GG906488@redhat.com \
    --to=berrange@redhat.com \
    --cc=alex.williamson@redhat.com \
    --cc=dgilbert@redhat.com \
    --cc=dinechin@redhat.com \
    --cc=felipe@nutanix.com \
    --cc=jasowang@redhat.com \
    --cc=john.g.johnson@oracle.com \
    --cc=kevin.tian@intel.com \
    --cc=kraxel@redhat.com \
    --cc=kwankhede@nvidia.com \
    --cc=mtsirkin@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=stefanha@redhat.com \
    --cc=thanos.makatos@nutanix.com \
    --cc=xin.zeng@intel.com \
    --cc=yan.y.zhao@intel.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).