6aff24c6 | 09-Jun-2016 |
Igor Mammedov <imammedo@redhat.com> |
pc: Parse CPU features only once
Considering that features are converted to global properties and global properties are automatically applied to every new instance of created CPU (at object_new() ti
pc: Parse CPU features only once
Considering that features are converted to global properties and global properties are automatically applied to every new instance of created CPU (at object_new() time), there is no point in parsing cpu_model string every time a CPU created. So move parsing outside CPU creation loop and do it only once.
Parsing also should be done before any CPU is created so that features would affect the first CPU a well.
Signed-off-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
show more ...
|
09f71b05 | 09-Jun-2016 |
Igor Mammedov <imammedo@redhat.com> |
arm: virt: Parse cpu_model only once
Considering that features are converted to global properties and global properties are automatically applied to every new instance of created CPU (at object_new(
arm: virt: Parse cpu_model only once
Considering that features are converted to global properties and global properties are automatically applied to every new instance of created CPU (at object_new() time), there is no point in parsing cpu_model string every time a CPU created. So move parsing outside CPU creation loop and do it only once.
Parsing also should be done before any CPU is created so that features would affect the first CPU a well.
Signed-off-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
show more ...
|
3b098d56 | 09-Jun-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Add new visit_complete() function
Making each output visitor provide its own output collection function was the only remaining reason for exposing visitor sub-types to the rest of the code bas
qapi: Add new visit_complete() function
Making each output visitor provide its own output collection function was the only remaining reason for exposing visitor sub-types to the rest of the code base. Add a polymorphic visit_complete() function which is a no-op for input visitors, and which populates an opaque pointer for output visitors. For maximum type-safety, also add a parameter to the output visitor constructors with a type-correct version of the output pointer, and assert that the two uses match.
This approach was considered superior to either passing the output parameter only during construction (action at a distance during visit_free() feels awkward) or only during visit_complete() (defeating type safety makes it easier to use incorrectly).
Most callers were function-local, and therefore a mechanical conversion; the testsuite was a bit trickier, but the previous cleanup patch minimized the churn here.
The visit_complete() function may be called at most once; doing so lets us use transfer semantics rather than duplication or ref-count semantics to get the just-built output back to the caller, even though it means our behavior is not idempotent.
Generated code is simplified as follows for events:
|@@ -26,7 +26,7 @@ void qapi_event_send_acpi_device_ost(ACP | QDict *qmp; | Error *err = NULL; | QMPEventFuncEmit emit; |- QmpOutputVisitor *qov; |+ QObject *obj; | Visitor *v; | q_obj_ACPI_DEVICE_OST_arg param = { | info |@@ -39,8 +39,7 @@ void qapi_event_send_acpi_device_ost(ACP | | qmp = qmp_event_build_dict("ACPI_DEVICE_OST"); | |- qov = qmp_output_visitor_new(); |- v = qmp_output_get_visitor(qov); |+ v = qmp_output_visitor_new(&obj); | | visit_start_struct(v, "ACPI_DEVICE_OST", NULL, 0, &err); | if (err) { |@@ -55,7 +54,8 @@ void qapi_event_send_acpi_device_ost(ACP | goto out; | } | |- qdict_put_obj(qmp, "data", qmp_output_get_qobject(qov)); |+ visit_complete(v, &obj); |+ qdict_put_obj(qmp, "data", obj); | emit(QAPI_EVENT_ACPI_DEVICE_OST, qmp, &err);
and for commands:
| { | Error *err = NULL; |- QmpOutputVisitor *qov = qmp_output_visitor_new(); | Visitor *v; | |- v = qmp_output_get_visitor(qov); |+ v = qmp_output_visitor_new(ret_out); | visit_type_AddfdInfo(v, "unused", &ret_in, &err); |- if (err) { |- goto out; |+ if (!err) { |+ visit_complete(v, ret_out); | } |- *ret_out = qmp_output_get_qobject(qov); |- |-out: | error_propagate(errp, err);
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1465490926-28625-13-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
1830f22a | 09-Jun-2016 |
Eric Blake <eblake@redhat.com> |
qmp-output-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need qmp_output_visitor_cleanup(); however, we still need to expose the subtype for qmp_
qmp-output-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need qmp_output_visitor_cleanup(); however, we still need to expose the subtype for qmp_output_get_qobject().
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1465490926-28625-10-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
e7ca5656 | 09-Jun-2016 |
Eric Blake <eblake@redhat.com> |
string-output-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need string_output_visitor_cleanup(); however, we still need to expose the subtype fo
string-output-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need string_output_visitor_cleanup(); however, we still need to expose the subtype for string_output_get_string().
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1465490926-28625-9-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
b70ce101 | 09-Jun-2016 |
Eric Blake <eblake@redhat.com> |
qmp-input-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need qmp_input_visitor_cleanup(); which in turn means we no longer need to return a subty
qmp-input-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need qmp_input_visitor_cleanup(); which in turn means we no longer need to return a subtype from qmp_input_visitor_new() nor a public upcast function.
Generated code changes to qmp-marshal.c look like:
|@@ -52,11 +52,10 @@ void qmp_marshal_add_fd(QDict *args, QOb | { | Error *err = NULL; | AddfdInfo *retval; |- QmpInputVisitor *qiv = qmp_input_visitor_new(QOBJECT(args), true); | Visitor *v; | q_obj_add_fd_arg arg = {0}; | |- v = qmp_input_get_visitor(qiv); |+ v = qmp_input_visitor_new(QOBJECT(args), true); | visit_start_struct(v, NULL, NULL, 0, &err); | if (err) { | goto out;
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1465490926-28625-8-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
7a0525c7 | 09-Jun-2016 |
Eric Blake <eblake@redhat.com> |
string-input-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need string_input_visitor_cleanup(); which in turn means we no longer need to return a
string-input-visitor: Favor new visit_free() function
Now that we have a polymorphic visit_free(), we no longer need string_input_visitor_cleanup(); which in turn means we no longer need to return a subtype from string_input_visitor_new() nor a public upcast function.
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1465490926-28625-7-git-send-email-eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
15c2f669 | 28-Apr-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Split visit_end_struct() into pieces
As mentioned in previous patches, we want to call visit_end_struct() functions unconditionally, so that visitors can release resources tied up since the ma
qapi: Split visit_end_struct() into pieces
As mentioned in previous patches, we want to call visit_end_struct() functions unconditionally, so that visitors can release resources tied up since the matching visit_start_struct() without also having to worry about error priority if more than one error occurs.
Even though error_propagate() can be safely used to ignore a second error during cleanup caused by a first error, it is simpler if the cleanup cannot set an error. So, split out the error checking portion (basically, input visitors checking for unvisited keys) into a new function visit_check_struct(), which can be safely skipped if any earlier errors are encountered, and leave the cleanup portion (which never fails, but must be called unconditionally if visit_start_struct() succeeded) in visit_end_struct().
Generated code in qapi-visit.c has diffs resembling:
|@@ -59,10 +59,12 @@ void visit_type_ACPIOSTInfo(Visitor *v, | goto out_obj; | } | visit_type_ACPIOSTInfo_members(v, obj, &err); |- error_propagate(errp, err); |- err = NULL; |+ if (err) { |+ goto out_obj; |+ } |+ visit_check_struct(v, &err); | out_obj: |- visit_end_struct(v, &err); |+ visit_end_struct(v); | out:
and in qapi-event.c:
@@ -47,7 +47,10 @@ void qapi_event_send_acpi_device_ost(ACP | goto out; | } | visit_type_q_obj_ACPI_DEVICE_OST_arg_members(v, ¶m, &err); |- visit_end_struct(v, err ? NULL : &err); |+ if (!err) { |+ visit_check_struct(v, &err); |+ } |+ visit_end_struct(v); | if (err) { | goto out;
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1461879932-9020-20-git-send-email-eblake@redhat.com> [Conflict with a doc fixup resolved] Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
ad739706 | 28-Apr-2016 |
Eric Blake <eblake@redhat.com> |
qom: Wrap prop visit in visit_start_struct
The qmp-input visitor was allowing callers to play rather fast and loose: when visiting a QDict, you could grab members of the root dictionary without firs
qom: Wrap prop visit in visit_start_struct
The qmp-input visitor was allowing callers to play rather fast and loose: when visiting a QDict, you could grab members of the root dictionary without first pushing into the dict; the final such culprit was the QOM code for converting to and from object properties. But we are about to tighten the input visitor, at which point user_creatable_add_type() as called with a QMP input visitor via qmp_object_add() MUST follow the same paradigms as everyone else, of pushing into the struct before grabbing its keys.
The use of 'err ? NULL : &err' is temporary; a later patch will clean that up when it splits visit_end_struct().
Furthermore, note that both callers always pass qdict, so we can convert the conditional into an assert and reduce indentation.
The change has no impact to the testsuite now, but is required to avoid a failure in tests/test-netfilter once qmp-input is made stricter to detect inconsistent 'name' arguments on the root visit.
Since user_creatable_add_type() is also called with OptsVisitor through user_creatable_add_opts(), we must also check that there is no negative impact there; both pre- and post-patch, we see:
$ ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio -object secret,id=sec0,data=letmein,format=raw,foo=bar qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
That is, the only new checking that the new visit_end_struct() can perform is for excess input, but we already catch excess input earlier in object_property_set().
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1461879932-9020-10-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|
240f64b6 | 28-Apr-2016 |
Eric Blake <eblake@redhat.com> |
qapi: Use strict QMP input visitor in more places
The following uses of a QMP input visitor should be strict (that is, excess keys in QDict input should be flagged if not converted to QAPI):
- Test
qapi: Use strict QMP input visitor in more places
The following uses of a QMP input visitor should be strict (that is, excess keys in QDict input should be flagged if not converted to QAPI):
- Testsuite code unrelated to explicitly testing non-strict mode (test-qmp-commands, test-visitor-serialization); since we want more code to be strict by default, having more tests of strict mode doesn't hurt
- Code used for cloning QAPI objects (replay-input.c, qemu-sockets.c); we are reparsing a QObject just barely produced by the qmp output visitor and which therefore should not have any garbage, so while it is extra work to be strict, it validates that our clone is correct [note that a later patch series will simplify these two uses by creating an actual clone visitor that is much more efficient than a generate/reparse cycle]
- qmp_object_add(), which calls into user_creatable_add_type(). Since command line parsing for '-object' uses the same user_creatable_add_type() through the OptsVisitor, and that is always strict, we want to ensure that any nested dictionaries would be treated the same in QMP and from the command line (I don't actually know if such nested dictionaries exist). Note that on this code change, strictness only matters for nested dictionaries (if even possible), since we already flag excess input at the top level during an earlier object_property_set() on an unknown key, whether from QemuOpts:
$ ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio -object secret,id=sec0,data=letmein,format=raw,foo=bar qemu-system-x86_64: -object secret,id=sec0,data=letmein,format=raw,foo=bar: Property '.foo' not found
or from QMP:
$ ./x86_64-softmmu/qemu-system-x86_64 -nographic -nodefaults -qmp stdio {"QMP": {"version": {"qemu": {"micro": 93, "minor": 5, "major": 2}, "package": ""}, "capabilities": []}} {"execute":"qmp_capabilities"} {"return": {}} {"execute":"object-add","arguments":{"qom-type":"secret","id":"sec0","props":{"format":"raw","data":"letmein","foo":"bar"}}} {"error": {"class": "GenericError", "desc": "Property '.foo' not found"}}
The only remaining uses of non-strict input visits are:
- QMP 'qom-set' (which eventually executes object_property_set_qobject()) - mark it as something to revisit in the future (I didn't want to spend any more time on this patch auditing if we have any QOM dictionary properties that might be impacted, and couldn't easily prove whether this code path is shared with anything else).
- test-qmp-input-visitor: explicit tests of non-strict mode. If we later get rid of users that don't need strictness, then this test should be merged with test-qmp-input-strict
Signed-off-by: Eric Blake <eblake@redhat.com> Message-Id: <1461879932-9020-7-git-send-email-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
show more ...
|