Re: [Qemu-devel] [PATCH for-2.9 17/17] target-i386: Implement query-cpu-model-expansion QMP command

[Eduardo Habkost]

On Tue, Dec 13, 2016 at 08:20:39PM +0100, Markus Armbruster wrote:
[...]
> >> > +    if (type ==  CPU_MODEL_EXPANSION_TYPE_STATIC) {
> >> > +        /* static expansion force migration-unsafe features off: */
> >> > +        ret->q_static = ret->migration_safe = true;
> >> > +        qdict_del(props, "pmu");
> >> > +        qdict_del(props, "host-cache-info");
> >> > +    } else if (type == CPU_MODEL_EXPANSION_TYPE_FULL) {
> >> > +        QObject *o;
> >> > +        /* full expansion clear the static/migration-safe flags
> >> > +         * to indicate migration-unsafe features are on:
> >> > +         */
> >> > +        ret->q_static = true;
> >> > +        ret->migration_safe = true;
> >> > +
> >> > +        o = qdict_get(props, "pmu");
> >> > +        if (o && qbool_get_bool(qobject_to_qbool(o))) {
> >> > +            ret->q_static = ret->migration_safe = false;
> >> > +        }
> >> > +        o = qdict_get(props, "host-cache-info");
> >> > +        if (o && qbool_get_bool(qobject_to_qbool(o))) {
> >> > +            ret->q_static = ret->migration_safe = false;
> >> > +        }
> >> > +    } else {
> >> > +        error_setg(&err, "The requested expansion type is not 
> >> > supported.");
> >> 
> >> How can this happen?
> >> 
> >> If it can, it bombs when x86_cpu_to_dict() already set an error (see
> >> "use of the error API" above).
> >
> > This can happen if we change the QAPI schema to support another
> > expansion type in the future.
> 
> I'd make this an assertion, because it's a programming error.

I don't think it's a programming error. For example, if one day
the ppc maintainers decide they need a new expansion type for
some arch-specific requirement they have, they won't need to
touch the x86 and s390x code when changing the schema.

-- 
Eduardo