qemu-block
[Top][All Lists]
Advanced

[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:04:39 +0100
User-agent: Mutt/1.11.3 (2019-02-01)

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.

Kevin

Attachment: signature.asc
Description: PGP signature


reply via email to

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