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

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



On Tue, May 10, 2016 at 08:32:53AM +0200, David Hildenbrand wrote:
> 
> > > 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).
> 
> And I would actually (later) prefer to have something like
> bool too_new; (name tbd)
> 
> to cover the cpu-generation problem instead of having to expose read-only
> properties just for the sake of communicating that a cpu model is simply too new
> and cannot be made runnable toggling features.

That could work. Or a "CPUNotRunnableReason" enum with values
like MISSING_FEATURES, TOO_NEW, etc.

> 
> > 
> > >   
> > > > > > 
> > > > > > 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?
> > 
> 
> I like the read-only part about that, but still you should somehow clarify that
> we are dealing with boolean properties a.k.a features. At least that's my
> opinion.

The point is to not restrict it to boolean properties, if the
architecture code wants to return other properties. The
information is less useful for them, but they would still be
allowed.

-- 
Eduardo


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