Re: [PATCH v17 05/14] qapi: Add builtin type time
Tao Xu 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
> ---
>
> 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 = _type_int64;
> ov->visitor.type_uint64 = _type_uint64;
> ov->visitor.type_size = _type_size;
> +ov->visitor.type_time = _type_time;
> ov->visitor.type_bool = _type_bool;
> ov->visitor.type_str= _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)
> +{
> +
[PATCH v17 05/14] qapi: Add builtin type time
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
---
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(+)
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 = _type_int64;
ov->visitor.type_uint64 = _type_uint64;
ov->visitor.type_size = _type_size;
+ov->visitor.type_time = _type_time;
ov->visitor.type_bool = _type_bool;
ov->visitor.type_str= _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 >visitor;
diff --git a/qapi/trace-events b/qapi/trace-events
index
