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

[Eduardo Habkost]

On Mon, May 09, 2016 at 03:06:18PM +0200, David Hildenbrand wrote:
> > > > > 
> > > > > Just FYI, on other architectures (e.g. s390x), other conditions (e.g. cpu
> > > > > generation) also define if a CPU model is runnable, so the pure availability of
> > > > > features does not mean that a cpu model is runnable.
> > > > > 
> > > > > We could have runnable=false and unavailable-features being an empty list.    
> > > > 
> > > > Even on those cases, can't the root cause be mapped to a QOM
> > > > property name (e.g. "cpu-generation"), even if it's property that
> > > > can't be changed by the user?  
> > > 
> > > In the current state, no.  
> > 
> > But it could be implemented by s390x in the future, if it's
> > considered useful, right?
> 
> Yes, we could fit that into read-only properties if we would ever need it
> (like cpu-generation you mentioned and cpu-ga-level - both will always be
> read-only).
> 
> However we could come up with more optional fields for that in the future.
> (like unsupported-values or sth. like that). I actually prefer
> unavailable-features over runnability-blockers.
> 
> > 
> > > 
> > > So I think for runnable=false:
> > > a) unavailable-features set -> can be made runnable
> > > b) unavailable-features not set -> cannot be made runnable
> > > 
> > > would be enough.  
> > 
> > I understand it would be enough, but I would like to at least
> > define semantics that would make sense for all architectures in
> > case it gets implemented in the future. Would the proposal below
> > make sense?
> > 
> 
> Yes, I think so. However to really make good hints, upper layers would most
> likely need more information about the exact problem with a property -
> maybe something like an enum value per problematic property.
> (UNAVAILABLE_FEATURE, VALUE_TOO_BIG, VALUE_TOO_SMALL, UNSUPPORTED_VALUE) ...

If a more powerful interface is needed, we can add extra fields,
yes. Although I'm not sure we really start providing that level
of detail in a generic way (I guess it will depend on how much
arch-independent code libvirt will use to handle runnability
information).

> 
> > > > 
> > > > We could replace this with something more generic, like:
> > > > 
> > > > @runnability-blockers: List of attributes that prevent the CPU
> > > >   model from running in the current host.
> > > >   
> > > >   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.
> > > > 
> > > > (I am looking for a better name than "runnability-blockers").
> > > >   
> 
> Not sure which approach would be better.

Note that this is only a change in documentation of the new
field, to allow it to cover extra cases without any changes in
the interface.

I also don't like the "runnability-blockers" name, but I prefer
the new documentation I wrote above because it is more explicit
about what exactly the field means. I plan to keep the
"unavailable-features" name but use the above documentation text
in the next version. Does that sound OK?

-- 
Eduardo