Re: [Qemu-devel] [PATCH 1/5] hmp: Use visitor api for hmp_migrate_set_parameter()

[Markus Armbruster <armbru@redhat.com>]

Juan Quintela <quintela@redhat.com> writes:

> We only use it for int64 at this point, I am not able to find a way to
> parse an int with MiB units.

Use the string input visitor and visit_type_size() to parse with unit
suffixes.  The problem is hmp_migrate_set_parameter() wants "no suffix"
to mean "mebibytes", which the string input visitor can't do.  Good for
new code, because it helps keeping things consistent.  Not so good for
converting existing code.

> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  hmp.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hmp.c b/hmp.c
> index b9dd933..acbbc5c 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -29,6 +29,7 @@
>  #include "monitor/qdev.h"
>  #include "qapi/opts-visitor.h"
>  #include "qapi/qmp/qerror.h"
> +#include "qapi/string-input-visitor.h"
>  #include "qapi/string-output-visitor.h"
>  #include "qapi/util.h"
>  #include "qapi-visit.h"
> @@ -1523,8 +1524,9 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>  {
>      const char *param = qdict_get_str(qdict, "parameter");
>      const char *valuestr = qdict_get_str(qdict, "value");
> +    Visitor *v = string_input_visitor_new(valuestr);
>      uint64_t valuebw = 0;
> -    long valueint = 0;
> +    int64_t valueint = 0;
>      Error *err = NULL;
>      bool use_int_value = false;
>      int i, ret;
> @@ -1582,9 +1584,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>              }
>  
>              if (use_int_value) {
> -                if (qemu_strtol(valuestr, NULL, 10, &valueint) < 0) {
> -                    error_setg(&err, "Unable to parse '%s' as an int",
> -                               valuestr);
> +                visit_type_int(v, param, &valueint, &err);

I figure the error message changes from

    Unable to parse 'PARAM' as an int

to

    Parameter 'PARAM' expects an int64 value or range

The "or range" part is due to the string input visitor's support for
integer list, which is basically horrible.


> +                if (err) {
>                      goto cleanup;
>                  }
>                  /* Set all integers; only one has_FOO will be set, and
> @@ -1608,6 +1609,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
>      }
>  
>   cleanup:
> +    visit_free(v);
>      if (err) {
>          error_report_err(err);
>      }

Reviewed-by: Markus Armbruster <armbru@redhat.com>