[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] qmp: Add query-qemu-features

From: Kevin Wolf
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH 1/2] qmp: Add query-qemu-features
Date: Fri, 29 Mar 2019 18:29:07 +0100
User-agent: Mutt/1.11.3 (2019-02-01)

Am 29.03.2019 um 18:04 hat Kevin Wolf geschrieben:
> Am 29.03.2019 um 16:06 hat Eric Blake geschrieben:
> > On 3/29/19 8:52 AM, Markus Armbruster wrote:
> > > Subjective summary:
> > > 
> > > * For the known use cases, query-qemu-features is merely a crutch for
> > >   getting information into the QAPI QMP schema.  Such crutches are ugly,
> > >   but in absence of better ideas, ugly wins by default.
> > 
> > That is, this series qualifies as the crutch (if we can't come up with
> > anything closer in time for 4.0) - but your arguments say we may have
> > something closer:
> With your suggestion that we cover things by driver name ('file',
> 'host_device') rather than source file ('file-posix'), we actually can
> return true or false and it's more than just getting things added to the
> schema.
> I know that you also suggested making the existence of the field
> conditional, but in my opinion that's just a result of asserting that
> you don't need to actually execute the command. There is no reason to
> make the field conditional unless you never intend to execute the
> command, but just introspect it and draw conclusions from that.
> > > * I think I'd prefer adding @dynamic-auto-read-only to
> > >   BlockdevOptionsFile, or perhaps to BlockdevOptions.  Takes precedence
> > >   over @auto-read-only.  Consider deprecating @auto-read-only.
> > 
> > This is interesting. The existing flag keeps its semantics:
> > 
> > Open the file with automatic fallback to read-only, may or may not be
> > dynamic.
> > 
> > And the new flag is documented as:
> > 
> > Overrides auto-read-only, open the file with automatic fallback to
> > read-only, guaranteed to be dynamic (or fails if the guarantee cannot be
> > provided).
> > 
> > I don't know if we need the deprecation or not (especially if our plans
> > over time are to allow the dynamic behavior in more places than just
> > file-posix, where a guaranteed failure if not dynamic vs. a sane
> > fallback to only-on-initial-open may still be useful to some callers),
> > but the new parameter with the guaranteed semantics is definitely
> > introspectible, so it is closer to the problem than creating
> > query-qemu-features.  It also seems like something we could pull
> > together in a short amount of time.
> This means that dynamic-auto-read-only is separate from auto-read-only
> and provides different semantics. auto-read-only is one of the features
> that are implemented using a BDRV_O_* flag and that required option
> inheritance. Both are things that I prefer to be avoided whenever we
> can.
> I'd rather miss 4.0 than adding another option like this.
> It would also be hard to use because libvirt would have to decide
> whether to use auto-read-only (which is the only option supported by
> most drivers) or dynamic-auto-read-only (file-posix only at the moment).
> In other words, in 4.1 we might have the same problem again because
> another driver implemented dynamic-auto-read-only and we need to get
> this information to libvirt.
> So I think this is not a real solution.
> > > * We need command line introspection (old news).
> We do, but that's orthogonal to this one. Introspecting -blockdev
> wouldn't provide more information than introspecting blockdev-add.
> > > * A general method to make semantic changes visible syntactically would
> > >   be useful.  The "augment the QAPI schema with feature flags" idea we
> > >   discussed in last KVM Forum's hallway track could be a starting point.
> > 
> > These are not 4.0 goals.
> But do we want to build it for 4.1?
> If so, I would try to find alternatives to query-qemu-features for the
> short term because we don't need both. But if we don't actually want to
> build it, then I think query-qemu-features is still the next best thing
> and there is no need to find a different solution now.

One way to buy us some more time until 4.1 would be to extend
BlockdevOptionsFile with an optional field x-auto-read-only-is-dynamic
(type null), which is not to be used except for introspection and which
will be dropped again as soon as we get real feature flags in the schema
in 4.1.


Attachment: signature.asc
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]