[v17,05/14] qapi: Add builtin type time
diff mbox series

Message ID 20191122074826.1373-6-tao3.xu@intel.com
State New
Headers show
Series
  • Build ACPI Heterogeneous Memory Attribute Table (HMAT)
Related show

Commit Message

Tao Xu Nov. 22, 2019, 7:48 a.m. UTC
Add optional builtin type time, fallback is uint64. This type use
qemu_strtotime_ns() for pre-converting time suffix to numbers.

Signed-off-by: Tao Xu <tao3.xu@intel.com>
---

No changes in v17.

Changes in v14:
    - Drop time unit picosecond (Eric)
---
 include/qapi/visitor-impl.h  |  4 ++++
 include/qapi/visitor.h       |  8 ++++++++
 qapi/opts-visitor.c          | 22 ++++++++++++++++++++++
 qapi/qapi-visit-core.c       | 12 ++++++++++++
 qapi/qobject-input-visitor.c | 18 ++++++++++++++++++
 qapi/trace-events            |  1 +
 scripts/qapi/schema.py       |  1 +
 7 files changed, 66 insertions(+)

Comments

Markus Armbruster Nov. 25, 2019, 8:04 a.m. UTC | #1
Tao Xu <tao3.xu@intel.com> writes:

> Add optional builtin type time, fallback is uint64. This type use
> qemu_strtotime_ns() for pre-converting time suffix to numbers.
>
> Signed-off-by: Tao Xu <tao3.xu@intel.com>
> ---
>
> No changes in v17.
>
> Changes in v14:
>     - Drop time unit picosecond (Eric)
> ---
>  include/qapi/visitor-impl.h  |  4 ++++
>  include/qapi/visitor.h       |  8 ++++++++
>  qapi/opts-visitor.c          | 22 ++++++++++++++++++++++
>  qapi/qapi-visit-core.c       | 12 ++++++++++++
>  qapi/qobject-input-visitor.c | 18 ++++++++++++++++++
>  qapi/trace-events            |  1 +
>  scripts/qapi/schema.py       |  1 +
>  7 files changed, 66 insertions(+)

Missing: test coverage for the new built-in type.  I'm not asking for
much.  Suggest to match what's done for 'size' in qapi-schema-test.json.

Possibly missing: string-input-visitor, string-output-visitor implement
type_size().  Both lack test coverage, though.  Should any of them
implement type_time(), too?

>
> diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
> index 8ccb3b6c20..e0979563c7 100644
> --- a/include/qapi/visitor-impl.h
> +++ b/include/qapi/visitor-impl.h
> @@ -88,6 +88,10 @@ struct Visitor
>      void (*type_size)(Visitor *v, const char *name, uint64_t *obj,
>                        Error **errp);
>  
> +    /* Optional; fallback is type_uint64() */
> +    void (*type_time)(Visitor *v, const char *name, uint64_t *obj,
> +                      Error **errp);
> +
>      /* Must be set */
>      void (*type_bool)(Visitor *v, const char *name, bool *obj, Error **errp);
>  
> diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
> index c5b23851a1..22242e706f 100644
> --- a/include/qapi/visitor.h
> +++ b/include/qapi/visitor.h
> @@ -554,6 +554,14 @@ void visit_type_int64(Visitor *v, const char *name, int64_t *obj,
>  void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
>                       Error **errp);
>  
> +/*
> + * Visit a uint64_t value.
> + * Like visit_type_uint64(), except that some visitors may choose to
> + * recognize numbers with timeunit suffix, such as "ns", "us" "ms" and "s".
> + */

Overspecifies what implementations may do.  Let's use
visit_type_size()'s contract here:

   /*
    * Visit a uint64_t value.
    * Like visit_type_uint64(), except that some visitors may choose to
    * recognize additional syntax, such as suffixes for easily scaling
    * values.
    */

> +void visit_type_time(Visitor *v, const char *name, uint64_t *obj,
> +                     Error **errp);
> +
>  /*
>   * Visit a boolean value.
>   *
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 5fe0276c1c..59b575f0fc 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -526,6 +526,27 @@ opts_type_size(Visitor *v, const char *name, uint64_t *obj, Error **errp)
>      processed(ov, name);
>  }
>  
> +static void
> +opts_type_time(Visitor *v, const char *name, uint64_t *obj, Error **errp)
> +{
> +    OptsVisitor *ov = to_ov(v);
> +    const QemuOpt *opt;
> +    int err;
> +
> +    opt = lookup_scalar(ov, name, errp);
> +    if (!opt) {
> +        return;
> +    }
> +
> +    err = qemu_strtotime_ns(opt->str ? opt->str : "", NULL, obj);
> +    if (err < 0) {
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
> +                   "a time value");
> +        return;
> +    }
> +
> +    processed(ov, name);
> +}
>  
>  static void
>  opts_optional(Visitor *v, const char *name, bool *present)
> @@ -573,6 +594,7 @@ opts_visitor_new(const QemuOpts *opts)
>      ov->visitor.type_int64  = &opts_type_int64;
>      ov->visitor.type_uint64 = &opts_type_uint64;
>      ov->visitor.type_size   = &opts_type_size;
> +    ov->visitor.type_time   = &opts_type_time;
>      ov->visitor.type_bool   = &opts_type_bool;
>      ov->visitor.type_str    = &opts_type_str;
>  
> diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
> index 5365561b07..ac8896455c 100644
> --- a/qapi/qapi-visit-core.c
> +++ b/qapi/qapi-visit-core.c
> @@ -277,6 +277,18 @@ void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
>      }
>  }
>  
> +void visit_type_time(Visitor *v, const char *name, uint64_t *obj,
> +                     Error **errp)
> +{
> +    assert(obj);
> +    trace_visit_type_time(v, name, obj);
> +    if (v->type_time) {
> +        v->type_time(v, name, obj, errp);
> +    } else {
> +        v->type_uint64(v, name, obj, errp);
> +    }
> +}
> +
>  void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
>  {
>      assert(obj);
> diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
> index 32236cbcb1..e476fe0d16 100644
> --- a/qapi/qobject-input-visitor.c
> +++ b/qapi/qobject-input-visitor.c
> @@ -627,6 +627,23 @@ static void qobject_input_type_size_keyval(Visitor *v, const char *name,
>      }
>  }
>  
> +static void qobject_input_type_time_keyval(Visitor *v, const char *name,
> +                                           uint64_t *obj, Error **errp)
> +{
> +    QObjectInputVisitor *qiv = to_qiv(v);
> +    const char *str = qobject_input_get_keyval(qiv, name, errp);
> +
> +    if (!str) {
> +        return;
> +    }
> +
> +    if (qemu_strtotime_ns(str, NULL, obj) < 0) {
> +        /* TODO report -ERANGE more nicely */
> +        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> +                   full_name(qiv, name), "time");
> +    }
> +}
> +
>  static void qobject_input_optional(Visitor *v, const char *name, bool *present)
>  {
>      QObjectInputVisitor *qiv = to_qiv(v);
> @@ -708,6 +725,7 @@ Visitor *qobject_input_visitor_new_keyval(QObject *obj)
>      v->visitor.type_any = qobject_input_type_any;
>      v->visitor.type_null = qobject_input_type_null;
>      v->visitor.type_size = qobject_input_type_size_keyval;
> +    v->visitor.type_time = qobject_input_type_time_keyval;
>      v->keyval = true;
>  
>      return &v->visitor;
> diff --git a/qapi/trace-events b/qapi/trace-events
> index 5eb4afa110..c4605a7ccc 100644
> --- a/qapi/trace-events
> +++ b/qapi/trace-events
> @@ -29,6 +29,7 @@ visit_type_int16(void *v, const char *name, int16_t *obj) "v=%p name=%s obj=%p"
>  visit_type_int32(void *v, const char *name, int32_t *obj) "v=%p name=%s obj=%p"
>  visit_type_int64(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
>  visit_type_size(void *v, const char *name, uint64_t *obj) "v=%p name=%s obj=%p"
> +visit_type_time(void *v, const char *name, uint64_t *obj) "v=%p name=%s obj=%p"
>  visit_type_bool(void *v, const char *name, bool *obj) "v=%p name=%s obj=%p"
>  visit_type_str(void *v, const char *name, char **obj) "v=%p name=%s obj=%p"
>  visit_type_number(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index cf0045f34e..210be64909 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -855,6 +855,7 @@ class QAPISchema(object):
>                    ('uint32', 'int',     'uint32_t'),
>                    ('uint64', 'int',     'uint64_t'),
>                    ('size',   'int',     'uint64_t'),
> +                  ('time',   'int',     'uint64_t'),
>                    ('bool',   'boolean', 'bool'),
>                    ('any',    'value',   'QObject' + pointer_suffix),
>                    ('null',   'null',    'QNull' + pointer_suffix)]:

Patch
diff mbox series

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index 8ccb3b6c20..e0979563c7 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -88,6 +88,10 @@  struct Visitor
     void (*type_size)(Visitor *v, const char *name, uint64_t *obj,
                       Error **errp);
 
+    /* Optional; fallback is type_uint64() */
+    void (*type_time)(Visitor *v, const char *name, uint64_t *obj,
+                      Error **errp);
+
     /* Must be set */
     void (*type_bool)(Visitor *v, const char *name, bool *obj, Error **errp);
 
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index c5b23851a1..22242e706f 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -554,6 +554,14 @@  void visit_type_int64(Visitor *v, const char *name, int64_t *obj,
 void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
                      Error **errp);
 
+/*
+ * Visit a uint64_t value.
+ * Like visit_type_uint64(), except that some visitors may choose to
+ * recognize numbers with timeunit suffix, such as "ns", "us" "ms" and "s".
+ */
+void visit_type_time(Visitor *v, const char *name, uint64_t *obj,
+                     Error **errp);
+
 /*
  * Visit a boolean value.
  *
diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
index 5fe0276c1c..59b575f0fc 100644
--- a/qapi/opts-visitor.c
+++ b/qapi/opts-visitor.c
@@ -526,6 +526,27 @@  opts_type_size(Visitor *v, const char *name, uint64_t *obj, Error **errp)
     processed(ov, name);
 }
 
+static void
+opts_type_time(Visitor *v, const char *name, uint64_t *obj, Error **errp)
+{
+    OptsVisitor *ov = to_ov(v);
+    const QemuOpt *opt;
+    int err;
+
+    opt = lookup_scalar(ov, name, errp);
+    if (!opt) {
+        return;
+    }
+
+    err = qemu_strtotime_ns(opt->str ? opt->str : "", NULL, obj);
+    if (err < 0) {
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
+                   "a time value");
+        return;
+    }
+
+    processed(ov, name);
+}
 
 static void
 opts_optional(Visitor *v, const char *name, bool *present)
@@ -573,6 +594,7 @@  opts_visitor_new(const QemuOpts *opts)
     ov->visitor.type_int64  = &opts_type_int64;
     ov->visitor.type_uint64 = &opts_type_uint64;
     ov->visitor.type_size   = &opts_type_size;
+    ov->visitor.type_time   = &opts_type_time;
     ov->visitor.type_bool   = &opts_type_bool;
     ov->visitor.type_str    = &opts_type_str;
 
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 5365561b07..ac8896455c 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -277,6 +277,18 @@  void visit_type_size(Visitor *v, const char *name, uint64_t *obj,
     }
 }
 
+void visit_type_time(Visitor *v, const char *name, uint64_t *obj,
+                     Error **errp)
+{
+    assert(obj);
+    trace_visit_type_time(v, name, obj);
+    if (v->type_time) {
+        v->type_time(v, name, obj, errp);
+    } else {
+        v->type_uint64(v, name, obj, errp);
+    }
+}
+
 void visit_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
 {
     assert(obj);
diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c
index 32236cbcb1..e476fe0d16 100644
--- a/qapi/qobject-input-visitor.c
+++ b/qapi/qobject-input-visitor.c
@@ -627,6 +627,23 @@  static void qobject_input_type_size_keyval(Visitor *v, const char *name,
     }
 }
 
+static void qobject_input_type_time_keyval(Visitor *v, const char *name,
+                                           uint64_t *obj, Error **errp)
+{
+    QObjectInputVisitor *qiv = to_qiv(v);
+    const char *str = qobject_input_get_keyval(qiv, name, errp);
+
+    if (!str) {
+        return;
+    }
+
+    if (qemu_strtotime_ns(str, NULL, obj) < 0) {
+        /* TODO report -ERANGE more nicely */
+        error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                   full_name(qiv, name), "time");
+    }
+}
+
 static void qobject_input_optional(Visitor *v, const char *name, bool *present)
 {
     QObjectInputVisitor *qiv = to_qiv(v);
@@ -708,6 +725,7 @@  Visitor *qobject_input_visitor_new_keyval(QObject *obj)
     v->visitor.type_any = qobject_input_type_any;
     v->visitor.type_null = qobject_input_type_null;
     v->visitor.type_size = qobject_input_type_size_keyval;
+    v->visitor.type_time = qobject_input_type_time_keyval;
     v->keyval = true;
 
     return &v->visitor;
diff --git a/qapi/trace-events b/qapi/trace-events
index 5eb4afa110..c4605a7ccc 100644
--- a/qapi/trace-events
+++ b/qapi/trace-events
@@ -29,6 +29,7 @@  visit_type_int16(void *v, const char *name, int16_t *obj) "v=%p name=%s obj=%p"
 visit_type_int32(void *v, const char *name, int32_t *obj) "v=%p name=%s obj=%p"
 visit_type_int64(void *v, const char *name, int64_t *obj) "v=%p name=%s obj=%p"
 visit_type_size(void *v, const char *name, uint64_t *obj) "v=%p name=%s obj=%p"
+visit_type_time(void *v, const char *name, uint64_t *obj) "v=%p name=%s obj=%p"
 visit_type_bool(void *v, const char *name, bool *obj) "v=%p name=%s obj=%p"
 visit_type_str(void *v, const char *name, char **obj) "v=%p name=%s obj=%p"
 visit_type_number(void *v, const char *name, void *obj) "v=%p name=%s obj=%p"
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cf0045f34e..210be64909 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -855,6 +855,7 @@  class QAPISchema(object):
                   ('uint32', 'int',     'uint32_t'),
                   ('uint64', 'int',     'uint64_t'),
                   ('size',   'int',     'uint64_t'),
+                  ('time',   'int',     'uint64_t'),
                   ('bool',   'boolean', 'bool'),
                   ('any',    'value',   'QObject' + pointer_suffix),
                   ('null',   'null',    'QNull' + pointer_suffix)]: