Re: [Qemu-devel] [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
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
> > 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. > > > > > > > > > > > > > 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. Apart from that, this is fine with me! David
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
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
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
> > > > > > > > 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) ... > > > > > > 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. David
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
On Mon, May 09, 2016 at 02:05:05PM +0200, David Hildenbrand wrote: > > On Mon, May 09, 2016 at 10:54:53AM +0200, David Hildenbrand wrote: > > > > Extend query-cpu-definitions schema to allow it to return two new > > > > optional fields: "runnable" and "unavailable-features". > > > > "runnable" will tell if the CPU model can be run in the current > > > > host. "unavailable-features" will contain a list of CPU > > > > properties that are preventing the CPU model from running in the > > > > current host. > > > > > > > > Cc: David Hildenbrand > > > > Cc: Michael Mueller > > > > Cc: Christian Borntraeger > > > > Cc: Cornelia Huck > > > > Cc: Jiri Denemark > > > > Cc: libvir-l...@redhat.com > > > > Signed-off-by: Eduardo Habkost > > > > --- > > > > qapi-schema.json | 10 +- > > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > > > index 54634c4..450e6e7 100644 > > > > --- a/qapi-schema.json > > > > +++ b/qapi-schema.json > > > > @@ -2948,11 +2948,19 @@ > > > > # Virtual CPU definition. > > > > # > > > > # @name: the name of the CPU definition > > > > +# @runnable: true if the CPU model is runnable using the current > > > > +#machine and accelerator. Optional. Since 2.6. > > > > +# @unavailable-features: List of properties that prevent the CPU > > > > +#model from running in the current host, > > > > +#if @runnable is false. Optional. > > > > +#Since 2.6. > > > > > > 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? > > 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? > > > > 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"). > > > > > > David > -- Eduardo
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
> On Mon, May 09, 2016 at 10:54:53AM +0200, David Hildenbrand wrote: > > > Extend query-cpu-definitions schema to allow it to return two new > > > optional fields: "runnable" and "unavailable-features". > > > "runnable" will tell if the CPU model can be run in the current > > > host. "unavailable-features" will contain a list of CPU > > > properties that are preventing the CPU model from running in the > > > current host. > > > > > > Cc: David Hildenbrand > > > Cc: Michael Mueller > > > Cc: Christian Borntraeger > > > Cc: Cornelia Huck > > > Cc: Jiri Denemark > > > Cc: libvir-l...@redhat.com > > > Signed-off-by: Eduardo Habkost > > > --- > > > qapi-schema.json | 10 +- > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > > index 54634c4..450e6e7 100644 > > > --- a/qapi-schema.json > > > +++ b/qapi-schema.json > > > @@ -2948,11 +2948,19 @@ > > > # Virtual CPU definition. > > > # > > > # @name: the name of the CPU definition > > > +# @runnable: true if the CPU model is runnable using the current > > > +#machine and accelerator. Optional. Since 2.6. > > > +# @unavailable-features: List of properties that prevent the CPU > > > +#model from running in the current host, > > > +#if @runnable is false. Optional. > > > +#Since 2.6. > > > > 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. 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. > > 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"). > David
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
On Mon, May 09, 2016 at 10:54:53AM +0200, David Hildenbrand wrote: > > Extend query-cpu-definitions schema to allow it to return two new > > optional fields: "runnable" and "unavailable-features". > > "runnable" will tell if the CPU model can be run in the current > > host. "unavailable-features" will contain a list of CPU > > properties that are preventing the CPU model from running in the > > current host. > > > > Cc: David Hildenbrand > > Cc: Michael Mueller > > Cc: Christian Borntraeger > > Cc: Cornelia Huck > > Cc: Jiri Denemark > > Cc: libvir-l...@redhat.com > > Signed-off-by: Eduardo Habkost > > --- > > qapi-schema.json | 10 +- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 54634c4..450e6e7 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -2948,11 +2948,19 @@ > > # Virtual CPU definition. > > # > > # @name: the name of the CPU definition > > +# @runnable: true if the CPU model is runnable using the current > > +#machine and accelerator. Optional. Since 2.6. > > +# @unavailable-features: List of properties that prevent the CPU > > +#model from running in the current host, > > +#if @runnable is false. Optional. > > +#Since 2.6. > > 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? 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"). -- Eduardo
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
> Extend query-cpu-definitions schema to allow it to return two new
> optional fields: "runnable" and "unavailable-features".
> "runnable" will tell if the CPU model can be run in the current
> host. "unavailable-features" will contain a list of CPU
> properties that are preventing the CPU model from running in the
> current host.
>
> Cc: David Hildenbrand
> Cc: Michael Mueller
> Cc: Christian Borntraeger
> Cc: Cornelia Huck
> Cc: Jiri Denemark
> Cc: libvir-l...@redhat.com
> Signed-off-by: Eduardo Habkost
> ---
> qapi-schema.json | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 54634c4..450e6e7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2948,11 +2948,19 @@
> # Virtual CPU definition.
> #
> # @name: the name of the CPU definition
> +# @runnable: true if the CPU model is runnable using the current
> +#machine and accelerator. Optional. Since 2.6.
> +# @unavailable-features: List of properties that prevent the CPU
> +#model from running in the current host,
> +#if @runnable is false. Optional.
> +#Since 2.6.
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.
> #
> # Since: 1.2.0
> ##
> { 'struct': 'CpuDefinitionInfo',
> - 'data': { 'name': 'str' } }
> + 'data': { 'name': 'str',
> +'*runnable': 'bool',
> +'*unavailable-features': [ 'str' ] } }
>
> ##
> # @query-cpu-definitions:
David
Re: [Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions
Eduardo Habkost writes:
> Extend query-cpu-definitions schema to allow it to return two new
> optional fields: "runnable" and "unavailable-features".
> "runnable" will tell if the CPU model can be run in the current
> host. "unavailable-features" will contain a list of CPU
> properties that are preventing the CPU model from running in the
> current host.
>
> Cc: David Hildenbrand
> Cc: Michael Mueller
> Cc: Christian Borntraeger
> Cc: Cornelia Huck
> Cc: Jiri Denemark
> Cc: libvir-l...@redhat.com
> Signed-off-by: Eduardo Habkost
> ---
> qapi-schema.json | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 54634c4..450e6e7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2948,11 +2948,19 @@
> # Virtual CPU definition.
> #
> # @name: the name of the CPU definition
> +# @runnable: true if the CPU model is runnable using the current
> +#machine and accelerator. Optional. Since 2.6.
> +# @unavailable-features: List of properties that prevent the CPU
> +#model from running in the current host,
> +#if @runnable is false. Optional.
> +#Since 2.6.
> #
> # Since: 1.2.0
> ##
> { 'struct': 'CpuDefinitionInfo',
> - 'data': { 'name': 'str' } }
> + 'data': { 'name': 'str',
> +'*runnable': 'bool',
> +'*unavailable-features': [ 'str' ] } }
>
> ##
> # @query-cpu-definitions:
Can @unavailable-features be empty or missing when @runnable is true?
