[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] vl: Partial support for non-scalar properti
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] vl: Partial support for non-scalar properties with -object |
Date: |
Thu, 10 Aug 2017 17:02:49 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 |
On 10/08/2017 14:25, Markus Armbruster wrote:
> We've wanted -object to support non-scalar properties for a while.
> Dan Berrange tried in "[PATCH v4 00/10]Provide a QOM-based
> authorization API". Review led to the conclusion that we need to
> replace rather than add to QemuOpts. Initial work towards that goal
> has been merged to provide -blockdev (commit 8746709), but there's
> substantial work left, mostly due to an bewildering array of
> compatibility problems.
>
> Even if a full solution is still out of reach, we can have a partial
> solution now: accept -object argument in JSON syntax. This should
> unblock development work that needs non-scalar properties with -object
>
> The implementation is similar to -blockdev, except we use the new
> infrastructure only for the new JSON case, and stick to QemuOpts for
> the existing KEY=VALUE,... case, to sidestep compatibility problems.
>
> If we did this for more options, we'd have to factor out common code.
> But for one option, this will do.
>
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> qapi-schema.json | 14 +++++++++++---
> vl.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 3 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 802ea53..7ed1db1 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3618,15 +3618,23 @@
> { 'command': 'netdev_del', 'data': {'id': 'str'} }
>
> ##
> -# @object-add:
> +# @ObjectOptions:
> #
> -# Create a QOM object.
> +# Options for creating an object.
> #
> # @qom-type: the class name for the object to be created
> #
> # @id: the name of the new object
> #
> # @props: a dictionary of properties to be passed to the backend
> +##
> +{ 'struct': 'ObjectOptions',
> + 'data': {'qom-type': 'str', 'id': 'str', '*props': 'any'} }
> +
> +##
> +# @object-add:
> +#
> +# Create a QOM object.
> #
> # Returns: Nothing on success
> # Error if @qom-type is not a valid class name
> @@ -3642,7 +3650,7 @@
> #
> ##
> { 'command': 'object-add',
> - 'data': {'qom-type': 'str', 'id': 'str', '*props': 'any'} }
> + 'data': 'ObjectOptions' }
>
> ##
> # @object-del:
> diff --git a/vl.c b/vl.c
> index fd98ed1..db4680b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2854,8 +2854,32 @@ static bool object_create_delayed(const char *type)
> return !object_create_initial(type);
> }
>
> +typedef struct ObjectOptionsQueueEntry {
> + ObjectOptions *oo;
> + Location loc;
> + QSIMPLEQ_ENTRY(ObjectOptionsQueueEntry) entry;
> +} ObjectOptionsQueueEntry;
> +
> +typedef QSIMPLEQ_HEAD(ObjectOptionsQueue, ObjectOptionsQueueEntry)
> + ObjectOptionsQueue;
> +
> +ObjectOptionsQueue oo_queue = QSIMPLEQ_HEAD_INITIALIZER(oo_queue);
> +
> +
> static void object_create(bool (*type_predicate)(const char *))
> {
> + ObjectOptionsQueueEntry *e;
> +
> + QSIMPLEQ_FOREACH(e, &oo_queue, entry) {
> + if (!type_predicate(e->oo->qom_type)) {
> + continue;
> + }
> + loc_push_restore(&e->loc);
> + qmp_object_add(e->oo->qom_type, e->oo->id,
> + e->oo->has_props, e->oo->props, &error_fatal);
> + loc_pop(&e->loc);
> + }
> +
> if (qemu_opts_foreach(qemu_find_opts("object"),
> user_creatable_add_opts_foreach,
> type_predicate, NULL)) {
> @@ -4078,6 +4102,29 @@ int main(int argc, char **argv, char **envp)
> #endif
> break;
> case QEMU_OPTION_object:
> + /*
> + * TODO Use qobject_input_visitor_new_str() instead of
> + * QemuOpts, not in addition to. Not done now because
> + * keyval_parse() isn't wart-compatible with QemuOpts.
> + */
> + if (optarg[0] == '{') {
> + Visitor *v;
> + ObjectOptionsQueueEntry *e;
> +
> + v = qobject_input_visitor_new_str(optarg, "qom-type",
> + &err);
> + if (!v) {
> + error_report_err(err);
> + exit(1);
> + }
> +
> + e = g_new(ObjectOptionsQueueEntry, 1);
> + visit_type_ObjectOptions(v, NULL, &e->oo, &error_fatal);
> + visit_free(v);
> + loc_save(&e->loc);
> + QSIMPLEQ_INSERT_TAIL(&oo_queue, e, entry);
> + break;
> + }
> opts = qemu_opts_parse_noisily(qemu_find_opts("object"),
> optarg, true);
> if (!opts) {
> @@ -4525,6 +4572,14 @@ int main(int argc, char **argv, char **envp)
>
> object_create(object_create_delayed);
>
> + while (!QSIMPLEQ_EMPTY(&oo_queue)) {
> + ObjectOptionsQueueEntry *e = QSIMPLEQ_FIRST(&oo_queue);
> +
> + QSIMPLEQ_REMOVE_HEAD(&oo_queue, entry);
> + qapi_free_ObjectOptions(e->oo);
> + g_free(e);
> + }
Why not free the queue entry in object_create, and assert here that it's
empty?
Paolo
> +
> #ifdef CONFIG_TPM
> if (tpm_init() < 0) {
> exit(1);
>
[Qemu-devel] [PATCH 1/2] vl: Factor object_create() out of main(), Markus Armbruster, 2017/08/10