[2.1,09/28] vl: redo -object parsing

Follow the lines of the HMP implementation, using OptsVisitor
to parse the options.  This gives access to OptsVisitor's
rich parsing of integer lists.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 vl.c | 87 +++++++++++++++++++++++++++-----------------------------------------
 1 file changed, 35 insertions(+), 52 deletions(-)

On Tue, Mar 04, 2014 at 03:00:37PM +0100, Paolo Bonzini wrote:
> Follow the lines of the HMP implementation, using OptsVisitor
> to parse the options.  This gives access to OptsVisitor's
> rich parsing of integer lists.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  vl.c | 87 +++++++++++++++++++++++++++-----------------------------------------
>  1 file changed, 35 insertions(+), 52 deletions(-)
> 
> diff --git a/vl.c b/vl.c
> index ac5f425..e8709ee 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -119,8 +119,7 @@ int main(int argc, char **argv)
>  #include "qemu/osdep.h"
>  
>  #include "ui/qemu-spice.h"
> -#include "qapi/string-input-visitor.h"
> -#include "qom/object_interfaces.h"
> +#include "qapi/opts-visitor.h"
>  
>  #define DEFAULT_RAM_SIZE 128
>  
> @@ -2629,69 +2628,53 @@ static void free_and_trace(gpointer mem)
>      free(mem);
>  }
>  
> -static int object_set_property(const char *name, const char *value, void *opaque)
> -{
> -    Object *obj = OBJECT(opaque);
> -    StringInputVisitor *siv;
> -    Error *local_err = NULL;
> -
> -    if (strcmp(name, "qom-type") == 0 || strcmp(name, "id") == 0) {
> -        return 0;
> -    }
> -
> -    siv = string_input_visitor_new(value);
> -    object_property_set(obj, string_input_get_visitor(siv), name, &local_err);
> -    string_input_visitor_cleanup(siv);
> -
> -    if (local_err) {
> -        qerror_report_err(local_err);
> -        error_free(local_err);
> -        return -1;
> -    }
> -
> -    return 0;
> -}
> -
>  static int object_create(QemuOpts *opts, void *opaque)
>  {
> -    const char *type = qemu_opt_get(opts, "qom-type");
> -    const char *id = qemu_opts_id(opts);
> -    Error *local_err = NULL;
> -    Object *obj;
> -
> -    g_assert(type != NULL);
> -
> -    if (id == NULL) {
> -        qerror_report(QERR_MISSING_PARAMETER, "id");
> -        return -1;
> +    Error *err = NULL;
> +    char *type = NULL;
> +    char *id = NULL;
> +    void *dummy = NULL;
> +    OptsVisitor *ov;
> +    QDict *pdict;
> +
> +    ov = opts_visitor_new(opts);
> +    pdict = qemu_opts_to_qdict(opts, NULL);
> +
> +    visit_start_struct(opts_get_visitor(ov), &dummy, NULL, NULL, 0, &err);
> +    if (err) {
> +        goto out;
>      }
>  
> -    obj = object_new(type);
> -    if (qemu_opt_foreach(opts, object_set_property, obj, 1) < 0) {
> -        object_unref(obj);
> -        return -1;
> +    qdict_del(pdict, "qom-type");
> +    visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
> +    if (err) {
> +        goto out;
>      }

Can be moved up right before creating qdict.

>  
> -    if (!object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
> -        error_setg(&local_err, "object '%s' isn't supported by -object",
> -                   id);
> +    qdict_del(pdict, "id");
> +    visit_type_str(opts_get_visitor(ov), &id, "id", &err);
> +    if (err) {
>          goto out;
>      }

Can be moved up right before creating qdict.

>  
> -    user_creatable_complete(obj, &local_err);
> -    if (local_err) {
> +    object_add(type, id, pdict, opts_get_visitor(ov), &err);

I think it's better to move object_add() from qmp.c to qom/object.c.

> +    if (err) {
>          goto out;
>      }
> -
> -    object_property_add_child(container_get(object_get_root(), "/objects"),
> -                              id, obj, &local_err);
> +    visit_end_struct(opts_get_visitor(ov), &err);
> +    if (err) {
> +        qmp_object_del(id, NULL);
> +    }
>  
>  out:
> -    object_unref(obj);
> -    if (local_err) {
> -        qerror_report_err(local_err);
> -        error_free(local_err);
> -        return -1;
> +    opts_visitor_cleanup(ov);
> +
> +    QDECREF(pdict);
> +    g_free(id);
> +    g_free(type);
> +    g_free(dummy);
> +    if (err) {
> +        qerror_report_err(err);
>      }
>      return 0;
>  }
> -- 
> 1.8.5.3
>

Il 07/03/2014 03:56, Hu Tao ha scritto:
>> -    obj = object_new(type);
>> -    if (qemu_opt_foreach(opts, object_set_property, obj, 1) < 0) {
>> -        object_unref(obj);
>> -        return -1;
>> +    qdict_del(pdict, "qom-type");
>> +    visit_type_str(opts_get_visitor(ov), &type, "qom-type", &err);
>> +    if (err) {
>> +        goto out;
>>      }
>
> Can be moved up right before creating qdict.
>
>>
>> -    if (!object_dynamic_cast(obj, TYPE_USER_CREATABLE)) {
>> -        error_setg(&local_err, "object '%s' isn't supported by -object",
>> -                   id);
>> +    qdict_del(pdict, "id");
>> +    visit_type_str(opts_get_visitor(ov), &id, "id", &err);
>> +    if (err) {
>>          goto out;
>>      }
>
> Can be moved up right before creating qdict.

In both cases I prefer to keep the qdict_del and visit_type_str together.

>>
>> -    user_creatable_complete(obj, &local_err);
>> -    if (local_err) {
>> +    object_add(type, id, pdict, opts_get_visitor(ov), &err);
>
> I think it's better to move object_add() from qmp.c to qom/object.c.

No, I don't think so.  qom/object.c is not using QDict.  It is common 
for "human user interface" files (hmp.c, ui/gtk.c, in this case vl.c) to 
use qmp.c, the structure should be

           hmp.c, ui/gtk.c, vl.c
                  |
                qmp.c
                  |
         qom/, cpus.c, etc.

We could move parts of qmp.c to qom/qmp.c, that would be fine.

Paolo

>> +    if (err) {
>>          goto out;
>>      }
>> -
>> -    object_property_add_child(container_get(object_get_root(), "/objects"),
>> -                              id, obj, &local_err);
>> +    visit_end_struct(opts_get_visitor(ov), &err);
>> +    if (err) {
>> +        qmp_object_del(id, NULL);
>> +    }
>>
>>  out:
>> -    object_unref(obj);
>> -    if (local_err) {
>> -        qerror_report_err(local_err);
>> -        error_free(local_err);
>> -        return -1;
>> +    opts_visitor_cleanup(ov);
>> +
>> +    QDECREF(pdict);
>> +    g_free(id);
>> +    g_free(type);
>> +    g_free(dummy);
>> +    if (err) {
>> +        qerror_report_err(err);
>>      }
>>      return 0;
>>  }
>> --
>> 1.8.5.3
>>
>
>