[libvirt] [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions

[Eduardo Habkost]

On Mon, May 30, 2016 at 11:33:38AM +0200, Markus Armbruster wrote:
> Eduardo Habkost <ehabkost at redhat.com> writes:
> 
> > Just noticed that I hadn't replied to this yet. Sorry for the
> > long delay!
> >
> > On Thu, May 12, 2016 at 09:46:25AM +0200, Markus Armbruster wrote:
> >> Eduardo Habkost <ehabkost at redhat.com> writes:
> > [...]
> >> > ##
> >> > # @CpuDefinitionInfo:
> >> > #
> >> > # Virtual CPU definition.
> >> > #
> >> > # @name: the name of the CPU definition
> >> > # @runnable: #optional. whether the CPU model us usable with the
> >> > #            current machine and accelerator. Omitted if we don't
> >> > #            know the answer. (since 2.7)
> >> > # @unavailable-features: List of attributes that prevent the CPU
> >> 
> >> Unless you drop the * sigil from '*unavailable-features', you need to
> >> insert #optional after the colon.
> >
> > Fixed.
> >
> >> 
> >> > #                        model from running in the current host.
> >> > #                        (since 2.7)
> >> > #
> >> > # @unavailable-features is a list of QOM property names that
> >> > # represent CPU model attributes that prevent the CPU from running.
> >> > # If the QOM property is read-only, that means the CPU model can
> >> > # never run in the current host. If the property is read-write, it
> >> > # means that it MAY be possible to run the CPU model in the current
> >> > # host if that property is changed. Management software can use it
> >> > # as hints to suggest or choose an alternative for the user, or
> >> > # just to generate meaningful error messages explaining why the CPU
> >> > # model can't be used.
> >> > #
> >> > # Since: 1.2.0
> >> > ##
> >> 
> >> Better.
> >> 
> >> Next issue: how @runnable and @unavailable-features are related isn't
> >> fully documented.  Here's my guess:
> >> 
> >> Combinations possible?            @runnable
> >> @unavailable-features       absent  false  true
> >> absent                         yes      ?     ?
> >> present, empty                   ?      ?     ?
> >> present, non-empty               ?    yes    no
> >
> > unavailable-features should be present only if runnable is false.
> > It may be absent or empty if the architecture code still doesn't
> > provide detailed info.
> >
> > Once we have additional architectures implementing the new
> > fields, we can consider requiring unavailable-features to be
> > always present (and non-empty) if runnable is false.
> >
> > In other words:
> >
> > Combinations possible?            @runnable
> > @unavailable-features       absent  false  true
> > absent                         yes  yes[1]  yes
> > present, empty                  no  yes[1]   no
> > present, non-empty              no   yes     no
> >
> > [1] I would like it to be "no", but I prefer to make it mandatory
> >     only after we get some experience with other architectures.
> >
> >
> > I'm making the following changes to the documentation:
> >
> >  # Virtual CPU definition.
> >  #
> >  # @name: the name of the CPU definition
> > -# @runnable: #optional. whether the CPU model us usable with the
> > +# @runnable: #optional Whether the CPU model us usable with the
> >  #            current machine and accelerator. Omitted if we don't
> >  #            know the answer. (since 2.7)
> > -# @unavailable-features: List of attributes that prevent the CPU
> > -#                        model from running in the current host.
> > +# @unavailable-features: #optional List of attributes that prevent
> > +#                        the CPU model from running in the current
> > +#                        host. Present only if @runnable is false.
> >  #                        (since 2.7)
> >  #
> >  # @unavailable-features is a list of QOM property names that
> 
> "Present only if @runnable is false" makes me wonder why we need two
> separate optional members tied together with constraints.  I dislike
> such constraints, and avoid them whenever practical.
> 
> The new members encode an answer to the question whether a certain CPU
> usable with the current machine an accelerator, and if no, why.
> The possible answers are:
> 
> (1) Don't know.
> (2) Yes.
> (3) No, but we can't say why.
> (4) No, and here's a list of reasons.
> 
> The two "dunno" answers (1) and (3) exist so we don't have to boil the
> CPU ocean now.
> 
> Without them, the natural solution is a single member, where (4) is
> encoded as nonempty list, and (2) could be encoded as empty list or
> absent.
> 
> Now let me try to fit in (1) and (3).
> 
> The obvious way to do (1) is absent.  So let's use empty list for (2).
> 
> That leaves (3).  I think the simplest solution that could possibly work
> is to treat it as a special "dunno" reason: encode it just like (4), but
> with a special "dunno" list element.  I'd use the empty string.
> 
> Could even be used if we need to distinguish
> 
> (4a) No, and here's the *complete* list of reasons.
> (4b) No, and here's a possibly incomplete list of reasons.
> 
> For (4b), include the "dunno" element with the others.
> 
> Unlike the proposed solution, this one doesn't leave interface crud
> behind if we succeed in getting rid of (1) and (3):
> 
> * When (1) goes away, the single member becomes mandatory.
> 
> * When (3) goes away, the special "dunno" list element no longer occurs.

I like your suggestion.

I suggest "type" as the "dunno" element. It would keep the
existing "QOM property name" semantics, and it would just mean
"sorry, the only advice we can currently give you is to choose a
different CPU type". It even matches the previous documentation I
sent describing the meaning of read-only property names.

Rewriting the docs again:

 # Virtual CPU definition.
 #
 # @name: the name of the CPU definition
-# @runnable: #optional Whether the CPU model us usable with the
-#            current machine and accelerator. Omitted if we don't
-#            know the answer. (since 2.7)
-# @unavailable-features: #optional List of attributes that prevent
+# @unavailable-features: #optional List of properties that prevent
 #                        the CPU model from running in the current
-#                        host. Present only if @runnable is false.
-#                        (since 2.7)
+#                        host. (since 2.7)
 #
 # @unavailable-features is a list of QOM property names that
 # represent CPU model attributes that prevent the CPU from running.
-# If the QOM property is read-only, that means the CPU model can
-# never run in the current host. If the property is read-write, it
+# If the QOM property is read-only, that means there's no known
+# way to make the CPU model run in the current host.
+# If the property is read-write, it
 # means that it MAY be possible to run the CPU model in the current
 # host if that property is changed. Management software can use it
 # as hints to suggest or choose an alternative for the user, or
 # just to generate meaningful error messages explaining why the CPU
 # model can't be used.
+# If @unavailable-features is an empty list, the CPU model is
+# runnable using the current host and machine-type.
+# If @unavailable-features is not present, runnability
+# information for the CPU model is not available.
 #
 # Since: 1.2.0
 ##


-- 
Eduardo