Re: [Qemu-block] [Qemu-devel] [PATCH RFC v3 3/5] qapi: Permit scalar type conversions in QObjectInputVisitor

[Eric Blake]

On 02/21/2017 03:01 PM, Markus Armbruster wrote:
> From: "Daniel P. Berrange" <address@hidden>
> 
> Currently the QObjectInputVisitor assumes that all scalar values are
> directly represented as the final types declared by the thing being
> visited. i.e. it assumes an 'int' is using QInt, and a 'bool' is using
> QBool, etc.  This is good when QObjectInputVisitor is fed a QObject
> that came from a JSON document on the QMP monitor, as it will strictly
> validate correctness.
> 
> To allow QObjectInputVisitor to be reused for visiting a QObject
> originating from keyval_parse(), an alternative mode is needed where
> all the scalars types are represented as QString and converted on the
> fly to the final desired type.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> Message-Id: <address@hidden>
> 
> Rebased, conflicts resolved, commit message updated to refer to
> keyval_parse().
> 
> Control flow in qobject_input_type_number_autocast() simplified.
> 
> Additional tests in test-qemu-opts.c to verify QemuOpts compatibility.
> To make the tests pass, use qemu_strtou64() instead of
> parse_uint_full().
> 
> Use qemu_strtou64() and qemu_strtosz() instead of
> parse_option_number() and parse_option_size() so we have to call
> qobject_input_get_name() only when actually needed.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  include/qapi/qobject-input-visitor.h |  32 ++++-
>  qapi/qobject-input-visitor.c         | 165 ++++++++++++++++++++++++
>  tests/test-keyval.c                  | 241 
> +++++++++++++++++++++++++++++++++++
>  tests/test-qobject-input-visitor.c   | 194 +++++++++++++++++++++++++++-
>  4 files changed, 624 insertions(+), 8 deletions(-)
> 

> @@ -71,6 +72,7 @@ static const char 
> *qobject_input_get_name(QObjectInputVisitor *qiv,
>              g_string_prepend(qiv->errname, name);
>              g_string_prepend_c(qiv->errname, '.');
>          } else {
> +            /* TODO needs to be .%zu for autocast */
>              snprintf(buf, sizeof(buf), "[%zu]", so->index);

Ah, the TODO means you started thinking about list integration, and
that's part of why this is still RFC.


> +static void qobject_input_type_bool_autocast(Visitor *v, const char *name,
> +                                             bool *obj, Error **errp)
> +{
> +    QObjectInputVisitor *qiv = to_qiv(v);
> +    QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
> +    QString *qstr;
> +    const char *str;
> +
> +    if (!qobj) {
> +        return;
> +    }
> +    qstr = qobject_to_qstring(qobj);
> +    if (!qstr) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
> +                   qobject_input_get_name(qiv, name), "string");
> +        return;
> +    }
> +
> +    str = qstring_get_str(qstr);
> +    if (!strcmp(str, "on")) {
> +        *obj = true;
> +    } else if (!strcmp(str, "off")) {
> +        *obj = false;

Why are we reimplementing only a subset of parse_option_bool() here?

Overall, most of the changes from Dan's original look sane.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature