Patchwork [1/2] qapi: add visitor for parsing int[KMGT] input string

login
register
mail settings
Submitter Igor Mammedov
Date Dec. 6, 2012, 9:12 p.m.
Message ID <1354828325-16568-2-git-send-email-imammedo@redhat.com>
Download mbox | patch
Permalink /patch/204322/
State New
Headers show

Comments

Igor Mammedov - Dec. 6, 2012, 9:12 p.m.
Caller of visit_type_unit_suffixed_int() will have to specify
value of 'K' suffix via unit argument.
For Kbytes it's 1024, for Khz it's 1000.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 v2:
  - convert type_freq to type_unit_suffixed_int.
  - provide qapi_dealloc_type_unit_suffixed_int() impl.
---
 qapi/qapi-dealloc-visitor.c |  7 +++++++
 qapi/qapi-visit-core.c      | 13 +++++++++++++
 qapi/qapi-visit-core.h      |  8 ++++++++
 qapi/string-input-visitor.c | 22 ++++++++++++++++++++++
 4 files changed, 50 insertions(+)
Michael Roth - Dec. 6, 2012, 10:24 p.m.
On Thu, Dec 06, 2012 at 10:12:04PM +0100, Igor Mammedov wrote:
> Caller of visit_type_unit_suffixed_int() will have to specify
> value of 'K' suffix via unit argument.
> For Kbytes it's 1024, for Khz it's 1000.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>

> ---
>  v2:
>   - convert type_freq to type_unit_suffixed_int.
>   - provide qapi_dealloc_type_unit_suffixed_int() impl.
> ---
>  qapi/qapi-dealloc-visitor.c |  7 +++++++
>  qapi/qapi-visit-core.c      | 13 +++++++++++++
>  qapi/qapi-visit-core.h      |  8 ++++++++
>  qapi/string-input-visitor.c | 22 ++++++++++++++++++++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index 75214e7..57e662c 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -143,6 +143,12 @@ static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char *strings[],
>  {
>  }
> 
> +static void qapi_dealloc_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> +                                                const char *name,
> +                                                const int unit, Error **errp)
> +{
> +}
> +
>  Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
>  {
>      return &v->visitor;
> @@ -170,6 +176,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
>      v->visitor.type_str = qapi_dealloc_type_str;
>      v->visitor.type_number = qapi_dealloc_type_number;
>      v->visitor.type_size = qapi_dealloc_type_size;
> +    v->visitor.type_unit_suffixed_int = qapi_dealloc_type_unit_suffixed_int;
> 
>      QTAILQ_INIT(&v->stack);
> 
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 7a82b63..dcbc1a9 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -311,3 +311,16 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
>      g_free(enum_str);
>      *obj = value;
>  }
> +
> +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> +                                  const int unit, Error **errp)
> +{
> +    if (!error_is_set(errp)) {
> +        return;
> +    }
> +    if (v->type_unit_suffixed_int) {
> +        v->type_unit_suffixed_int(v, obj, name, unit, errp);
> +    } else {
> +        visit_type_int64(v, obj, name, errp);
> +    }
> +}
> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> index 60aceda..04e690a 100644
> --- a/qapi/qapi-visit-core.h
> +++ b/qapi/qapi-visit-core.h
> @@ -62,6 +62,12 @@ struct Visitor
>      void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
>      /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
>      void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> +    /*
> +     * visit_unit_suffixed_int() falls back to (*type_int64)()
> +     * if type_unit_suffixed_int is unset
> +    */
> +    void (*type_unit_suffixed_int)(Visitor *v, int64_t *obj, const char *name,
> +                                   const int unit, Error **errp);
>  };
> 
>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -91,5 +97,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> +                                  const int unit, Error **errp);
> 
>  #endif
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 497eb9a..d2bd154 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool *present,
>      *present = true;
>  }
> 
> +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> +                                         const char *name, const int unit,
> +                                         Error **errp)
> +{
> +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +    char *endp = (char *) siv->string;
> +    long long val = 0;
> +
> +    if (siv->string) {
> +        val = strtosz_suffix_unit(siv->string, &endp,
> +                             STRTOSZ_DEFSUFFIX_B, unit);
> +    }
> +    if (!siv->string || val == -1 || *endp) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +              "a value representable as a non-negative int64");
> +        return;
> +    }
> +
> +    *obj = val;
> +}
> +
>  Visitor *string_input_get_visitor(StringInputVisitor *v)
>  {
>      return &v->visitor;
> @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
>      v->visitor.type_str = parse_type_str;
>      v->visitor.type_number = parse_type_number;
>      v->visitor.start_optional = parse_start_optional;
> +    v->visitor.type_unit_suffixed_int = parse_type_unit_suffixed_int;
> 
>      v->string = str;
>      return v;
> -- 
> 1.7.11.7
>
Eduardo Habkost - Dec. 7, 2012, 11:55 a.m.
On Thu, Dec 06, 2012 at 10:12:04PM +0100, Igor Mammedov wrote:
> Caller of visit_type_unit_suffixed_int() will have to specify
> value of 'K' suffix via unit argument.
> For Kbytes it's 1024, for Khz it's 1000.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  v2:
>   - convert type_freq to type_unit_suffixed_int.
>   - provide qapi_dealloc_type_unit_suffixed_int() impl.
> ---
>  qapi/qapi-dealloc-visitor.c |  7 +++++++
>  qapi/qapi-visit-core.c      | 13 +++++++++++++
>  qapi/qapi-visit-core.h      |  8 ++++++++
>  qapi/string-input-visitor.c | 22 ++++++++++++++++++++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index 75214e7..57e662c 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -143,6 +143,12 @@ static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char *strings[],
>  {
>  }
>  
> +static void qapi_dealloc_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> +                                                const char *name,
> +                                                const int unit, Error **errp)
> +{
> +}
> +
>  Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
>  {
>      return &v->visitor;
> @@ -170,6 +176,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
>      v->visitor.type_str = qapi_dealloc_type_str;
>      v->visitor.type_number = qapi_dealloc_type_number;
>      v->visitor.type_size = qapi_dealloc_type_size;
> +    v->visitor.type_unit_suffixed_int = qapi_dealloc_type_unit_suffixed_int;
>  
>      QTAILQ_INIT(&v->stack);
>  
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 7a82b63..dcbc1a9 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -311,3 +311,16 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
>      g_free(enum_str);
>      *obj = value;
>  }
> +
> +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> +                                  const int unit, Error **errp)
> +{
> +    if (!error_is_set(errp)) {
> +        return;
> +    }
> +    if (v->type_unit_suffixed_int) {
> +        v->type_unit_suffixed_int(v, obj, name, unit, errp);
> +    } else {
> +        visit_type_int64(v, obj, name, errp);
> +    }
> +}
> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> index 60aceda..04e690a 100644
> --- a/qapi/qapi-visit-core.h
> +++ b/qapi/qapi-visit-core.h
> @@ -62,6 +62,12 @@ struct Visitor
>      void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
>      /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
>      void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> +    /*
> +     * visit_unit_suffixed_int() falls back to (*type_int64)()
> +     * if type_unit_suffixed_int is unset
> +    */
> +    void (*type_unit_suffixed_int)(Visitor *v, int64_t *obj, const char *name,
> +                                   const int unit, Error **errp);
>  };
>  
>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -91,5 +97,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> +                                  const int unit, Error **errp);
>  
>  #endif
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 497eb9a..d2bd154 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool *present,
>      *present = true;
>  }
>  
> +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> +                                         const char *name, const int unit,
> +                                         Error **errp)
> +{
> +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +    char *endp = (char *) siv->string;
> +    long long val = 0;
> +
> +    if (siv->string) {
> +        val = strtosz_suffix_unit(siv->string, &endp,
> +                             STRTOSZ_DEFSUFFIX_B, unit);
> +    }

I just noticed that strtosz_suffix_unit() is not as generic as I
expected: it interprets "100B" as "100", but the value we're reading may
be completely unrelated to byte counts.

It would be nice to fix this later, but the current interface/behavior
doesn't seem to break anything (as the k/M/G/T cases seem to work
properly), and it even matches the current frequency parsing code on
target-i386/cpu.c, so:

Reviewed-by: Eduardo Habkost <ehabkost@redhat.com>


> +    if (!siv->string || val == -1 || *endp) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +              "a value representable as a non-negative int64");
> +        return;
> +    }
> +
> +    *obj = val;
> +}
> +
>  Visitor *string_input_get_visitor(StringInputVisitor *v)
>  {
>      return &v->visitor;
> @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
>      v->visitor.type_str = parse_type_str;
>      v->visitor.type_number = parse_type_number;
>      v->visitor.start_optional = parse_start_optional;
> +    v->visitor.type_unit_suffixed_int = parse_type_unit_suffixed_int;
>  
>      v->string = str;
>      return v;
> -- 
> 1.7.11.7
>
Andreas Färber - Dec. 7, 2012, 6:57 p.m.
Am 06.12.2012 22:12, schrieb Igor Mammedov:
> Caller of visit_type_unit_suffixed_int() will have to specify
> value of 'K' suffix via unit argument.
> For Kbytes it's 1024, for Khz it's 1000.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  v2:
>   - convert type_freq to type_unit_suffixed_int.
>   - provide qapi_dealloc_type_unit_suffixed_int() impl.
> ---
>  qapi/qapi-dealloc-visitor.c |  7 +++++++
>  qapi/qapi-visit-core.c      | 13 +++++++++++++
>  qapi/qapi-visit-core.h      |  8 ++++++++
>  qapi/string-input-visitor.c | 22 ++++++++++++++++++++++
>  4 files changed, 50 insertions(+)
> 
> diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> index 75214e7..57e662c 100644
> --- a/qapi/qapi-dealloc-visitor.c
> +++ b/qapi/qapi-dealloc-visitor.c
> @@ -143,6 +143,12 @@ static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char *strings[],
>  {
>  }
>  
> +static void qapi_dealloc_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> +                                                const char *name,
> +                                                const int unit, Error **errp)
> +{
> +}
> +
>  Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
>  {
>      return &v->visitor;
> @@ -170,6 +176,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
>      v->visitor.type_str = qapi_dealloc_type_str;
>      v->visitor.type_number = qapi_dealloc_type_number;
>      v->visitor.type_size = qapi_dealloc_type_size;
> +    v->visitor.type_unit_suffixed_int = qapi_dealloc_type_unit_suffixed_int;
>  
>      QTAILQ_INIT(&v->stack);
>  
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 7a82b63..dcbc1a9 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -311,3 +311,16 @@ void input_type_enum(Visitor *v, int *obj, const char *strings[],
>      g_free(enum_str);
>      *obj = value;
>  }
> +
> +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> +                                  const int unit, Error **errp)
> +{
> +    if (!error_is_set(errp)) {

if (error_is_set(errp)) {

> +        return;
> +    }
> +    if (v->type_unit_suffixed_int) {
> +        v->type_unit_suffixed_int(v, obj, name, unit, errp);
> +    } else {
> +        visit_type_int64(v, obj, name, errp);
> +    }
> +}
> diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> index 60aceda..04e690a 100644
> --- a/qapi/qapi-visit-core.h
> +++ b/qapi/qapi-visit-core.h
> @@ -62,6 +62,12 @@ struct Visitor
>      void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
>      /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
>      void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
> +    /*
> +     * visit_unit_suffixed_int() falls back to (*type_int64)()
> +     * if type_unit_suffixed_int is unset
> +    */

Indentation is one off.

> +    void (*type_unit_suffixed_int)(Visitor *v, int64_t *obj, const char *name,
> +                                   const int unit, Error **errp);

Are we expecting differently suffixed ints? Otherwise we could
optionally shorten to type_suffixed_int (but that probably still doesn't
fit within one comment line ;)).

>  };
>  
>  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> @@ -91,5 +97,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
>  void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
>  void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
>  void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
> +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> +                                  const int unit, Error **errp);
>  
>  #endif
> diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> index 497eb9a..d2bd154 100644
> --- a/qapi/string-input-visitor.c
> +++ b/qapi/string-input-visitor.c
> @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool *present,
>      *present = true;
>  }
>  
> +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> +                                         const char *name, const int unit,
> +                                         Error **errp)
> +{
> +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> +    char *endp = (char *) siv->string;
> +    long long val = 0;
> +
> +    if (siv->string) {
> +        val = strtosz_suffix_unit(siv->string, &endp,
> +                             STRTOSZ_DEFSUFFIX_B, unit);
> +    }
> +    if (!siv->string || val == -1 || *endp) {
> +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> +              "a value representable as a non-negative int64");

Weird indentation remaining, looks as if we could align with errp within
80 chars.

However, I wonder if "unit" is the (physically etc.) correct term here?
Isn't the "unit" Hz / byte / ... and 1000 more of a conversion factor or
something? At least that's the way I've seen unit used in the API of
another project, passing an enum of Hertz, gram, meter/second, etc.

Andreas

> +        return;
> +    }
> +
> +    *obj = val;
> +}
> +
>  Visitor *string_input_get_visitor(StringInputVisitor *v)
>  {
>      return &v->visitor;
> @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const char *str)
>      v->visitor.type_str = parse_type_str;
>      v->visitor.type_number = parse_type_number;
>      v->visitor.start_optional = parse_start_optional;
> +    v->visitor.type_unit_suffixed_int = parse_type_unit_suffixed_int;
>  
>      v->string = str;
>      return v;
>
Eduardo Habkost - Dec. 7, 2012, 7:53 p.m.
On Fri, Dec 07, 2012 at 07:57:35PM +0100, Andreas Färber wrote:
[...]
> > +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > +                                         const char *name, const int unit,
> > +                                         Error **errp)
> > +{
> > +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> > +    char *endp = (char *) siv->string;
> > +    long long val = 0;
> > +
> > +    if (siv->string) {
> > +        val = strtosz_suffix_unit(siv->string, &endp,
> > +                             STRTOSZ_DEFSUFFIX_B, unit);
> > +    }
> > +    if (!siv->string || val == -1 || *endp) {
> > +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > +              "a value representable as a non-negative int64");
> 
> Weird indentation remaining, looks as if we could align with errp within
> 80 chars.
> 
> However, I wonder if "unit" is the (physically etc.) correct term here?
> Isn't the "unit" Hz / byte / ... and 1000 more of a conversion factor or
> something? At least that's the way I've seen unit used in the API of
> another project, passing an enum of Hertz, gram, meter/second, etc.

I also think that calling it "unit" is confusing. Strictly speaking, the
"unit" we're dealing with is Hz (and the visitor simply don't care about
unit, it only looks for "unit prefixes").

Can't we call the 1000/1024 parameter "base" or "factor", instead?
Igor Mammedov - Dec. 10, 2012, 4:01 p.m.
On Fri, 07 Dec 2012 19:57:35 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 06.12.2012 22:12, schrieb Igor Mammedov:
> > Caller of visit_type_unit_suffixed_int() will have to specify
> > value of 'K' suffix via unit argument.
> > For Kbytes it's 1024, for Khz it's 1000.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  v2:
> >   - convert type_freq to type_unit_suffixed_int.
> >   - provide qapi_dealloc_type_unit_suffixed_int() impl.
> > ---
> >  qapi/qapi-dealloc-visitor.c |  7 +++++++
> >  qapi/qapi-visit-core.c      | 13 +++++++++++++
> >  qapi/qapi-visit-core.h      |  8 ++++++++
> >  qapi/string-input-visitor.c | 22 ++++++++++++++++++++++
> >  4 files changed, 50 insertions(+)
> > 
> > diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> > index 75214e7..57e662c 100644
> > --- a/qapi/qapi-dealloc-visitor.c
> > +++ b/qapi/qapi-dealloc-visitor.c
> > @@ -143,6 +143,12 @@ static void qapi_dealloc_type_enum(Visitor *v, int
> > *obj, const char *strings[], {
> >  }
> >  
> > +static void qapi_dealloc_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > +                                                const char *name,
> > +                                                const int unit, Error
> > **errp) +{
> > +}
> > +
> >  Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
> >  {
> >      return &v->visitor;
> > @@ -170,6 +176,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
> >      v->visitor.type_str = qapi_dealloc_type_str;
> >      v->visitor.type_number = qapi_dealloc_type_number;
> >      v->visitor.type_size = qapi_dealloc_type_size;
> > +    v->visitor.type_unit_suffixed_int =
> > qapi_dealloc_type_unit_suffixed_int; 
> >      QTAILQ_INIT(&v->stack);
> >  
> > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> > index 7a82b63..dcbc1a9 100644
> > --- a/qapi/qapi-visit-core.c
> > +++ b/qapi/qapi-visit-core.c
> > @@ -311,3 +311,16 @@ void input_type_enum(Visitor *v, int *obj, const
> > char *strings[], g_free(enum_str);
> >      *obj = value;
> >  }
> > +
> > +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char
> > *name,
> > +                                  const int unit, Error **errp)
> > +{
> > +    if (!error_is_set(errp)) {
> 
> if (error_is_set(errp)) {
Thanks, I'll fix it.

> > +        return;
> > +    }
> > +    if (v->type_unit_suffixed_int) {
> > +        v->type_unit_suffixed_int(v, obj, name, unit, errp);
> > +    } else {
> > +        visit_type_int64(v, obj, name, errp);
> > +    }
> > +}
> > diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> > index 60aceda..04e690a 100644
> > --- a/qapi/qapi-visit-core.h
> > +++ b/qapi/qapi-visit-core.h
> > @@ -62,6 +62,12 @@ struct Visitor
> >      void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error
> > **errp); /* visit_type_size() falls back to (*type_uint64)() if type_size
> > is unset */ void (*type_size)(Visitor *v, uint64_t *obj, const char
> > *name, Error **errp);
> > +    /*
> > +     * visit_unit_suffixed_int() falls back to (*type_int64)()
> > +     * if type_unit_suffixed_int is unset
> > +    */
> 
> Indentation is one off.
ditto

> 
> > +    void (*type_unit_suffixed_int)(Visitor *v, int64_t *obj, const char
> > *name,
> > +                                   const int unit, Error **errp);
> 
> Are we expecting differently suffixed ints? Otherwise we could
> optionally shorten to type_suffixed_int (but that probably still doesn't
> fit within one comment line ;)).
Not with current implementation. I'll shorten it as you've suggested.

> 
> >  };
> >  
> >  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> > @@ -91,5 +97,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const
> > char *name, Error **errp); void visit_type_bool(Visitor *v, bool *obj,
> > const char *name, Error **errp); void visit_type_str(Visitor *v, char
> > **obj, const char *name, Error **errp); void visit_type_number(Visitor
> > *v, double *obj, const char *name, Error **errp); +void
> > visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> > +                                  const int unit, Error **errp);
> >  
> >  #endif
> > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > index 497eb9a..d2bd154 100644
> > --- a/qapi/string-input-visitor.c
> > +++ b/qapi/string-input-visitor.c
> > @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool
> > *present, *present = true;
> >  }
> >  
> > +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > +                                         const char *name, const int
> > unit,
> > +                                         Error **errp)
> > +{
> > +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> > +    char *endp = (char *) siv->string;
> > +    long long val = 0;
> > +
> > +    if (siv->string) {
> > +        val = strtosz_suffix_unit(siv->string, &endp,
> > +                             STRTOSZ_DEFSUFFIX_B, unit);
> > +    }
> > +    if (!siv->string || val == -1 || *endp) {
> > +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > +              "a value representable as a non-negative int64");
> 
> Weird indentation remaining, looks as if we could align with errp within
> 80 chars.
Thanks, I'll fix it.

> 
> However, I wonder if "unit" is the (physically etc.) correct term here?
> Isn't the "unit" Hz / byte / ... and 1000 more of a conversion factor or
> something? At least that's the way I've seen unit used in the API of
> another project, passing an enum of Hertz, gram, meter/second, etc.
If we are to generalize it to integer than units might not make much sense,
they could be anything. Perhaps 'suffix_factor' would be descriptive enough
+ adding documentation comment to the visitor.

> 
> Andreas
> 
> > +        return;
> > +    }
> > +
> > +    *obj = val;
> > +}
> > +
> >  Visitor *string_input_get_visitor(StringInputVisitor *v)
> >  {
> >      return &v->visitor;
> > @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const
> > char *str) v->visitor.type_str = parse_type_str;
> >      v->visitor.type_number = parse_type_number;
> >      v->visitor.start_optional = parse_start_optional;
> > +    v->visitor.type_unit_suffixed_int = parse_type_unit_suffixed_int;
> >  
> >      v->string = str;
> >      return v;
> > 
> 
>
Michael Roth - Dec. 10, 2012, 5:27 p.m.
On Mon, Dec 10, 2012 at 05:01:38PM +0100, Igor Mammedov wrote:
> On Fri, 07 Dec 2012 19:57:35 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
> > Am 06.12.2012 22:12, schrieb Igor Mammedov:
> > > Caller of visit_type_unit_suffixed_int() will have to specify
> > > value of 'K' suffix via unit argument.
> > > For Kbytes it's 1024, for Khz it's 1000.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > ---
> > >  v2:
> > >   - convert type_freq to type_unit_suffixed_int.
> > >   - provide qapi_dealloc_type_unit_suffixed_int() impl.
> > > ---
> > >  qapi/qapi-dealloc-visitor.c |  7 +++++++
> > >  qapi/qapi-visit-core.c      | 13 +++++++++++++
> > >  qapi/qapi-visit-core.h      |  8 ++++++++
> > >  qapi/string-input-visitor.c | 22 ++++++++++++++++++++++
> > >  4 files changed, 50 insertions(+)
> > > 
> > > diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> > > index 75214e7..57e662c 100644
> > > --- a/qapi/qapi-dealloc-visitor.c
> > > +++ b/qapi/qapi-dealloc-visitor.c
> > > @@ -143,6 +143,12 @@ static void qapi_dealloc_type_enum(Visitor *v, int
> > > *obj, const char *strings[], {
> > >  }
> > >  
> > > +static void qapi_dealloc_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > > +                                                const char *name,
> > > +                                                const int unit, Error
> > > **errp) +{
> > > +}
> > > +
> > >  Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
> > >  {
> > >      return &v->visitor;
> > > @@ -170,6 +176,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
> > >      v->visitor.type_str = qapi_dealloc_type_str;
> > >      v->visitor.type_number = qapi_dealloc_type_number;
> > >      v->visitor.type_size = qapi_dealloc_type_size;
> > > +    v->visitor.type_unit_suffixed_int =
> > > qapi_dealloc_type_unit_suffixed_int; 
> > >      QTAILQ_INIT(&v->stack);
> > >  
> > > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> > > index 7a82b63..dcbc1a9 100644
> > > --- a/qapi/qapi-visit-core.c
> > > +++ b/qapi/qapi-visit-core.c
> > > @@ -311,3 +311,16 @@ void input_type_enum(Visitor *v, int *obj, const
> > > char *strings[], g_free(enum_str);
> > >      *obj = value;
> > >  }
> > > +
> > > +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char
> > > *name,
> > > +                                  const int unit, Error **errp)
> > > +{
> > > +    if (!error_is_set(errp)) {
> > 
> > if (error_is_set(errp)) {
> Thanks, I'll fix it.
> 
> > > +        return;
> > > +    }
> > > +    if (v->type_unit_suffixed_int) {
> > > +        v->type_unit_suffixed_int(v, obj, name, unit, errp);
> > > +    } else {
> > > +        visit_type_int64(v, obj, name, errp);
> > > +    }
> > > +}
> > > diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> > > index 60aceda..04e690a 100644
> > > --- a/qapi/qapi-visit-core.h
> > > +++ b/qapi/qapi-visit-core.h
> > > @@ -62,6 +62,12 @@ struct Visitor
> > >      void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error
> > > **errp); /* visit_type_size() falls back to (*type_uint64)() if type_size
> > > is unset */ void (*type_size)(Visitor *v, uint64_t *obj, const char
> > > *name, Error **errp);
> > > +    /*
> > > +     * visit_unit_suffixed_int() falls back to (*type_int64)()
> > > +     * if type_unit_suffixed_int is unset
> > > +    */
> > 
> > Indentation is one off.
> ditto
> 
> > 
> > > +    void (*type_unit_suffixed_int)(Visitor *v, int64_t *obj, const char
> > > *name,
> > > +                                   const int unit, Error **errp);
> > 
> > Are we expecting differently suffixed ints? Otherwise we could
> > optionally shorten to type_suffixed_int (but that probably still doesn't
> > fit within one comment line ;)).
> Not with current implementation. I'll shorten it as you've suggested.
> 
> > 
> > >  };
> > >  
> > >  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> > > @@ -91,5 +97,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const
> > > char *name, Error **errp); void visit_type_bool(Visitor *v, bool *obj,
> > > const char *name, Error **errp); void visit_type_str(Visitor *v, char
> > > **obj, const char *name, Error **errp); void visit_type_number(Visitor
> > > *v, double *obj, const char *name, Error **errp); +void
> > > visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> > > +                                  const int unit, Error **errp);
> > >  
> > >  #endif
> > > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > > index 497eb9a..d2bd154 100644
> > > --- a/qapi/string-input-visitor.c
> > > +++ b/qapi/string-input-visitor.c
> > > @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool
> > > *present, *present = true;
> > >  }
> > >  
> > > +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > > +                                         const char *name, const int
> > > unit,
> > > +                                         Error **errp)
> > > +{
> > > +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> > > +    char *endp = (char *) siv->string;
> > > +    long long val = 0;
> > > +
> > > +    if (siv->string) {
> > > +        val = strtosz_suffix_unit(siv->string, &endp,
> > > +                             STRTOSZ_DEFSUFFIX_B, unit);
> > > +    }
> > > +    if (!siv->string || val == -1 || *endp) {
> > > +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > > +              "a value representable as a non-negative int64");
> > 
> > Weird indentation remaining, looks as if we could align with errp within
> > 80 chars.
> Thanks, I'll fix it.
> 
> > 
> > However, I wonder if "unit" is the (physically etc.) correct term here?
> > Isn't the "unit" Hz / byte / ... and 1000 more of a conversion factor or
> > something? At least that's the way I've seen unit used in the API of
> > another project, passing an enum of Hertz, gram, meter/second, etc.
> If we are to generalize it to integer than units might not make much sense,
> they could be anything. Perhaps 'suffix_factor' would be descriptive enough
> + adding documentation comment to the visitor.

The real distinction I think is base=2 vs. base=10, but that might cause
confusion WRT to how the numberical value should be intepreted (10==2
vs. 10==10), so maybe suffix_base==2|10, or suffix_factor==1000/1024 as you
suggested. I think I'd prefer the former but either works for me.

> 
> > 
> > Andreas
> > 
> > > +        return;
> > > +    }
> > > +
> > > +    *obj = val;
> > > +}
> > > +
> > >  Visitor *string_input_get_visitor(StringInputVisitor *v)
> > >  {
> > >      return &v->visitor;
> > > @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const
> > > char *str) v->visitor.type_str = parse_type_str;
> > >      v->visitor.type_number = parse_type_number;
> > >      v->visitor.start_optional = parse_start_optional;
> > > +    v->visitor.type_unit_suffixed_int = parse_type_unit_suffixed_int;
> > >  
> > >      v->string = str;
> > >      return v;
> > > 
> > 
> > 
>
Igor Mammedov - Dec. 10, 2012, 7:03 p.m.
On Mon, 10 Dec 2012 11:27:46 -0600
mdroth <mdroth@linux.vnet.ibm.com> wrote:

> On Mon, Dec 10, 2012 at 05:01:38PM +0100, Igor Mammedov wrote:
> > On Fri, 07 Dec 2012 19:57:35 +0100
> > Andreas Färber <afaerber@suse.de> wrote:
> > 
> > > Am 06.12.2012 22:12, schrieb Igor Mammedov:
> > > > Caller of visit_type_unit_suffixed_int() will have to specify
> > > > value of 'K' suffix via unit argument.
> > > > For Kbytes it's 1024, for Khz it's 1000.
> > > > 
> > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > > > ---
> > > >  v2:
> > > >   - convert type_freq to type_unit_suffixed_int.
> > > >   - provide qapi_dealloc_type_unit_suffixed_int() impl.
> > > > ---
> > > >  qapi/qapi-dealloc-visitor.c |  7 +++++++
> > > >  qapi/qapi-visit-core.c      | 13 +++++++++++++
> > > >  qapi/qapi-visit-core.h      |  8 ++++++++
> > > >  qapi/string-input-visitor.c | 22 ++++++++++++++++++++++
> > > >  4 files changed, 50 insertions(+)
> > > > 
> > > > diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
> > > > index 75214e7..57e662c 100644
> > > > --- a/qapi/qapi-dealloc-visitor.c
> > > > +++ b/qapi/qapi-dealloc-visitor.c
> > > > @@ -143,6 +143,12 @@ static void qapi_dealloc_type_enum(Visitor *v, int
> > > > *obj, const char *strings[], {
> > > >  }
> > > >  
> > > > +static void qapi_dealloc_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > > > +                                                const char *name,
> > > > +                                                const int unit, Error
> > > > **errp) +{
> > > > +}
> > > > +
> > > >  Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
> > > >  {
> > > >      return &v->visitor;
> > > > @@ -170,6 +176,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
> > > >      v->visitor.type_str = qapi_dealloc_type_str;
> > > >      v->visitor.type_number = qapi_dealloc_type_number;
> > > >      v->visitor.type_size = qapi_dealloc_type_size;
> > > > +    v->visitor.type_unit_suffixed_int =
> > > > qapi_dealloc_type_unit_suffixed_int; 
> > > >      QTAILQ_INIT(&v->stack);
> > > >  
> > > > diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> > > > index 7a82b63..dcbc1a9 100644
> > > > --- a/qapi/qapi-visit-core.c
> > > > +++ b/qapi/qapi-visit-core.c
> > > > @@ -311,3 +311,16 @@ void input_type_enum(Visitor *v, int *obj, const
> > > > char *strings[], g_free(enum_str);
> > > >      *obj = value;
> > > >  }
> > > > +
> > > > +void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char
> > > > *name,
> > > > +                                  const int unit, Error **errp)
> > > > +{
> > > > +    if (!error_is_set(errp)) {
> > > 
> > > if (error_is_set(errp)) {
> > Thanks, I'll fix it.
> > 
> > > > +        return;
> > > > +    }
> > > > +    if (v->type_unit_suffixed_int) {
> > > > +        v->type_unit_suffixed_int(v, obj, name, unit, errp);
> > > > +    } else {
> > > > +        visit_type_int64(v, obj, name, errp);
> > > > +    }
> > > > +}
> > > > diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
> > > > index 60aceda..04e690a 100644
> > > > --- a/qapi/qapi-visit-core.h
> > > > +++ b/qapi/qapi-visit-core.h
> > > > @@ -62,6 +62,12 @@ struct Visitor
> > > >      void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error
> > > > **errp); /* visit_type_size() falls back to (*type_uint64)() if type_size
> > > > is unset */ void (*type_size)(Visitor *v, uint64_t *obj, const char
> > > > *name, Error **errp);
> > > > +    /*
> > > > +     * visit_unit_suffixed_int() falls back to (*type_int64)()
> > > > +     * if type_unit_suffixed_int is unset
> > > > +    */
> > > 
> > > Indentation is one off.
> > ditto
> > 
> > > 
> > > > +    void (*type_unit_suffixed_int)(Visitor *v, int64_t *obj, const char
> > > > *name,
> > > > +                                   const int unit, Error **errp);
> > > 
> > > Are we expecting differently suffixed ints? Otherwise we could
> > > optionally shorten to type_suffixed_int (but that probably still doesn't
> > > fit within one comment line ;)).
> > Not with current implementation. I'll shorten it as you've suggested.
> > 
> > > 
> > > >  };
> > > >  
> > > >  void visit_start_handle(Visitor *v, void **obj, const char *kind,
> > > > @@ -91,5 +97,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const
> > > > char *name, Error **errp); void visit_type_bool(Visitor *v, bool *obj,
> > > > const char *name, Error **errp); void visit_type_str(Visitor *v, char
> > > > **obj, const char *name, Error **errp); void visit_type_number(Visitor
> > > > *v, double *obj, const char *name, Error **errp); +void
> > > > visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
> > > > +                                  const int unit, Error **errp);
> > > >  
> > > >  #endif
> > > > diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
> > > > index 497eb9a..d2bd154 100644
> > > > --- a/qapi/string-input-visitor.c
> > > > +++ b/qapi/string-input-visitor.c
> > > > @@ -110,6 +110,27 @@ static void parse_start_optional(Visitor *v, bool
> > > > *present, *present = true;
> > > >  }
> > > >  
> > > > +static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
> > > > +                                         const char *name, const int
> > > > unit,
> > > > +                                         Error **errp)
> > > > +{
> > > > +    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
> > > > +    char *endp = (char *) siv->string;
> > > > +    long long val = 0;
> > > > +
> > > > +    if (siv->string) {
> > > > +        val = strtosz_suffix_unit(siv->string, &endp,
> > > > +                             STRTOSZ_DEFSUFFIX_B, unit);
> > > > +    }
> > > > +    if (!siv->string || val == -1 || *endp) {
> > > > +        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
> > > > +              "a value representable as a non-negative int64");
> > > 
> > > Weird indentation remaining, looks as if we could align with errp within
> > > 80 chars.
> > Thanks, I'll fix it.
> > 
> > > 
> > > However, I wonder if "unit" is the (physically etc.) correct term here?
> > > Isn't the "unit" Hz / byte / ... and 1000 more of a conversion factor or
> > > something? At least that's the way I've seen unit used in the API of
> > > another project, passing an enum of Hertz, gram, meter/second, etc.
> > If we are to generalize it to integer than units might not make much sense,
> > they could be anything. Perhaps 'suffix_factor' would be descriptive enough
> > + adding documentation comment to the visitor.
> 
> The real distinction I think is base=2 vs. base=10, but that might cause
> confusion WRT to how the numberical value should be intepreted (10==2
> vs. 10==10), so maybe suffix_base==2|10, or suffix_factor==1000/1024 as you
> suggested. I think I'd prefer the former but either works for me.
I'd prefer suffix_factor to avoid confusion with word 'base' which is commonly
used in conversion routines (i.e. man strtoll).

>
> > 
> > > 
> > > Andreas
> > > 
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    *obj = val;
> > > > +}
> > > > +
> > > >  Visitor *string_input_get_visitor(StringInputVisitor *v)
> > > >  {
> > > >      return &v->visitor;
> > > > @@ -132,6 +153,7 @@ StringInputVisitor *string_input_visitor_new(const
> > > > char *str) v->visitor.type_str = parse_type_str;
> > > >      v->visitor.type_number = parse_type_number;
> > > >      v->visitor.start_optional = parse_start_optional;
> > > > +    v->visitor.type_unit_suffixed_int = parse_type_unit_suffixed_int;
> > > >  
> > > >      v->string = str;
> > > >      return v;
> > > > 
> > > 
> > > 
> > 
>

Patch

diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index 75214e7..57e662c 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -143,6 +143,12 @@  static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char *strings[],
 {
 }
 
+static void qapi_dealloc_type_unit_suffixed_int(Visitor *v, int64_t *obj,
+                                                const char *name,
+                                                const int unit, Error **errp)
+{
+}
+
 Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
 {
     return &v->visitor;
@@ -170,6 +176,7 @@  QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
     v->visitor.type_str = qapi_dealloc_type_str;
     v->visitor.type_number = qapi_dealloc_type_number;
     v->visitor.type_size = qapi_dealloc_type_size;
+    v->visitor.type_unit_suffixed_int = qapi_dealloc_type_unit_suffixed_int;
 
     QTAILQ_INIT(&v->stack);
 
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 7a82b63..dcbc1a9 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -311,3 +311,16 @@  void input_type_enum(Visitor *v, int *obj, const char *strings[],
     g_free(enum_str);
     *obj = value;
 }
+
+void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
+                                  const int unit, Error **errp)
+{
+    if (!error_is_set(errp)) {
+        return;
+    }
+    if (v->type_unit_suffixed_int) {
+        v->type_unit_suffixed_int(v, obj, name, unit, errp);
+    } else {
+        visit_type_int64(v, obj, name, errp);
+    }
+}
diff --git a/qapi/qapi-visit-core.h b/qapi/qapi-visit-core.h
index 60aceda..04e690a 100644
--- a/qapi/qapi-visit-core.h
+++ b/qapi/qapi-visit-core.h
@@ -62,6 +62,12 @@  struct Visitor
     void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
     /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
     void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+    /*
+     * visit_unit_suffixed_int() falls back to (*type_int64)()
+     * if type_unit_suffixed_int is unset
+    */
+    void (*type_unit_suffixed_int)(Visitor *v, int64_t *obj, const char *name,
+                                   const int unit, Error **errp);
 };
 
 void visit_start_handle(Visitor *v, void **obj, const char *kind,
@@ -91,5 +97,7 @@  void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
+void visit_type_unit_suffixed_int(Visitor *v, int64_t *obj, const char *name,
+                                  const int unit, Error **errp);
 
 #endif
diff --git a/qapi/string-input-visitor.c b/qapi/string-input-visitor.c
index 497eb9a..d2bd154 100644
--- a/qapi/string-input-visitor.c
+++ b/qapi/string-input-visitor.c
@@ -110,6 +110,27 @@  static void parse_start_optional(Visitor *v, bool *present,
     *present = true;
 }
 
+static void parse_type_unit_suffixed_int(Visitor *v, int64_t *obj,
+                                         const char *name, const int unit,
+                                         Error **errp)
+{
+    StringInputVisitor *siv = DO_UPCAST(StringInputVisitor, visitor, v);
+    char *endp = (char *) siv->string;
+    long long val = 0;
+
+    if (siv->string) {
+        val = strtosz_suffix_unit(siv->string, &endp,
+                             STRTOSZ_DEFSUFFIX_B, unit);
+    }
+    if (!siv->string || val == -1 || *endp) {
+        error_set(errp, QERR_INVALID_PARAMETER_VALUE, name,
+              "a value representable as a non-negative int64");
+        return;
+    }
+
+    *obj = val;
+}
+
 Visitor *string_input_get_visitor(StringInputVisitor *v)
 {
     return &v->visitor;
@@ -132,6 +153,7 @@  StringInputVisitor *string_input_visitor_new(const char *str)
     v->visitor.type_str = parse_type_str;
     v->visitor.type_number = parse_type_number;
     v->visitor.start_optional = parse_start_optional;
+    v->visitor.type_unit_suffixed_int = parse_type_unit_suffixed_int;
 
     v->string = str;
     return v;