* [PATCH v2 00/11] vl: compound properties for machines
@ 2021-06-17 15:52 Paolo Bonzini
2021-06-17 15:52 ` [PATCH v2 01/11] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
` (11 more replies)
0 siblings, 12 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:52 UTC (permalink / raw
To: qemu-devel; +Cc: armbru
This series converts -M to keyval parsing, so that they can use
compound properties. The series also converts -smp to a
compound property.
This is also a preparatory work for SGX support, which would like to use
an array-type machine property.
Patches 1-3 introduce the infrastructure and the new functions that
will be used for "-M help" and to use keyval with "merge_lists = true"
semantics.
Patch 4 does the actual switch of -M to keyval and patch 5 removes now
dead code from qemu-option
Patches 6-11 finally validate the concept by implementing -smp as a
compound property; that is "-smp X=Y" is converted to "-machine smp.X=Y"
automatically. Really the only interesting patches are 9 and 11,
while the others are just cleanups.
The series does not support JSON syntax for -M because we need
to consider what happens if string-based dictionaries (produced by -M
key=val) have to be merged with strongly-typed dictionaries (produced
by -M {'key': 123}).
The plan is to only allow exactly one -M option when JSON syntax is in
use; the problem is that this is a forwards-compatibility landmine.
As soon as -smp is converted, for example, -smp becomes a shortcut for -M
and it is now forbidden to use -M '{....}' together with -smp. It would
be impossible to know which options in the future will become incompatible
with -M JSON syntax. Therefore, support for JSON syntax is delayed
until after the main options are converted to QOM compound properties.
These could be -boot, -acpitable, -smbios, -m, -semihosting-config,
-rtc and -fw_cfg.
Paolo
Paolo Bonzini (11):
qom: export more functions for use with non-UserCreatable objects
keyval: introduce keyval_merge
keyval: introduce keyval_parse_into
vl: switch -M parsing to keyval
qemu-option: remove now-dead code
machine: move dies from X86MachineState to CpuTopology
machine: move common smp_parse code to caller
machine: add error propagation to mc->smp_parse
machine: pass QAPI struct to mc->smp_parse
machine: reject -smp dies!=1 for non-PC machines
machine: add smp compound property
hw/core/machine.c | 192 +++++++++++---------
hw/i386/pc.c | 110 +++++-------
hw/i386/x86.c | 15 +-
include/hw/boards.h | 4 +-
include/hw/i386/pc.h | 3 -
include/hw/i386/x86.h | 1 -
include/qemu/option.h | 6 +-
include/qom/object.h | 23 +++
qapi/machine.json | 27 +++
qom/object_interfaces.c | 58 ++++--
softmmu/vl.c | 348 ++++++++++++++++++------------------
tests/qtest/numa-test.c | 22 +--
tests/unit/test-keyval.c | 58 ++++++
tests/unit/test-qemu-opts.c | 35 ----
util/keyval.c | 114 +++++++++++-
util/qemu-option.c | 51 ++----
16 files changed, 621 insertions(+), 446 deletions(-)
--
2.31.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 01/11] qom: export more functions for use with non-UserCreatable objects
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
@ 2021-06-17 15:52 ` Paolo Bonzini
2021-06-17 15:52 ` [PATCH v2 02/11] keyval: introduce keyval_merge Paolo Bonzini
` (10 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:52 UTC (permalink / raw
To: qemu-devel; +Cc: Daniel P . Berrangé, armbru
Machines and accelerators are not user-creatable but they are going
to share similar command-line parsing machinery. Export functions
that will be used with -machine and -accel in softmmu/vl.c.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qom/object.h | 23 ++++++++++++++++
qom/object_interfaces.c | 58 +++++++++++++++++++++++++++++------------
2 files changed, 65 insertions(+), 16 deletions(-)
diff --git a/include/qom/object.h b/include/qom/object.h
index 6721cd312e..faae0d841f 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -861,6 +861,29 @@ static void do_qemu_init_ ## type_array(void) \
} \
type_init(do_qemu_init_ ## type_array)
+/**
+ * type_print_class_properties:
+ * @type: a QOM class name
+ *
+ * Print the object's class properties to stdout or the monitor.
+ * Return whether an object was found.
+ */
+bool type_print_class_properties(const char *type);
+
+/**
+ * object_set_properties_from_keyval:
+ * @obj: a QOM object
+ * @qdict: a dictionary with the properties to be set
+ * @from_json: true if leaf values of @qdict are typed, false if they
+ * are strings
+ * @errp: pointer to error object
+ *
+ * For each key in the dictionary, parse the value string if needed,
+ * then set the corresponding property in @obj.
+ */
+void object_set_properties_from_keyval(Object *obj, const QDict *qdict,
+ bool from_json, Error **errp);
+
/**
* object_class_dynamic_cast_assert:
* @klass: The #ObjectClass to attempt to cast.
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 4479ee693a..ad9b56b59a 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -42,6 +42,44 @@ bool user_creatable_can_be_deleted(UserCreatable *uc)
}
}
+static void object_set_properties_from_qdict(Object *obj, const QDict *qdict,
+ Visitor *v, Error **errp)
+{
+ const QDictEntry *e;
+ Error *local_err = NULL;
+
+ if (!visit_start_struct(v, NULL, NULL, 0, &local_err)) {
+ goto out;
+ }
+ for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
+ if (!object_property_set(obj, e->key, v, &local_err)) {
+ break;
+ }
+ }
+ if (!local_err) {
+ visit_check_struct(v, &local_err);
+ }
+ visit_end_struct(v, NULL);
+
+out:
+ if (local_err) {
+ error_propagate(errp, local_err);
+ }
+}
+
+void object_set_properties_from_keyval(Object *obj, const QDict *qdict,
+ bool from_json, Error **errp)
+{
+ Visitor *v;
+ if (from_json) {
+ v = qobject_input_visitor_new(QOBJECT(qdict));
+ } else {
+ v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+ }
+ object_set_properties_from_qdict(obj, qdict, v, errp);
+ visit_free(v);
+}
+
Object *user_creatable_add_type(const char *type, const char *id,
const QDict *qdict,
Visitor *v, Error **errp)
@@ -49,7 +87,6 @@ Object *user_creatable_add_type(const char *type, const char *id,
ERRP_GUARD();
Object *obj;
ObjectClass *klass;
- const QDictEntry *e;
Error *local_err = NULL;
if (id != NULL && !id_wellformed(id)) {
@@ -78,18 +115,7 @@ Object *user_creatable_add_type(const char *type, const char *id,
assert(qdict);
obj = object_new(type);
- if (!visit_start_struct(v, NULL, NULL, 0, &local_err)) {
- goto out;
- }
- for (e = qdict_first(qdict); e; e = qdict_next(qdict, e)) {
- if (!object_property_set(obj, e->key, v, &local_err)) {
- break;
- }
- }
- if (!local_err) {
- visit_check_struct(v, &local_err);
- }
- visit_end_struct(v, NULL);
+ object_set_properties_from_qdict(obj, qdict, v, &local_err);
if (local_err) {
goto out;
}
@@ -178,7 +204,7 @@ static void user_creatable_print_types(void)
g_slist_free(list);
}
-static bool user_creatable_print_type_properites(const char *type)
+bool type_print_class_properties(const char *type)
{
ObjectClass *klass;
ObjectPropertyIterator iter;
@@ -224,7 +250,7 @@ bool user_creatable_print_help(const char *type, QemuOpts *opts)
}
if (qemu_opt_has_help_opt(opts)) {
- return user_creatable_print_type_properites(type);
+ return type_print_class_properties(type);
}
return false;
@@ -234,7 +260,7 @@ static void user_creatable_print_help_from_qdict(QDict *args)
{
const char *type = qdict_get_try_str(args, "qom-type");
- if (!type || !user_creatable_print_type_properites(type)) {
+ if (!type || !type_print_class_properties(type)) {
user_creatable_print_types();
}
}
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 02/11] keyval: introduce keyval_merge
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
2021-06-17 15:52 ` [PATCH v2 01/11] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
@ 2021-06-17 15:52 ` Paolo Bonzini
2021-06-18 7:42 ` Markus Armbruster
2021-06-17 15:53 ` [PATCH v2 03/11] keyval: introduce keyval_parse_into Paolo Bonzini
` (9 subsequent siblings)
11 siblings, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:52 UTC (permalink / raw
To: qemu-devel; +Cc: armbru
This patch introduces a function that merges two keyval-produced
(or keyval-like) QDicts. It can be used to emulate the behavior of
.merge_lists = true QemuOpts groups, merging -readconfig sections and
command-line options in a single QDict, and also to implement -set.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/option.h | 1 +
tests/unit/test-keyval.c | 58 ++++++++++++++++++++++++++++++++
util/keyval.c | 71 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 130 insertions(+)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index f73e0dc7d9..d89c66145a 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -149,5 +149,6 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
QDict *keyval_parse(const char *params, const char *implied_key,
bool *help, Error **errp);
+void keyval_merge(QDict *old, const QDict *new, Error **errp);
#endif
diff --git a/tests/unit/test-keyval.c b/tests/unit/test-keyval.c
index e20c07cf3e..af0581ae6c 100644
--- a/tests/unit/test-keyval.c
+++ b/tests/unit/test-keyval.c
@@ -747,6 +747,61 @@ static void test_keyval_visit_any(void)
visit_free(v);
}
+static void test_keyval_merge_dict(void)
+{
+ QDict *first = keyval_parse("opt1=abc,opt2.sub1=def,opt2.sub2=ghi,opt3=xyz",
+ NULL, NULL, &error_abort);
+ QDict *second = keyval_parse("opt1=ABC,opt2.sub2=GHI,opt2.sub3=JKL",
+ NULL, NULL, &error_abort);
+ QDict *combined = keyval_parse("opt1=ABC,opt2.sub1=def,opt2.sub2=GHI,opt2.sub3=JKL,opt3=xyz",
+ NULL, NULL, &error_abort);
+ Error *err = NULL;
+
+ keyval_merge(first, second, &err);
+ g_assert(!err);
+ g_assert(qobject_is_equal(QOBJECT(combined), QOBJECT(first)));
+ qobject_unref(first);
+ qobject_unref(second);
+ qobject_unref(combined);
+}
+
+static void test_keyval_merge_list(void)
+{
+ QDict *first = keyval_parse("opt1.0=abc,opt2.0=xyz",
+ NULL, NULL, &error_abort);
+ QDict *second = keyval_parse("opt1.0=def",
+ NULL, NULL, &error_abort);
+ QDict *combined = keyval_parse("opt1.0=abc,opt1.1=def,opt2.0=xyz",
+ NULL, NULL, &error_abort);
+ Error *err = NULL;
+
+ keyval_merge(first, second, &err);
+ g_assert(!err);
+ g_assert(qobject_is_equal(QOBJECT(combined), QOBJECT(first)));
+ qobject_unref(first);
+ qobject_unref(second);
+ qobject_unref(combined);
+}
+
+static void test_keyval_merge_conflict(void)
+{
+ QDict *first = keyval_parse("opt2=ABC",
+ NULL, NULL, &error_abort);
+ QDict *second = keyval_parse("opt2.sub1=def,opt2.sub2=ghi",
+ NULL, NULL, &error_abort);
+ QDict *third = qdict_clone_shallow(first);
+ Error *err = NULL;
+
+ keyval_merge(first, second, &err);
+ error_free_or_abort(&err);
+ keyval_merge(second, third, &err);
+ error_free_or_abort(&err);
+
+ qobject_unref(first);
+ qobject_unref(second);
+ qobject_unref(third);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -760,6 +815,9 @@ int main(int argc, char *argv[])
g_test_add_func("/keyval/visit/optional", test_keyval_visit_optional);
g_test_add_func("/keyval/visit/alternate", test_keyval_visit_alternate);
g_test_add_func("/keyval/visit/any", test_keyval_visit_any);
+ g_test_add_func("/keyval/merge/dict", test_keyval_merge_dict);
+ g_test_add_func("/keyval/merge/list", test_keyval_merge_list);
+ g_test_add_func("/keyval/merge/conflict", test_keyval_merge_conflict);
g_test_run();
return 0;
}
diff --git a/util/keyval.c b/util/keyval.c
index be34928813..8006c5df20 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -310,6 +310,77 @@ static char *reassemble_key(GSList *key)
return g_string_free(s, FALSE);
}
+/*
+ * Recursive worker for keyval_merge. @str is the path that led to the
+ * current dictionary, to be used for error messages. It is modified
+ * internally but restored before the function returns.
+ */
+static void keyval_do_merge(QDict *dest, const QDict *merged, GString *str, Error **errp)
+{
+ size_t save_len = str->len;
+ const QDictEntry *ent;
+ QObject *old_value;
+
+ for (ent = qdict_first(merged); ent; ent = qdict_next(merged, ent)) {
+ old_value = qdict_get(dest, ent->key);
+ if (old_value) {
+ if (qobject_type(old_value) != qobject_type(ent->value)) {
+ error_setg(errp, "Parameter '%s%s' used inconsistently", str->str, ent->key);
+ return;
+ } else if (qobject_type(ent->value) == QTYPE_QDICT) {
+ /* Merge sub-dictionaries. */
+ g_string_append(str, ent->key);
+ g_string_append_c(str, '.');
+ keyval_do_merge(qobject_to(QDict, old_value),
+ qobject_to(QDict, ent->value),
+ str, errp);
+ g_string_truncate(str, save_len);
+ continue;
+ } else if (qobject_type(ent->value) == QTYPE_QLIST) {
+ /* Append to old list. */
+ QList *old = qobject_to(QList, old_value);
+ QList *new = qobject_to(QList, ent->value);
+ const QListEntry *item;
+ QLIST_FOREACH_ENTRY(new, item) {
+ qobject_ref(item->value);
+ qlist_append_obj(old, item->value);
+ }
+ continue;
+ } else {
+ assert(qobject_type(ent->value) == QTYPE_QSTRING);
+ }
+ }
+
+ qobject_ref(ent->value);
+ qdict_put_obj(dest, ent->key, ent->value);
+ }
+}
+
+/* Merge the @merged dictionary into @dest. The dictionaries are expected to be
+ * returned by the keyval parser, and therefore the only expected scalar type
+ * is the * string. In case the same path is present in both @dest and @merged,
+ * the semantics are as follows:
+ *
+ * - lists are concatenated
+ *
+ * - dictionaries are merged recursively
+ *
+ * - for scalar values, @merged wins
+ *
+ * In case an error is reported, @dest may already have been modified.
+ *
+ * This function can be used to implement semantics analogous to QemuOpts's
+ * .merge_lists = true case, or to implement -set for options backed by QDicts.
+ */
+void keyval_merge(QDict *dest, const QDict *merged, Error **errp)
+{
+ GString *str;
+
+ str = g_string_new("");
+ keyval_do_merge(dest, merged, str, errp);
+ g_string_free(str, TRUE);
+}
+
/*
* Listify @cur recursively.
* Replace QDicts whose keys are all valid list indexes by QLists.
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 03/11] keyval: introduce keyval_parse_into
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
2021-06-17 15:52 ` [PATCH v2 01/11] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
2021-06-17 15:52 ` [PATCH v2 02/11] keyval: introduce keyval_merge Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 04/11] vl: switch -M parsing to keyval Paolo Bonzini
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: Daniel P . Berrangé, armbru
Allow parsing multiple keyval sequences into the same dictionary.
This will be used to simplify the parsing of the -M command line
option, which is currently a .merge_lists = true QemuOpts group.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/option.h | 2 ++
util/keyval.c | 43 +++++++++++++++++++++++++++++++++++--------
2 files changed, 37 insertions(+), 8 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index d89c66145a..fffb03d848 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -147,6 +147,8 @@ void qemu_opts_print_help(QemuOptsList *list, bool print_caption);
void qemu_opts_free(QemuOptsList *list);
QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
+ bool *p_help, Error **errp);
QDict *keyval_parse(const char *params, const char *implied_key,
bool *help, Error **errp);
void keyval_merge(QDict *old, const QDict *new, Error **errp);
diff --git a/util/keyval.c b/util/keyval.c
index 8006c5df20..a6cf1d9606 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -502,13 +502,14 @@ static QObject *keyval_listify(QDict *cur, GSList *key_of_cur, Error **errp)
* If @p_help is not NULL, store whether help is requested there.
* If @p_help is NULL and help is requested, fail.
*
- * On success, return a dictionary of the parsed keys and values.
- * On failure, store an error through @errp and return NULL.
+ * On success, return @dict, now filled with the parsed keys and values.
+ *
+ * On failure, store an error through @errp and return NULL. Any keys
+ * and values parsed so far will be in @dict nevertheless.
*/
-QDict *keyval_parse(const char *params, const char *implied_key,
- bool *p_help, Error **errp)
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
+ bool *p_help, Error **errp)
{
- QDict *qdict = qdict_new();
QObject *listified;
const char *s;
bool help = false;
@@ -517,7 +518,6 @@ QDict *keyval_parse(const char *params, const char *implied_key,
while (*s) {
s = keyval_parse_one(qdict, s, implied_key, &help, errp);
if (!s) {
- qobject_unref(qdict);
return NULL;
}
implied_key = NULL;
@@ -527,15 +527,42 @@ QDict *keyval_parse(const char *params, const char *implied_key,
*p_help = help;
} else if (help) {
error_setg(errp, "Help is not available for this option");
- qobject_unref(qdict);
return NULL;
}
listified = keyval_listify(qdict, NULL, errp);
if (!listified) {
- qobject_unref(qdict);
return NULL;
}
assert(listified == QOBJECT(qdict));
return qdict;
}
+
+/*
+ * Parse @params in QEMU's traditional KEY=VALUE,... syntax.
+ *
+ * If @implied_key, the first KEY= can be omitted. @implied_key is
+ * implied then, and VALUE can't be empty or contain ',' or '='.
+ *
+ * A parameter "help" or "?" without a value isn't added to the
+ * resulting dictionary, but instead is interpreted as help request.
+ * All other options are parsed and returned normally so that context
+ * specific help can be printed.
+ *
+ * If @p_help is not NULL, store whether help is requested there.
+ * If @p_help is NULL and help is requested, fail.
+ *
+ * On success, return a dictionary of the parsed keys and values.
+ * On failure, store an error through @errp and return NULL.
+ */
+QDict *keyval_parse(const char *params, const char *implied_key,
+ bool *p_help, Error **errp)
+{
+ QDict *qdict = qdict_new();
+ QDict *ret = keyval_parse_into(qdict, params, implied_key, p_help, errp);
+
+ if (!ret) {
+ qobject_unref(qdict);
+ }
+ return ret;
+}
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 04/11] vl: switch -M parsing to keyval
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (2 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 03/11] keyval: introduce keyval_parse_into Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 05/11] qemu-option: remove now-dead code Paolo Bonzini
` (7 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: armbru
Switch from QemuOpts to keyval. This enables the introduction
of non-scalar machine properties, and JSON syntax in the future.
For JSON syntax to be supported right now, we would have to
consider what would happen if string-based dictionaries (produced by
-M key=val) were to be merged with strongly-typed dictionaries
(produced by -M {'key': 123}).
The simplest way out is to never enter the situation, and only allow one
-M option when JSON syntax is in use. However, we want options such as
-smp to become syntactic sugar for -M, and this is a problem; as soon
as -smp becomes a shortcut for -M, QEMU would forbid using -M '{....}'
together with -smp. Therefore, allowing JSON syntax right now for -M
would be a forward-compatibility nightmare and it would be impossible
anyway to introduce -M incrementally in tools.
Instead, support for JSON syntax is delayed until after the main
options are converted to QOM compound properties. These include -boot,
-acpitable, -smbios, -m, -semihosting-config, -rtc and -fw_cfg. Once JSON
syntax is introduced, these options will _also_ be forbidden together
with -M '{...}'.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
softmmu/vl.c | 315 ++++++++++++++++++++++++---------------------------
1 file changed, 146 insertions(+), 169 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index feb4d201f3..01bcb16560 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -145,6 +145,8 @@ static const char *cpu_option;
static const char *mem_path;
static const char *incoming;
static const char *loadvm;
+static const char *accelerators;
+static QDict *machine_opts_dict;
static QTAILQ_HEAD(, ObjectOption) object_opts = QTAILQ_HEAD_INITIALIZER(object_opts);
static ram_addr_t maxram_size;
static uint64_t ram_slots;
@@ -235,21 +237,6 @@ static QemuOptsList qemu_option_rom_opts = {
},
};
-static QemuOptsList qemu_machine_opts = {
- .name = "machine",
- .implied_opt_name = "type",
- .merge_lists = true,
- .head = QTAILQ_HEAD_INITIALIZER(qemu_machine_opts.head),
- .desc = {
- /*
- * no elements => accept any
- * sanity checking will happen later
- * when setting machine properties
- */
- { }
- },
-};
-
static QemuOptsList qemu_accel_opts = {
.name = "accel",
.implied_opt_name = "accel",
@@ -498,16 +485,6 @@ static QemuOptsList qemu_action_opts = {
},
};
-/**
- * Get machine options
- *
- * Returns: machine options (never null).
- */
-static QemuOpts *qemu_get_machine_opts(void)
-{
- return qemu_find_opts_singleton("machine");
-}
-
const char *qemu_get_vm_name(void)
{
return qemu_name;
@@ -815,33 +792,6 @@ static MachineClass *find_default_machine(GSList *machines)
return default_machineclass;
}
-static int machine_help_func(QemuOpts *opts, MachineState *machine)
-{
- ObjectProperty *prop;
- ObjectPropertyIterator iter;
-
- if (!qemu_opt_has_help_opt(opts)) {
- return 0;
- }
-
- object_property_iter_init(&iter, OBJECT(machine));
- while ((prop = object_property_iter_next(&iter))) {
- if (!prop->set) {
- continue;
- }
-
- printf("%s.%s=%s", MACHINE_GET_CLASS(machine)->name,
- prop->name, prop->type);
- if (prop->description) {
- printf(" (%s)\n", prop->description);
- } else {
- printf("\n");
- }
- }
-
- return 1;
-}
-
static void version(void)
{
printf("QEMU emulator version " QEMU_FULL_VERSION "\n"
@@ -1546,33 +1496,31 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
object_class_get_name(OBJECT_CLASS(mc1)));
}
-static MachineClass *machine_parse(const char *name, GSList *machines)
+static void machine_help_func(const QDict *qdict)
{
- MachineClass *mc;
- GSList *el;
+ GSList *machines, *el;
+ const char *type = qdict_get_try_str(qdict, "type");
- if (is_help_option(name)) {
- printf("Supported machines are:\n");
- machines = g_slist_sort(machines, machine_class_cmp);
- for (el = machines; el; el = el->next) {
- MachineClass *mc = el->data;
- if (mc->alias) {
- printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
- }
- printf("%-20s %s%s%s\n", mc->name, mc->desc,
- mc->is_default ? " (default)" : "",
- mc->deprecation_reason ? " (deprecated)" : "");
+ machines = object_class_get_list(TYPE_MACHINE, false);
+ if (type) {
+ ObjectClass *machine_class = OBJECT_CLASS(find_machine(type, machines));
+ if (machine_class) {
+ type_print_class_properties(object_class_get_name(machine_class));
+ return;
}
- exit(0);
}
- mc = find_machine(name, machines);
- if (!mc) {
- error_report("unsupported machine type");
- error_printf("Use -machine help to list supported machines\n");
- exit(1);
+ printf("Supported machines are:\n");
+ machines = g_slist_sort(machines, machine_class_cmp);
+ for (el = machines; el; el = el->next) {
+ MachineClass *mc = el->data;
+ if (mc->alias) {
+ printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
+ }
+ printf("%-20s %s%s%s\n", mc->name, mc->desc,
+ mc->is_default ? " (default)" : "",
+ mc->deprecation_reason ? " (deprecated)" : "");
}
- return mc;
}
static const char *pid_file;
@@ -1625,32 +1573,31 @@ static const QEMUOption *lookup_opt(int argc, char **argv,
return popt;
}
-static MachineClass *select_machine(void)
+static MachineClass *select_machine(QDict *qdict, Error **errp)
{
+ const char *optarg = qdict_get_try_str(qdict, "type");
GSList *machines = object_class_get_list(TYPE_MACHINE, false);
- MachineClass *machine_class = find_default_machine(machines);
- const char *optarg;
- QemuOpts *opts;
- Location loc;
+ MachineClass *machine_class;
+ Error *local_err = NULL;
- loc_push_none(&loc);
-
- opts = qemu_get_machine_opts();
- qemu_opts_loc_restore(opts);
-
- optarg = qemu_opt_get(opts, "type");
if (optarg) {
- machine_class = machine_parse(optarg, machines);
+ machine_class = find_machine(optarg, machines);
+ qdict_del(qdict, "type");
+ if (!machine_class) {
+ error_setg(&local_err, "unsupported machine type");
+ }
+ } else {
+ machine_class = find_default_machine(machines);
+ if (!machine_class) {
+ error_setg(&local_err, "No machine specified, and there is no default");
+ }
}
- if (!machine_class) {
- error_report("No machine specified, and there is no default");
- error_printf("Use -machine help to list supported machines\n");
- exit(1);
- }
-
- loc_pop(&loc);
g_slist_free(machines);
+ if (local_err) {
+ error_append_hint(&local_err, "Use -machine help to list supported machines\n");
+ error_propagate(errp, local_err);
+ }
return machine_class;
}
@@ -1669,42 +1616,70 @@ static int object_parse_property_opt(Object *obj,
return 0;
}
-static int machine_set_property(void *opaque,
- const char *name, const char *value,
- Error **errp)
+/* *Non*recursively replace underscores with dashes in QDict keys. */
+static void keyval_dashify(QDict *qdict, Error **errp)
{
- g_autofree char *qom_name = g_strdup(name);
+ const QDictEntry *ent, *next;
char *p;
- for (p = qom_name; *p; p++) {
- if (*p == '_') {
- *p = '-';
+ for (ent = qdict_first(qdict); ent; ent = next) {
+ g_autofree char *new_key = NULL;
+
+ next = qdict_next(qdict, ent);
+ if (!strchr(ent->key, '_')) {
+ continue;
}
+ new_key = g_strdup(ent->key);
+ for (p = new_key; *p; p++) {
+ if (*p == '_') {
+ *p = '-';
+ }
+ }
+ if (qdict_haskey(qdict, new_key)) {
+ error_setg(errp, "Conflict between '%s' and '%s'", ent->key, new_key);
+ return;
+ }
+ qobject_ref(ent->value);
+ qdict_put_obj(qdict, new_key, ent->value);
+ qdict_del(qdict, ent->key);
}
+}
+
+static void qemu_apply_legacy_machine_options(QDict *qdict)
+{
+ const char *value;
+
+ keyval_dashify(qdict, &error_fatal);
/* Legacy options do not correspond to MachineState properties. */
- if (g_str_equal(qom_name, "accel")) {
- return 0;
- }
- if (g_str_equal(qom_name, "igd-passthru")) {
- object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value,
- false);
- return 0;
- }
- if (g_str_equal(qom_name, "kvm-shadow-mem")) {
- object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value,
- false);
- return 0;
- }
- if (g_str_equal(qom_name, "kernel-irqchip")) {
- object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value,
- false);
- object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), qom_name, value,
- false);
- return 0;
+ value = qdict_get_try_str(qdict, "accel");
+ if (value) {
+ accelerators = g_strdup(value);
+ qdict_del(qdict, "accel");
}
- return object_parse_property_opt(opaque, name, value, "type", errp);
+ value = qdict_get_try_str(qdict, "igd-passthru");
+ if (value) {
+ object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", value,
+ false);
+ qdict_del(qdict, "igd-passthru");
+ }
+
+ value = qdict_get_try_str(qdict, "kvm-shadow-mem");
+ if (value) {
+ object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kvm-shadow-mem", value,
+ false);
+ qdict_del(qdict, "kvm-shadow-mem");
+ }
+
+ value = qdict_get_try_str(qdict, "kernel-irqchip");
+ if (value) {
+ object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kernel-irqchip", value,
+ false);
+ object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), "kernel-irqchip", value,
+ false);
+ qdict_del(qdict, "kernel-irqchip");
+ }
}
static void object_option_foreach_add(bool (*type_opt_predicate)(const char *))
@@ -1819,16 +1794,14 @@ static bool object_create_early(const char *type)
return true;
}
-static void qemu_apply_machine_options(void)
+static void qemu_apply_machine_options(QDict *qdict)
{
MachineClass *machine_class = MACHINE_GET_CLASS(current_machine);
- QemuOpts *machine_opts = qemu_get_machine_opts();
const char *boot_order = NULL;
const char *boot_once = NULL;
QemuOpts *opts;
- qemu_opt_foreach(machine_opts, machine_set_property, current_machine,
- &error_fatal);
+ object_set_properties_from_keyval(OBJECT(current_machine), qdict, false, &error_fatal);
current_machine->ram_size = ram_size;
current_machine->maxram_size = maxram_size;
current_machine->ram_slots = ram_slots;
@@ -1857,10 +1830,8 @@ static void qemu_apply_machine_options(void)
current_machine->boot_once = boot_once;
if (semihosting_enabled() && !semihosting_get_argc()) {
- const char *kernel_filename = qemu_opt_get(machine_opts, "kernel");
- const char *kernel_cmdline = qemu_opt_get(machine_opts, "append") ?: "";
/* fall back to the -kernel/-append */
- semihosting_arg_fallback(kernel_filename, kernel_cmdline);
+ semihosting_arg_fallback(current_machine->kernel_filename, current_machine->kernel_cmdline);
}
}
@@ -1907,8 +1878,7 @@ static void qemu_create_early_backends(void)
/*
* Note: we need to create audio and block backends before
- * machine_set_property(), so machine properties can refer to
- * them.
+ * setting machine properties, so they can be referred to.
*/
configure_blockdev(&bdo_queue, machine_class, snapshot);
audio_init_audiodevs();
@@ -2074,16 +2044,14 @@ static void set_memory_options(MachineClass *mc)
loc_pop(&loc);
}
-static void qemu_create_machine(MachineClass *machine_class)
+static void qemu_create_machine(QDict *qdict)
{
+ MachineClass *machine_class = select_machine(qdict, &error_fatal);
object_set_machine_compat_props(machine_class->compat_props);
set_memory_options(machine_class);
current_machine = MACHINE(object_new_with_class(OBJECT_CLASS(machine_class)));
- if (machine_help_func(qemu_get_machine_opts(), current_machine)) {
- exit(0);
- }
object_property_add_child(object_get_root(), "machine",
OBJECT(current_machine));
object_property_add_child(container_get(OBJECT(current_machine),
@@ -2114,8 +2082,12 @@ static void qemu_create_machine(MachineClass *machine_class)
* specified either by the configuration file or by the command line.
*/
if (machine_class->default_machine_opts) {
- qemu_opts_set_defaults(qemu_find_opts("machine"),
- machine_class->default_machine_opts, 0);
+ QDict *default_opts =
+ keyval_parse(machine_class->default_machine_opts, NULL, NULL,
+ &error_abort);
+ object_set_properties_from_keyval(OBJECT(current_machine), default_opts,
+ false, &error_abort);
+ qobject_unref(default_opts);
}
}
@@ -2137,7 +2109,8 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
*/
static bool is_qemuopts_group(const char *group)
{
- if (g_str_equal(group, "object")) {
+ if (g_str_equal(group, "object") ||
+ g_str_equal(group, "machine")) {
return false;
}
return true;
@@ -2150,6 +2123,13 @@ static void qemu_record_config_group(const char *group, QDict *dict,
Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(dict));
object_option_add_visitor(v);
visit_free(v);
+ } else if (g_str_equal(group, "machine")) {
+ /*
+ * Cannot merge string-valued and type-safe dictionaries, so JSON
+ * is not accepted yet for -M.
+ */
+ assert(!from_json);
+ keyval_merge(machine_opts_dict, dict, errp);
} else {
abort();
}
@@ -2280,13 +2260,11 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
static void configure_accelerators(const char *progname)
{
- const char *accelerators;
bool init_failed = false;
qemu_opts_foreach(qemu_find_opts("icount"),
do_configure_icount, NULL, &error_fatal);
- accelerators = qemu_opt_get(qemu_get_machine_opts(), "accel");
if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
char **accel_list, **tmp;
@@ -2374,12 +2352,11 @@ static void create_default_memdev(MachineState *ms, const char *path)
&error_fatal);
}
-static void qemu_validate_options(void)
+static void qemu_validate_options(const QDict *machine_opts)
{
- QemuOpts *machine_opts = qemu_get_machine_opts();
- const char *kernel_filename = qemu_opt_get(machine_opts, "kernel");
- const char *initrd_filename = qemu_opt_get(machine_opts, "initrd");
- const char *kernel_cmdline = qemu_opt_get(machine_opts, "append");
+ const char *kernel_filename = qdict_get_try_str(machine_opts, "kernel");
+ const char *initrd_filename = qdict_get_try_str(machine_opts, "initrd");
+ const char *kernel_cmdline = qdict_get_try_str(machine_opts, "append");
if (kernel_filename == NULL) {
if (kernel_cmdline != NULL) {
@@ -2719,7 +2696,6 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_add_opts(&qemu_trace_opts);
qemu_plugin_add_opts();
qemu_add_opts(&qemu_option_rom_opts);
- qemu_add_opts(&qemu_machine_opts);
qemu_add_opts(&qemu_accel_opts);
qemu_add_opts(&qemu_mem_opts);
qemu_add_opts(&qemu_smp_opts);
@@ -2760,6 +2736,7 @@ void qemu_init(int argc, char **argv, char **envp)
}
}
+ machine_opts_dict = qdict_new();
if (userconfig) {
qemu_read_default_config_file(&error_fatal);
}
@@ -2849,8 +2826,7 @@ void qemu_init(int argc, char **argv, char **envp)
parse_display(optarg);
break;
case QEMU_OPTION_nographic:
- olist = qemu_find_opts("machine");
- qemu_opts_parse_noisily(olist, "graphics=off", false);
+ qdict_put_str(machine_opts_dict, "graphics", "off");
nographic = true;
dpy.type = DISPLAY_TYPE_NONE;
break;
@@ -2874,16 +2850,16 @@ void qemu_init(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_kernel:
- qemu_opts_set(qemu_find_opts("machine"), "kernel", optarg, &error_abort);
+ qdict_put_str(machine_opts_dict, "kernel", optarg);
break;
case QEMU_OPTION_initrd:
- qemu_opts_set(qemu_find_opts("machine"), "initrd", optarg, &error_abort);
+ qdict_put_str(machine_opts_dict, "initrd", optarg);
break;
case QEMU_OPTION_append:
- qemu_opts_set(qemu_find_opts("machine"), "append", optarg, &error_abort);
+ qdict_put_str(machine_opts_dict, "append", optarg);
break;
case QEMU_OPTION_dtb:
- qemu_opts_set(qemu_find_opts("machine"), "dtb", optarg, &error_abort);
+ qdict_put_str(machine_opts_dict, "dtb", optarg);
break;
case QEMU_OPTION_cdrom:
drive_add(IF_DEFAULT, 2, optarg, CDROM_OPTS);
@@ -2993,7 +2969,7 @@ void qemu_init(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_bios:
- qemu_opts_set(qemu_find_opts("machine"), "firmware", optarg, &error_abort);
+ qdict_put_str(machine_opts_dict, "firmware", optarg);
break;
case QEMU_OPTION_singlestep:
singlestep = 1;
@@ -3262,17 +3238,20 @@ void qemu_init(int argc, char **argv, char **envp)
preconfig_requested = true;
break;
case QEMU_OPTION_enable_kvm:
- olist = qemu_find_opts("machine");
- qemu_opts_parse_noisily(olist, "accel=kvm", false);
+ qdict_put_str(machine_opts_dict, "accel", "kvm");
break;
case QEMU_OPTION_M:
case QEMU_OPTION_machine:
- olist = qemu_find_opts("machine");
- opts = qemu_opts_parse_noisily(olist, optarg, true);
- if (!opts) {
- exit(1);
+ {
+ bool help;
+
+ keyval_parse_into(machine_opts_dict, optarg, "type", &help, &error_fatal);
+ if (help) {
+ machine_help_func(machine_opts_dict);
+ exit(EXIT_SUCCESS);
+ }
+ break;
}
- break;
case QEMU_OPTION_accel:
accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"),
optarg, true);
@@ -3299,12 +3278,10 @@ void qemu_init(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_usb:
- olist = qemu_find_opts("machine");
- qemu_opts_parse_noisily(olist, "usb=on", false);
+ qdict_put_str(machine_opts_dict, "usb", "on");
break;
case QEMU_OPTION_usbdevice:
- olist = qemu_find_opts("machine");
- qemu_opts_parse_noisily(olist, "usb=on", false);
+ qdict_put_str(machine_opts_dict, "usb", "on");
add_device_config(DEV_USB, optarg);
break;
case QEMU_OPTION_device:
@@ -3323,12 +3300,10 @@ void qemu_init(int argc, char **argv, char **envp)
vnc_parse(optarg);
break;
case QEMU_OPTION_no_acpi:
- olist = qemu_find_opts("machine");
- qemu_opts_parse_noisily(olist, "acpi=off", false);
+ qdict_put_str(machine_opts_dict, "acpi", "off");
break;
case QEMU_OPTION_no_hpet:
- olist = qemu_find_opts("machine");
- qemu_opts_parse_noisily(olist, "hpet=off", false);
+ qdict_put_str(machine_opts_dict, "hpet", "off");
break;
case QEMU_OPTION_no_reboot:
olist = qemu_find_opts("action");
@@ -3581,7 +3556,7 @@ void qemu_init(int argc, char **argv, char **envp)
*/
loc_set_none();
- qemu_validate_options();
+ qemu_validate_options(machine_opts_dict);
qemu_process_sugar_options();
/*
@@ -3614,7 +3589,7 @@ void qemu_init(int argc, char **argv, char **envp)
configure_rtc(qemu_find_opts_singleton("rtc"));
- qemu_create_machine(select_machine());
+ qemu_create_machine(machine_opts_dict);
suspend_mux_open();
@@ -3622,12 +3597,14 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_create_default_devices();
qemu_create_early_backends();
- qemu_apply_machine_options();
+ qemu_apply_legacy_machine_options(machine_opts_dict);
+ qemu_apply_machine_options(machine_opts_dict);
+ qobject_unref(machine_opts_dict);
phase_advance(PHASE_MACHINE_CREATED);
/*
* Note: uses machine properties such as kernel-irqchip, must run
- * after machine_set_property().
+ * after qemu_apply_machine_options.
*/
configure_accelerators(argv[0]);
phase_advance(PHASE_ACCEL_CREATED);
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 05/11] qemu-option: remove now-dead code
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (3 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 04/11] vl: switch -M parsing to keyval Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 06/11] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: armbru
-M was the sole user of qemu_opts_set and qemu_opts_set_defaults,
remove them and the arguments that they used.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/option.h | 3 ---
tests/unit/test-qemu-opts.c | 35 -------------------------
util/qemu-option.c | 51 ++++++++-----------------------------
3 files changed, 10 insertions(+), 79 deletions(-)
diff --git a/include/qemu/option.h b/include/qemu/option.h
index fffb03d848..306bf07575 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -119,7 +119,6 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
int fail_if_exists, Error **errp);
void qemu_opts_reset(QemuOptsList *list);
void qemu_opts_loc_restore(QemuOpts *opts);
-bool qemu_opts_set(QemuOptsList *list, const char *name, const char *value, Error **errp);
const char *qemu_opts_id(QemuOpts *opts);
void qemu_opts_set_id(QemuOpts *opts, char *id);
void qemu_opts_del(QemuOpts *opts);
@@ -130,8 +129,6 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
bool permit_abbrev);
QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp);
-void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
- int permit_abbrev);
QemuOpts *qemu_opts_from_qdict(QemuOptsList *list, const QDict *qdict,
Error **errp);
QDict *qemu_opts_to_qdict_filtered(QemuOpts *opts, QDict *qdict,
diff --git a/tests/unit/test-qemu-opts.c b/tests/unit/test-qemu-opts.c
index 6568e31a72..828d40e928 100644
--- a/tests/unit/test-qemu-opts.c
+++ b/tests/unit/test-qemu-opts.c
@@ -410,40 +410,6 @@ static void test_qemu_opts_reset(void)
g_assert(opts == NULL);
}
-static void test_qemu_opts_set(void)
-{
- QemuOptsList *list;
- QemuOpts *opts;
- const char *opt;
-
- list = qemu_find_opts("opts_list_04");
- g_assert(list != NULL);
- g_assert(QTAILQ_EMPTY(&list->head));
- g_assert_cmpstr(list->name, ==, "opts_list_04");
-
- /* should not find anything at this point */
- opts = qemu_opts_find(list, NULL);
- g_assert(opts == NULL);
-
- /* implicitly create opts and set str3 value */
- qemu_opts_set(list, "str3", "value", &error_abort);
- g_assert(!QTAILQ_EMPTY(&list->head));
-
- /* get the just created opts */
- opts = qemu_opts_find(list, NULL);
- g_assert(opts != NULL);
-
- /* check the str3 value */
- opt = qemu_opt_get(opts, "str3");
- g_assert_cmpstr(opt, ==, "value");
-
- qemu_opts_del(opts);
-
- /* should not find anything at this point */
- opts = qemu_opts_find(list, NULL);
- g_assert(opts == NULL);
-}
-
static int opts_count_iter(void *opaque, const char *name, const char *value,
Error **errp)
{
@@ -1041,7 +1007,6 @@ int main(int argc, char *argv[])
g_test_add_func("/qemu-opts/opt_get_size", test_qemu_opt_get_size);
g_test_add_func("/qemu-opts/opt_unset", test_qemu_opt_unset);
g_test_add_func("/qemu-opts/opts_reset", test_qemu_opts_reset);
- g_test_add_func("/qemu-opts/opts_set", test_qemu_opts_set);
g_test_add_func("/qemu-opts/opts_parse/general", test_opts_parse);
g_test_add_func("/qemu-opts/opts_parse/bool", test_opts_parse_bool);
g_test_add_func("/qemu-opts/opts_parse/number", test_opts_parse_number);
diff --git a/util/qemu-option.c b/util/qemu-option.c
index 4944015a25..ee78e42216 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -479,19 +479,14 @@ int qemu_opt_unset(QemuOpts *opts, const char *name)
}
}
-static QemuOpt *opt_create(QemuOpts *opts, const char *name, char *value,
- bool prepend)
+static QemuOpt *opt_create(QemuOpts *opts, const char *name, char *value)
{
QemuOpt *opt = g_malloc0(sizeof(*opt));
opt->name = g_strdup(name);
opt->str = value;
opt->opts = opts;
- if (prepend) {
- QTAILQ_INSERT_HEAD(&opts->head, opt, next);
- } else {
- QTAILQ_INSERT_TAIL(&opts->head, opt, next);
- }
+ QTAILQ_INSERT_TAIL(&opts->head, opt, next);
return opt;
}
@@ -518,7 +513,7 @@ static bool opt_validate(QemuOpt *opt, Error **errp)
bool qemu_opt_set(QemuOpts *opts, const char *name, const char *value,
Error **errp)
{
- QemuOpt *opt = opt_create(opts, name, g_strdup(value), false);
+ QemuOpt *opt = opt_create(opts, name, g_strdup(value));
if (!opt_validate(opt, errp)) {
qemu_opt_del(opt);
@@ -662,15 +657,6 @@ void qemu_opts_loc_restore(QemuOpts *opts)
loc_restore(&opts->loc);
}
-bool qemu_opts_set(QemuOptsList *list, const char *name, const char *value, Error **errp)
-{
- QemuOpts *opts;
-
- assert(list->merge_lists);
- opts = qemu_opts_create(list, NULL, 0, &error_abort);
- return qemu_opt_set(opts, name, value, errp);
-}
-
const char *qemu_opts_id(QemuOpts *opts)
{
return opts->id;
@@ -811,7 +797,7 @@ static const char *get_opt_name_value(const char *params,
}
static bool opts_do_parse(QemuOpts *opts, const char *params,
- const char *firstname, bool prepend,
+ const char *firstname,
bool warn_on_flag, bool *help_wanted, Error **errp)
{
char *option, *value;
@@ -833,7 +819,7 @@ static bool opts_do_parse(QemuOpts *opts, const char *params,
continue;
}
- opt = opt_create(opts, option, value, prepend);
+ opt = opt_create(opts, option, value);
g_free(option);
if (!opt_validate(opt, errp)) {
qemu_opt_del(opt);
@@ -889,11 +875,11 @@ bool has_help_option(const char *params)
bool qemu_opts_do_parse(QemuOpts *opts, const char *params,
const char *firstname, Error **errp)
{
- return opts_do_parse(opts, params, firstname, false, false, NULL, errp);
+ return opts_do_parse(opts, params, firstname, false, NULL, errp);
}
static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
- bool permit_abbrev, bool defaults,
+ bool permit_abbrev,
bool warn_on_flag, bool *help_wanted, Error **errp)
{
const char *firstname;
@@ -903,21 +889,13 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
assert(!permit_abbrev || list->implied_opt_name);
firstname = permit_abbrev ? list->implied_opt_name : NULL;
- /*
- * This code doesn't work for defaults && !list->merge_lists: when
- * params has no id=, and list has an element with !opts->id, it
- * appends a new element instead of returning the existing opts.
- * However, we got no use for this case. Guard against possible
- * (if unlikely) future misuse:
- */
- assert(!defaults || list->merge_lists);
opts = qemu_opts_create(list, id, !list->merge_lists, errp);
g_free(id);
if (opts == NULL) {
return NULL;
}
- if (!opts_do_parse(opts, params, firstname, defaults,
+ if (!opts_do_parse(opts, params, firstname,
warn_on_flag, help_wanted, errp)) {
qemu_opts_del(opts);
return NULL;
@@ -936,7 +914,7 @@ static QemuOpts *opts_parse(QemuOptsList *list, const char *params,
QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params,
bool permit_abbrev, Error **errp)
{
- return opts_parse(list, params, permit_abbrev, false, false, NULL, errp);
+ return opts_parse(list, params, permit_abbrev, false, NULL, errp);
}
/**
@@ -954,7 +932,7 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
QemuOpts *opts;
bool help_wanted = false;
- opts = opts_parse(list, params, permit_abbrev, false, true,
+ opts = opts_parse(list, params, permit_abbrev, true,
opts_accepts_any(list) ? NULL : &help_wanted,
&err);
if (!opts) {
@@ -968,15 +946,6 @@ QemuOpts *qemu_opts_parse_noisily(QemuOptsList *list, const char *params,
return opts;
}
-void qemu_opts_set_defaults(QemuOptsList *list, const char *params,
- int permit_abbrev)
-{
- QemuOpts *opts;
-
- opts = opts_parse(list, params, permit_abbrev, true, false, NULL, NULL);
- assert(opts);
-}
-
static bool qemu_opts_from_qdict_entry(QemuOpts *opts,
const QDictEntry *entry,
Error **errp)
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 06/11] machine: move dies from X86MachineState to CpuTopology
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (4 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 05/11] qemu-option: remove now-dead code Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 07/11] machine: move common smp_parse code to caller Paolo Bonzini
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: Daniel P . Berrangé, armbru
In order to make SMP configuration a Machine property, we need a getter as
well as a setter. To simplify the implementation put everything that the
getter needs in the CpuTopology struct.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/machine.c | 1 +
hw/i386/pc.c | 4 +---
hw/i386/x86.c | 15 +++++++--------
include/hw/boards.h | 1 +
include/hw/i386/pc.h | 1 -
include/hw/i386/x86.h | 1 -
6 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 55b9bc7817..d776c8cf20 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -970,6 +970,7 @@ static void machine_initfn(Object *obj)
ms->smp.cpus = mc->default_cpus;
ms->smp.max_cpus = mc->default_cpus;
ms->smp.cores = 1;
+ ms->smp.dies = 1;
ms->smp.threads = 1;
ms->smp.sockets = 1;
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index c6d8d0d84d..92958e9ad7 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -712,8 +712,6 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
*/
void pc_smp_parse(MachineState *ms, QemuOpts *opts)
{
- X86MachineState *x86ms = X86_MACHINE(ms);
-
if (opts) {
unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
@@ -769,7 +767,7 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
ms->smp.cores = cores;
ms->smp.threads = threads;
ms->smp.sockets = sockets;
- x86ms->smp_dies = dies;
+ ms->smp.dies = dies;
}
if (ms->smp.cpus > 1) {
diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index ed796fe6ba..2a99942016 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -64,7 +64,7 @@ inline void init_topo_info(X86CPUTopoInfo *topo_info,
{
MachineState *ms = MACHINE(x86ms);
- topo_info->dies_per_pkg = x86ms->smp_dies;
+ topo_info->dies_per_pkg = ms->smp.dies;
topo_info->cores_per_die = ms->smp.cores;
topo_info->threads_per_core = ms->smp.threads;
}
@@ -293,7 +293,7 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
init_topo_info(&topo_info, x86ms);
- env->nr_dies = x86ms->smp_dies;
+ env->nr_dies = ms->smp.dies;
/*
* If APIC ID is not set,
@@ -301,13 +301,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
*/
if (cpu->apic_id == UNASSIGNED_APIC_ID) {
int max_socket = (ms->smp.max_cpus - 1) /
- smp_threads / smp_cores / x86ms->smp_dies;
+ smp_threads / smp_cores / ms->smp.dies;
/*
* die-id was optional in QEMU 4.0 and older, so keep it optional
* if there's only one die per socket.
*/
- if (cpu->die_id < 0 && x86ms->smp_dies == 1) {
+ if (cpu->die_id < 0 && ms->smp.dies == 1) {
cpu->die_id = 0;
}
@@ -322,9 +322,9 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
if (cpu->die_id < 0) {
error_setg(errp, "CPU die-id is not set");
return;
- } else if (cpu->die_id > x86ms->smp_dies - 1) {
+ } else if (cpu->die_id > ms->smp.dies - 1) {
error_setg(errp, "Invalid CPU die-id: %u must be in range 0:%u",
- cpu->die_id, x86ms->smp_dies - 1);
+ cpu->die_id, ms->smp.dies - 1);
return;
}
if (cpu->core_id < 0) {
@@ -477,7 +477,7 @@ const CPUArchIdList *x86_possible_cpu_arch_ids(MachineState *ms)
&topo_info, &topo_ids);
ms->possible_cpus->cpus[i].props.has_socket_id = true;
ms->possible_cpus->cpus[i].props.socket_id = topo_ids.pkg_id;
- if (x86ms->smp_dies > 1) {
+ if (ms->smp.dies > 1) {
ms->possible_cpus->cpus[i].props.has_die_id = true;
ms->possible_cpus->cpus[i].props.die_id = topo_ids.die_id;
}
@@ -1252,7 +1252,6 @@ static void x86_machine_initfn(Object *obj)
x86ms->smm = ON_OFF_AUTO_AUTO;
x86ms->acpi = ON_OFF_AUTO_AUTO;
- x86ms->smp_dies = 1;
x86ms->pci_irq_mask = ACPI_BUILD_PCI_IRQS;
x86ms->oem_id = g_strndup(ACPI_BUILD_APPNAME6, 6);
x86ms->oem_table_id = g_strndup(ACPI_BUILD_APPNAME8, 8);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 3d55d2bd62..87ae5cc300 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -282,6 +282,7 @@ typedef struct DeviceMemoryState {
*/
typedef struct CpuTopology {
unsigned int cpus;
+ unsigned int dies;
unsigned int cores;
unsigned int threads;
unsigned int sockets;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 1522a3359a..4c2ca6d36a 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -19,7 +19,6 @@
* PCMachineState:
* @acpi_dev: link to ACPI PM device that performs ACPI hotplug handling
* @boot_cpus: number of present VCPUs
- * @smp_dies: number of dies per one package
*/
typedef struct PCMachineState {
/*< private >*/
diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
index c09b648dff..a6ffd94562 100644
--- a/include/hw/i386/x86.h
+++ b/include/hw/i386/x86.h
@@ -62,7 +62,6 @@ struct X86MachineState {
unsigned pci_irq_mask;
unsigned apic_id_limit;
uint16_t boot_cpus;
- unsigned smp_dies;
OnOffAuto smm;
OnOffAuto acpi;
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 07/11] machine: move common smp_parse code to caller
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (5 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 06/11] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 08/11] machine: add error propagation to mc->smp_parse Paolo Bonzini
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: armbru
Most of smp_parse and pc_smp_parse is guarded by an "if (opts)"
conditional, and the rest is common to both function. Move the
conditional and the common code to the caller, machine_smp_parse.
Move the replay_add_blocker call after all errors are checked for.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/machine.c | 114 +++++++++++++++++++++++-----------------------
hw/i386/pc.c | 108 ++++++++++++++++++++-----------------------
2 files changed, 107 insertions(+), 115 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index d776c8cf20..1016ec9e1c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -741,67 +741,59 @@ void machine_set_cpu_numa_node(MachineState *machine,
static void smp_parse(MachineState *ms, QemuOpts *opts)
{
- if (opts) {
- unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
- unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
- unsigned cores = qemu_opt_get_number(opts, "cores", 0);
- unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+ unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
+ unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+ unsigned cores = qemu_opt_get_number(opts, "cores", 0);
+ unsigned threads = qemu_opt_get_number(opts, "threads", 0);
- /* compute missing values, prefer sockets over cores over threads */
- if (cpus == 0 || sockets == 0) {
- cores = cores > 0 ? cores : 1;
- threads = threads > 0 ? threads : 1;
- if (cpus == 0) {
- sockets = sockets > 0 ? sockets : 1;
- cpus = cores * threads * sockets;
- } else {
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
- sockets = ms->smp.max_cpus / (cores * threads);
- }
- } else if (cores == 0) {
- threads = threads > 0 ? threads : 1;
- cores = cpus / (sockets * threads);
- cores = cores > 0 ? cores : 1;
- } else if (threads == 0) {
- threads = cpus / (cores * sockets);
- threads = threads > 0 ? threads : 1;
- } else if (sockets * cores * threads < cpus) {
- error_report("cpu topology: "
- "sockets (%u) * cores (%u) * threads (%u) < "
- "smp_cpus (%u)",
- sockets, cores, threads, cpus);
- exit(1);
+ /* compute missing values, prefer sockets over cores over threads */
+ if (cpus == 0 || sockets == 0) {
+ cores = cores > 0 ? cores : 1;
+ threads = threads > 0 ? threads : 1;
+ if (cpus == 0) {
+ sockets = sockets > 0 ? sockets : 1;
+ cpus = cores * threads * sockets;
+ } else {
+ ms->smp.max_cpus =
+ qemu_opt_get_number(opts, "maxcpus", cpus);
+ sockets = ms->smp.max_cpus / (cores * threads);
}
-
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
-
- if (ms->smp.max_cpus < cpus) {
- error_report("maxcpus must be equal to or greater than smp");
- exit(1);
- }
-
- if (sockets * cores * threads != ms->smp.max_cpus) {
- error_report("Invalid CPU topology: "
- "sockets (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, cores, threads,
- ms->smp.max_cpus);
- exit(1);
- }
-
- ms->smp.cpus = cpus;
- ms->smp.cores = cores;
- ms->smp.threads = threads;
- ms->smp.sockets = sockets;
+ } else if (cores == 0) {
+ threads = threads > 0 ? threads : 1;
+ cores = cpus / (sockets * threads);
+ cores = cores > 0 ? cores : 1;
+ } else if (threads == 0) {
+ threads = cpus / (cores * sockets);
+ threads = threads > 0 ? threads : 1;
+ } else if (sockets * cores * threads < cpus) {
+ error_report("cpu topology: "
+ "sockets (%u) * cores (%u) * threads (%u) < "
+ "smp_cpus (%u)",
+ sockets, cores, threads, cpus);
+ exit(1);
}
- if (ms->smp.cpus > 1) {
- Error *blocker = NULL;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
- replay_add_blocker(blocker);
+ ms->smp.max_cpus =
+ qemu_opt_get_number(opts, "maxcpus", cpus);
+
+ if (ms->smp.max_cpus < cpus) {
+ error_report("maxcpus must be equal to or greater than smp");
+ exit(1);
}
+
+ if (sockets * cores * threads != ms->smp.max_cpus) {
+ error_report("Invalid CPU topology: "
+ "sockets (%u) * cores (%u) * threads (%u) "
+ "!= maxcpus (%u)",
+ sockets, cores, threads,
+ ms->smp.max_cpus);
+ exit(1);
+ }
+
+ ms->smp.cpus = cpus;
+ ms->smp.cores = cores;
+ ms->smp.threads = threads;
+ ms->smp.sockets = sockets;
}
static void machine_class_init(ObjectClass *oc, void *data)
@@ -1135,7 +1127,9 @@ bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
{
MachineClass *mc = MACHINE_GET_CLASS(ms);
- mc->smp_parse(ms, opts);
+ if (opts) {
+ mc->smp_parse(ms, opts);
+ }
/* sanity-check smp_cpus and max_cpus against mc */
if (ms->smp.cpus < mc->min_cpus) {
@@ -1151,6 +1145,12 @@ bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
mc->name, mc->max_cpus);
return false;
}
+
+ if (ms->smp.cpus > 1) {
+ Error *blocker = NULL;
+ error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+ replay_add_blocker(blocker);
+ }
return true;
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 92958e9ad7..e206ac85f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -712,69 +712,61 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
*/
void pc_smp_parse(MachineState *ms, QemuOpts *opts)
{
- if (opts) {
- unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
- unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
- unsigned dies = qemu_opt_get_number(opts, "dies", 1);
- unsigned cores = qemu_opt_get_number(opts, "cores", 0);
- unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+ unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
+ unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
+ unsigned dies = qemu_opt_get_number(opts, "dies", 1);
+ unsigned cores = qemu_opt_get_number(opts, "cores", 0);
+ unsigned threads = qemu_opt_get_number(opts, "threads", 0);
- /* compute missing values, prefer sockets over cores over threads */
- if (cpus == 0 || sockets == 0) {
- cores = cores > 0 ? cores : 1;
- threads = threads > 0 ? threads : 1;
- if (cpus == 0) {
- sockets = sockets > 0 ? sockets : 1;
- cpus = cores * threads * dies * sockets;
- } else {
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
- sockets = ms->smp.max_cpus / (cores * threads * dies);
- }
- } else if (cores == 0) {
- threads = threads > 0 ? threads : 1;
- cores = cpus / (sockets * dies * threads);
- cores = cores > 0 ? cores : 1;
- } else if (threads == 0) {
- threads = cpus / (cores * dies * sockets);
- threads = threads > 0 ? threads : 1;
- } else if (sockets * dies * cores * threads < cpus) {
- error_report("cpu topology: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
- "smp_cpus (%u)",
- sockets, dies, cores, threads, cpus);
- exit(1);
+ /* compute missing values, prefer sockets over cores over threads */
+ if (cpus == 0 || sockets == 0) {
+ cores = cores > 0 ? cores : 1;
+ threads = threads > 0 ? threads : 1;
+ if (cpus == 0) {
+ sockets = sockets > 0 ? sockets : 1;
+ cpus = cores * threads * dies * sockets;
+ } else {
+ ms->smp.max_cpus =
+ qemu_opt_get_number(opts, "maxcpus", cpus);
+ sockets = ms->smp.max_cpus / (cores * threads * dies);
}
-
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
-
- if (ms->smp.max_cpus < cpus) {
- error_report("maxcpus must be equal to or greater than smp");
- exit(1);
- }
-
- if (sockets * dies * cores * threads != ms->smp.max_cpus) {
- error_report("Invalid CPU topology deprecated: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, dies, cores, threads,
- ms->smp.max_cpus);
- exit(1);
- }
-
- ms->smp.cpus = cpus;
- ms->smp.cores = cores;
- ms->smp.threads = threads;
- ms->smp.sockets = sockets;
- ms->smp.dies = dies;
+ } else if (cores == 0) {
+ threads = threads > 0 ? threads : 1;
+ cores = cpus / (sockets * dies * threads);
+ cores = cores > 0 ? cores : 1;
+ } else if (threads == 0) {
+ threads = cpus / (cores * dies * sockets);
+ threads = threads > 0 ? threads : 1;
+ } else if (sockets * dies * cores * threads < cpus) {
+ error_report("cpu topology: "
+ "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
+ "smp_cpus (%u)",
+ sockets, dies, cores, threads, cpus);
+ exit(1);
}
- if (ms->smp.cpus > 1) {
- Error *blocker = NULL;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
- replay_add_blocker(blocker);
+ ms->smp.max_cpus =
+ qemu_opt_get_number(opts, "maxcpus", cpus);
+
+ if (ms->smp.max_cpus < cpus) {
+ error_report("maxcpus must be equal to or greater than smp");
+ exit(1);
}
+
+ if (sockets * dies * cores * threads != ms->smp.max_cpus) {
+ error_report("Invalid CPU topology deprecated: "
+ "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+ "!= maxcpus (%u)",
+ sockets, dies, cores, threads,
+ ms->smp.max_cpus);
+ exit(1);
+ }
+
+ ms->smp.cpus = cpus;
+ ms->smp.cores = cores;
+ ms->smp.threads = threads;
+ ms->smp.sockets = sockets;
+ ms->smp.dies = dies;
}
static
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 08/11] machine: add error propagation to mc->smp_parse
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (6 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 07/11] machine: move common smp_parse code to caller Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 09/11] machine: pass QAPI struct " Paolo Bonzini
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: Daniel P . Berrangé, armbru
Clean up the smp_parse functions to use Error** instead of exiting.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/machine.c | 34 +++++++++++++++++++---------------
hw/i386/pc.c | 28 ++++++++++++++--------------
include/hw/boards.h | 2 +-
include/hw/i386/pc.h | 2 --
4 files changed, 34 insertions(+), 32 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1016ec9e1c..5a9c97ccc5 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -739,7 +739,7 @@ void machine_set_cpu_numa_node(MachineState *machine,
}
}
-static void smp_parse(MachineState *ms, QemuOpts *opts)
+static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
{
unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
@@ -766,28 +766,28 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
threads = cpus / (cores * sockets);
threads = threads > 0 ? threads : 1;
} else if (sockets * cores * threads < cpus) {
- error_report("cpu topology: "
- "sockets (%u) * cores (%u) * threads (%u) < "
- "smp_cpus (%u)",
- sockets, cores, threads, cpus);
- exit(1);
+ error_setg(errp, "cpu topology: "
+ "sockets (%u) * cores (%u) * threads (%u) < "
+ "smp_cpus (%u)",
+ sockets, cores, threads, cpus);
+ return;
}
ms->smp.max_cpus =
qemu_opt_get_number(opts, "maxcpus", cpus);
if (ms->smp.max_cpus < cpus) {
- error_report("maxcpus must be equal to or greater than smp");
- exit(1);
+ error_setg(errp, "maxcpus must be equal to or greater than smp");
+ return;
}
if (sockets * cores * threads != ms->smp.max_cpus) {
- error_report("Invalid CPU topology: "
- "sockets (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, cores, threads,
- ms->smp.max_cpus);
- exit(1);
+ error_setg(errp, "Invalid CPU topology: "
+ "sockets (%u) * cores (%u) * threads (%u) "
+ "!= maxcpus (%u)",
+ sockets, cores, threads,
+ ms->smp.max_cpus);
+ return;
}
ms->smp.cpus = cpus;
@@ -1126,9 +1126,13 @@ MemoryRegion *machine_consume_memdev(MachineState *machine,
bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
{
MachineClass *mc = MACHINE_GET_CLASS(ms);
+ ERRP_GUARD();
if (opts) {
- mc->smp_parse(ms, opts);
+ mc->smp_parse(ms, opts, errp);
+ if (*errp) {
+ return false;
+ }
}
/* sanity-check smp_cpus and max_cpus against mc */
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index e206ac85f3..cce275dcb1 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -710,7 +710,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
* This function is very similar to smp_parse()
* in hw/core/machine.c but includes CPU die support.
*/
-void pc_smp_parse(MachineState *ms, QemuOpts *opts)
+static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
{
unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
@@ -738,28 +738,28 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
threads = cpus / (cores * dies * sockets);
threads = threads > 0 ? threads : 1;
} else if (sockets * dies * cores * threads < cpus) {
- error_report("cpu topology: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
- "smp_cpus (%u)",
- sockets, dies, cores, threads, cpus);
- exit(1);
+ error_setg(errp, "cpu topology: "
+ "sockets (%u) * dies (%u) * cores (%u) * threads (%u) < "
+ "smp_cpus (%u)",
+ sockets, dies, cores, threads, cpus);
+ return;
}
ms->smp.max_cpus =
qemu_opt_get_number(opts, "maxcpus", cpus);
if (ms->smp.max_cpus < cpus) {
- error_report("maxcpus must be equal to or greater than smp");
- exit(1);
+ error_setg(errp, "maxcpus must be equal to or greater than smp");
+ return;
}
if (sockets * dies * cores * threads != ms->smp.max_cpus) {
- error_report("Invalid CPU topology deprecated: "
- "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
- "!= maxcpus (%u)",
- sockets, dies, cores, threads,
- ms->smp.max_cpus);
- exit(1);
+ error_setg(errp, "Invalid CPU topology deprecated: "
+ "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+ "!= maxcpus (%u)",
+ sockets, dies, cores, threads,
+ ms->smp.max_cpus);
+ return;
}
ms->smp.cpus = cpus;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 87ae5cc300..0483d6af86 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -210,7 +210,7 @@ struct MachineClass {
void (*reset)(MachineState *state);
void (*wakeup)(MachineState *state);
int (*kvm_type)(MachineState *machine, const char *arg);
- void (*smp_parse)(MachineState *ms, QemuOpts *opts);
+ void (*smp_parse)(MachineState *ms, QemuOpts *opts, Error **errp);
BlockInterfaceType block_default_type;
int units_per_default_bus;
diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 4c2ca6d36a..87294f2632 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -138,8 +138,6 @@ extern int fd_bootchk;
void pc_acpi_smi_interrupt(void *opaque, int irq, int level);
-void pc_smp_parse(MachineState *ms, QemuOpts *opts);
-
void pc_guest_info_init(PCMachineState *pcms);
#define PCI_HOST_PROP_PCI_HOLE_START "pci-hole-start"
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 09/11] machine: pass QAPI struct to mc->smp_parse
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (7 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 08/11] machine: add error propagation to mc->smp_parse Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 10/11] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: Daniel P . Berrangé, armbru
As part of converting -smp to a property with a QAPI type, define
the struct and use it to do the actual parsing. machine_smp_parse
takes care of doing the QemuOpts->QAPI conversion by hand, for now.
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/machine.c | 33 +++++++++++++++++++++++----------
hw/i386/pc.c | 18 ++++++++----------
include/hw/boards.h | 2 +-
qapi/machine.json | 27 +++++++++++++++++++++++++++
4 files changed, 59 insertions(+), 21 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 5a9c97ccc5..9ad8341a31 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -739,12 +739,12 @@ void machine_set_cpu_numa_node(MachineState *machine,
}
}
-static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
+static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
{
- unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
- unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
- unsigned cores = qemu_opt_get_number(opts, "cores", 0);
- unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+ unsigned cpus = config->has_cpus ? config->cpus : 0;
+ unsigned sockets = config->has_sockets ? config->sockets : 0;
+ unsigned cores = config->has_cores ? config->cores : 0;
+ unsigned threads = config->has_threads ? config->threads : 0;
/* compute missing values, prefer sockets over cores over threads */
if (cpus == 0 || sockets == 0) {
@@ -754,8 +754,7 @@ static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
sockets = sockets > 0 ? sockets : 1;
cpus = cores * threads * sockets;
} else {
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
+ ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
sockets = ms->smp.max_cpus / (cores * threads);
}
} else if (cores == 0) {
@@ -773,8 +772,7 @@ static void smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
return;
}
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
+ ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
if (ms->smp.max_cpus < cpus) {
error_setg(errp, "maxcpus must be equal to or greater than smp");
@@ -1129,7 +1127,22 @@ bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
ERRP_GUARD();
if (opts) {
- mc->smp_parse(ms, opts, errp);
+ SMPConfiguration config = {
+ .has_cpus = !!qemu_opt_get(opts, "cpus"),
+ .cpus = qemu_opt_get_number(opts, "cpus", 0),
+ .has_sockets = !!qemu_opt_get(opts, "sockets"),
+ .sockets = qemu_opt_get_number(opts, "sockets", 0),
+ .has_dies = !!qemu_opt_get(opts, "dies"),
+ .dies = qemu_opt_get_number(opts, "dies", 0),
+ .has_cores = !!qemu_opt_get(opts, "cores"),
+ .cores = qemu_opt_get_number(opts, "cores", 0),
+ .has_threads = !!qemu_opt_get(opts, "threads"),
+ .threads = qemu_opt_get_number(opts, "threads", 0),
+ .has_maxcpus = !!qemu_opt_get(opts, "maxcpus"),
+ .maxcpus = qemu_opt_get_number(opts, "maxcpus", 0),
+ };
+
+ mc->smp_parse(ms, &config, errp);
if (*errp) {
return false;
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index cce275dcb1..8e1220db72 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -710,13 +710,13 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
* This function is very similar to smp_parse()
* in hw/core/machine.c but includes CPU die support.
*/
-static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
+static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
{
- unsigned cpus = qemu_opt_get_number(opts, "cpus", 0);
- unsigned sockets = qemu_opt_get_number(opts, "sockets", 0);
- unsigned dies = qemu_opt_get_number(opts, "dies", 1);
- unsigned cores = qemu_opt_get_number(opts, "cores", 0);
- unsigned threads = qemu_opt_get_number(opts, "threads", 0);
+ unsigned cpus = config->has_cpus ? config->cpus : 0;
+ unsigned sockets = config->has_sockets ? config->sockets : 0;
+ unsigned dies = config->has_dies ? config->dies : 1;
+ unsigned cores = config->has_cores ? config->cores : 0;
+ unsigned threads = config->has_threads ? config->threads : 0;
/* compute missing values, prefer sockets over cores over threads */
if (cpus == 0 || sockets == 0) {
@@ -726,8 +726,7 @@ static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
sockets = sockets > 0 ? sockets : 1;
cpus = cores * threads * dies * sockets;
} else {
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
+ ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
sockets = ms->smp.max_cpus / (cores * threads * dies);
}
} else if (cores == 0) {
@@ -745,8 +744,7 @@ static void pc_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
return;
}
- ms->smp.max_cpus =
- qemu_opt_get_number(opts, "maxcpus", cpus);
+ ms->smp.max_cpus = config->has_maxcpus ? config->maxcpus : cpus;
if (ms->smp.max_cpus < cpus) {
error_setg(errp, "maxcpus must be equal to or greater than smp");
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0483d6af86..1eae4427e8 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -210,7 +210,7 @@ struct MachineClass {
void (*reset)(MachineState *state);
void (*wakeup)(MachineState *state);
int (*kvm_type)(MachineState *machine, const char *arg);
- void (*smp_parse)(MachineState *ms, QemuOpts *opts, Error **errp);
+ void (*smp_parse)(MachineState *ms, SMPConfiguration *config, Error **errp);
BlockInterfaceType block_default_type;
int units_per_default_bus;
diff --git a/qapi/machine.json b/qapi/machine.json
index e4d0f9b24f..3301f5235e 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1284,3 +1284,31 @@
##
{ 'event': 'MEM_UNPLUG_ERROR',
'data': { 'device': 'str', 'msg': 'str' } }
+
+##
+# @SMPConfiguration:
+#
+# Schema for CPU topology configuration. "0" or a missing value is taken to
+# mean "figure out a suitable value based on the ones that are provided.
+#
+# @cpus: number of virtual CPUs in the virtual machine
+#
+# @sockets: number of sockets in the CPU topology
+#
+# @dies: number of dies per socket in the CPU topology
+#
+# @cores: number of cores per thread in the CPU topology
+#
+# @threads: number of threads per core in the CPU topology
+#
+# @maxcpus: maximum number of hotpluggable virtual CPUs in the virtual machine
+#
+# Since: 6.1
+##
+{ 'struct': 'SMPConfiguration', 'data': {
+ '*cpus': 'int',
+ '*sockets': 'int',
+ '*dies': 'int',
+ '*cores': 'int',
+ '*threads': 'int',
+ '*maxcpus': 'int' } }
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 10/11] machine: reject -smp dies!=1 for non-PC machines
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (8 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 09/11] machine: pass QAPI struct " Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 11/11] machine: add smp compound property Paolo Bonzini
2021-06-17 16:21 ` [PATCH v2 00/11] vl: compound properties for machines no-reply
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: Daniel P . Berrangé, armbru
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/machine.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 9ad8341a31..ffc076ae84 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -746,6 +746,10 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
unsigned cores = config->has_cores ? config->cores : 0;
unsigned threads = config->has_threads ? config->threads : 0;
+ if (config->has_dies && config->dies != 0 && config->dies != 1) {
+ error_setg(errp, "dies not supported by this machine's CPU topology");
+ }
+
/* compute missing values, prefer sockets over cores over threads */
if (cpus == 0 || sockets == 0) {
cores = cores > 0 ? cores : 1;
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 11/11] machine: add smp compound property
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (9 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 10/11] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
@ 2021-06-17 15:53 ` Paolo Bonzini
2021-06-17 16:21 ` [PATCH v2 00/11] vl: compound properties for machines no-reply
11 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2021-06-17 15:53 UTC (permalink / raw
To: qemu-devel; +Cc: armbru
Make -smp syntactic sugar for a compound property "-machine
smp.{cores,threads,cpu,...}". machine_smp_parse is replaced by the
setter for the property.
numa-test will now cover the new syntax, while other tests
still use -smp.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
hw/core/machine.c | 108 +++++++++++++++++++++-------------------
include/hw/boards.h | 1 -
softmmu/vl.c | 33 +++++++++---
tests/qtest/numa-test.c | 22 ++++----
4 files changed, 95 insertions(+), 69 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ffc076ae84..c6ae89efec 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -19,6 +19,7 @@
#include "hw/loader.h"
#include "qapi/error.h"
#include "qapi/qapi-visit-common.h"
+#include "qapi/qapi-visit-machine.h"
#include "qapi/visitor.h"
#include "hw/sysbus.h"
#include "sysemu/cpus.h"
@@ -798,6 +799,57 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
ms->smp.sockets = sockets;
}
+static void machine_get_smp(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ MachineState *ms = MACHINE(obj);
+ SMPConfiguration *config = &(SMPConfiguration){
+ .has_cores = true, .cores = ms->smp.cores,
+ .has_sockets = true, .sockets = ms->smp.sockets,
+ .has_dies = true, .dies = ms->smp.dies,
+ .has_threads = true, .threads = ms->smp.threads,
+ .has_cpus = true, .cpus = ms->smp.cpus,
+ .has_maxcpus = true, .maxcpus = ms->smp.max_cpus,
+ };
+ if (!visit_type_SMPConfiguration(v, name, &config, &error_abort)) {
+ return;
+ }
+}
+
+static void machine_set_smp(Object *obj, Visitor *v, const char *name,
+ void *opaque, Error **errp)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(obj);
+ MachineState *ms = MACHINE(obj);
+ SMPConfiguration *config;
+ ERRP_GUARD();
+
+ if (!visit_type_SMPConfiguration(v, name, &config, errp)) {
+ return;
+ }
+
+ mc->smp_parse(ms, config, errp);
+ if (errp) {
+ goto out_free;
+ }
+
+ /* sanity-check smp_cpus and max_cpus against mc */
+ if (ms->smp.cpus < mc->min_cpus) {
+ error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
+ "supported by machine '%s' is %d",
+ ms->smp.cpus,
+ mc->name, mc->min_cpus);
+ } else if (ms->smp.max_cpus > mc->max_cpus) {
+ error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
+ "supported by machine '%s' is %d",
+ current_machine->smp.max_cpus,
+ mc->name, mc->max_cpus);
+ }
+
+out_free:
+ qapi_free_SMPConfiguration(config);
+}
+
static void machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -837,6 +889,12 @@ static void machine_class_init(ObjectClass *oc, void *data)
object_class_property_set_description(oc, "dumpdtb",
"Dump current dtb to a file and quit");
+ object_class_property_add(oc, "smp", "SMPConfiguration",
+ machine_get_smp, machine_set_smp,
+ NULL, NULL);
+ object_class_property_set_description(oc, "smp",
+ "CPU topology");
+
object_class_property_add(oc, "phandle-start", "int",
machine_get_phandle_start, machine_set_phandle_start,
NULL, NULL);
@@ -1125,56 +1183,6 @@ MemoryRegion *machine_consume_memdev(MachineState *machine,
return ret;
}
-bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp)
-{
- MachineClass *mc = MACHINE_GET_CLASS(ms);
- ERRP_GUARD();
-
- if (opts) {
- SMPConfiguration config = {
- .has_cpus = !!qemu_opt_get(opts, "cpus"),
- .cpus = qemu_opt_get_number(opts, "cpus", 0),
- .has_sockets = !!qemu_opt_get(opts, "sockets"),
- .sockets = qemu_opt_get_number(opts, "sockets", 0),
- .has_dies = !!qemu_opt_get(opts, "dies"),
- .dies = qemu_opt_get_number(opts, "dies", 0),
- .has_cores = !!qemu_opt_get(opts, "cores"),
- .cores = qemu_opt_get_number(opts, "cores", 0),
- .has_threads = !!qemu_opt_get(opts, "threads"),
- .threads = qemu_opt_get_number(opts, "threads", 0),
- .has_maxcpus = !!qemu_opt_get(opts, "maxcpus"),
- .maxcpus = qemu_opt_get_number(opts, "maxcpus", 0),
- };
-
- mc->smp_parse(ms, &config, errp);
- if (*errp) {
- return false;
- }
- }
-
- /* sanity-check smp_cpus and max_cpus against mc */
- if (ms->smp.cpus < mc->min_cpus) {
- error_setg(errp, "Invalid SMP CPUs %d. The min CPUs "
- "supported by machine '%s' is %d",
- ms->smp.cpus,
- mc->name, mc->min_cpus);
- return false;
- } else if (ms->smp.max_cpus > mc->max_cpus) {
- error_setg(errp, "Invalid SMP CPUs %d. The max CPUs "
- "supported by machine '%s' is %d",
- current_machine->smp.max_cpus,
- mc->name, mc->max_cpus);
- return false;
- }
-
- if (ms->smp.cpus > 1) {
- Error *blocker = NULL;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
- replay_add_blocker(blocker);
- }
- return true;
-}
-
void machine_run_board_init(MachineState *machine)
{
MachineClass *machine_class = MACHINE_GET_CLASS(machine);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 1eae4427e8..accd6eff35 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -26,7 +26,6 @@ OBJECT_DECLARE_TYPE(MachineState, MachineClass, MACHINE)
extern MachineState *current_machine;
void machine_run_board_init(MachineState *machine);
-bool machine_smp_parse(MachineState *ms, QemuOpts *opts, Error **errp);
bool machine_usb(MachineState *machine);
int machine_phandle_start(MachineState *machine);
bool machine_dump_guest_core(MachineState *machine);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 01bcb16560..683f2bb488 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1523,6 +1523,25 @@ static void machine_help_func(const QDict *qdict)
}
}
+static void
+machine_parse_property_opt(QemuOptsList *opts_list, const char *propname,
+ const char *arg, Error **errp)
+{
+ QDict *opts, *prop;
+ bool help = false;
+ ERRP_GUARD();
+
+ prop = keyval_parse(arg, opts_list->implied_opt_name, &help, errp);
+ if (help) {
+ qemu_opts_print_help(opts_list, true);
+ return;
+ }
+ opts = qdict_new();
+ qdict_put(opts, propname, prop);
+ keyval_merge(machine_opts_dict, opts, errp);
+ qobject_unref(opts);
+}
+
static const char *pid_file;
static Notifier qemu_unlink_pidfile_notifier;
@@ -1833,6 +1852,12 @@ static void qemu_apply_machine_options(QDict *qdict)
/* fall back to the -kernel/-append */
semihosting_arg_fallback(current_machine->kernel_filename, current_machine->kernel_cmdline);
}
+
+ if (current_machine->smp.cpus > 1) {
+ Error *blocker = NULL;
+ error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
+ replay_add_blocker(blocker);
+ }
}
static void qemu_create_early_backends(void)
@@ -2074,9 +2099,6 @@ static void qemu_create_machine(QDict *qdict)
qemu_set_hw_version(machine_class->hw_version);
}
- machine_smp_parse(current_machine,
- qemu_opts_find(qemu_find_opts("smp-opts"), NULL), &error_fatal);
-
/*
* Get the default machine options from the machine if it is not already
* specified either by the configuration file or by the command line.
@@ -3291,10 +3313,7 @@ void qemu_init(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_smp:
- if (!qemu_opts_parse_noisily(qemu_find_opts("smp-opts"),
- optarg, true)) {
- exit(1);
- }
+ machine_parse_property_opt(qemu_find_opts("smp-opts"), "smp", optarg, &error_fatal);
break;
case QEMU_OPTION_vnc:
vnc_parse(optarg);
diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index dc0ec571ca..c677cd63c4 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -25,7 +25,7 @@ static void test_mon_explicit(const void *data)
g_autofree char *s = NULL;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-smp 8 -numa node,nodeid=0,memdev=ram,cpus=0-3 "
+ cli = make_cli(data, "-machine smp.cpus=8 -numa node,nodeid=0,memdev=ram,cpus=0-3 "
"-numa node,nodeid=1,cpus=4-7");
qts = qtest_init(cli);
@@ -42,7 +42,7 @@ static void test_def_cpu_split(const void *data)
g_autofree char *s = NULL;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-smp 8 -numa node,memdev=ram -numa node");
+ cli = make_cli(data, "-machine smp.cpus=8 -numa node,memdev=ram -numa node");
qts = qtest_init(cli);
s = qtest_hmp(qts, "info numa");
@@ -58,7 +58,7 @@ static void test_mon_partial(const void *data)
g_autofree char *s = NULL;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-smp 8 "
+ cli = make_cli(data, "-machine smp.cpus=8 "
"-numa node,nodeid=0,memdev=ram,cpus=0-1 "
"-numa node,nodeid=1,cpus=4-5 ");
qts = qtest_init(cli);
@@ -86,7 +86,7 @@ static void test_query_cpus(const void *data)
QTestState *qts;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-smp 8 -numa node,memdev=ram,cpus=0-3 "
+ cli = make_cli(data, "-machine smp.cpus=8 -numa node,memdev=ram,cpus=0-3 "
"-numa node,cpus=4-7");
qts = qtest_init(cli);
cpus = get_cpus(qts, &resp);
@@ -124,7 +124,7 @@ static void pc_numa_cpu(const void *data)
QTestState *qts;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-cpu pentium -smp 8,sockets=2,cores=2,threads=2 "
+ cli = make_cli(data, "-cpu pentium -machine smp.cpus=8,smp.sockets=2,smp.cores=2,smp.threads=2 "
"-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
"-numa cpu,node-id=1,socket-id=0 "
"-numa cpu,node-id=0,socket-id=1,core-id=0 "
@@ -177,7 +177,7 @@ static void spapr_numa_cpu(const void *data)
QTestState *qts;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-smp 4,cores=4 "
+ cli = make_cli(data, "-machine smp.cpus=4,smp.cores=4 "
"-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
"-numa cpu,node-id=0,core-id=0 "
"-numa cpu,node-id=0,core-id=1 "
@@ -222,7 +222,7 @@ static void aarch64_numa_cpu(const void *data)
QTestState *qts;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-smp 2 "
+ cli = make_cli(data, "-machine smp.cpus=2 "
"-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
"-numa cpu,node-id=1,thread-id=0 "
"-numa cpu,node-id=0,thread-id=1");
@@ -265,7 +265,7 @@ static void pc_dynamic_cpu_cfg(const void *data)
QTestState *qs;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-nodefaults --preconfig -smp 2");
+ cli = make_cli(data, "-nodefaults --preconfig -machine smp.cpus=2");
qs = qtest_init(cli);
/* create 2 numa nodes */
@@ -324,7 +324,7 @@ static void pc_hmat_build_cfg(const void *data)
g_autofree char *cli = NULL;
cli = make_cli(data, "-nodefaults --preconfig -machine hmat=on "
- "-smp 2,sockets=2 "
+ "-machine smp.cpus=2,smp.sockets=2 "
"-m 128M,slots=2,maxmem=1G "
"-object memory-backend-ram,size=64M,id=m0 "
"-object memory-backend-ram,size=64M,id=m1 "
@@ -453,7 +453,7 @@ static void pc_hmat_off_cfg(const void *data)
g_autofree char *cli = NULL;
cli = make_cli(data, "-nodefaults --preconfig "
- "-smp 2,sockets=2 "
+ "-machine smp.cpus=2,smp.sockets=2 "
"-m 128M,slots=2,maxmem=1G "
"-object memory-backend-ram,size=64M,id=m0,prealloc=y "
"-object memory-backend-ram,size=64M,id=m1 "
@@ -492,7 +492,7 @@ static void pc_hmat_erange_cfg(const void *data)
g_autofree char *cli = NULL;
cli = make_cli(data, "-nodefaults --preconfig -machine hmat=on "
- "-smp 2,sockets=2 "
+ "-machine smp.cpus=2,smp.sockets=2 "
"-m 128M,slots=2,maxmem=1G "
"-object memory-backend-ram,size=64M,id=m0 "
"-object memory-backend-ram,size=64M,id=m1 "
--
2.31.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 00/11] vl: compound properties for machines
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
` (10 preceding siblings ...)
2021-06-17 15:53 ` [PATCH v2 11/11] machine: add smp compound property Paolo Bonzini
@ 2021-06-17 16:21 ` no-reply
11 siblings, 0 replies; 14+ messages in thread
From: no-reply @ 2021-06-17 16:21 UTC (permalink / raw
To: pbonzini; +Cc: qemu-devel, armbru
Patchew URL: https://patchew.org/QEMU/20210617155308.928754-1-pbonzini@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210617155308.928754-1-pbonzini@redhat.com
Subject: [PATCH v2 00/11] vl: compound properties for machines
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag] patchew/20210617155308.928754-1-pbonzini@redhat.com -> patchew/20210617155308.928754-1-pbonzini@redhat.com
Switched to a new branch 'test'
7860c75 machine: add smp compound property
4041654 machine: reject -smp dies!=1 for non-PC machines
9f99606 machine: pass QAPI struct to mc->smp_parse
c40ee9c machine: add error propagation to mc->smp_parse
a25c706 machine: move common smp_parse code to caller
b484366 machine: move dies from X86MachineState to CpuTopology
610f271 qemu-option: remove now-dead code
9fe2b23 vl: switch -M parsing to keyval
49a23ce keyval: introduce keyval_parse_into
d3a4e0d keyval: introduce keyval_merge
2dee6bc qom: export more functions for use with non-UserCreatable objects
=== OUTPUT BEGIN ===
1/11 Checking commit 2dee6bc29821 (qom: export more functions for use with non-UserCreatable objects)
2/11 Checking commit d3a4e0dd01bb (keyval: introduce keyval_merge)
ERROR: line over 90 characters
#45: FILE: tests/unit/test-keyval.c:756:
+ QDict *combined = keyval_parse("opt1=ABC,opt2.sub1=def,opt2.sub2=GHI,opt2.sub3=JKL,opt3=xyz",
WARNING: line over 80 characters
#119: FILE: util/keyval.c:318:
+static void keyval_do_merge(QDict *dest, const QDict *merged, GString *str, Error **errp)
ERROR: line over 90 characters
#129: FILE: util/keyval.c:328:
+ error_setg(errp, "Parameter '%s%s' used inconsistently", str->str, ent->key);
WARNING: Block comments use a leading /* on a separate line
#160: FILE: util/keyval.c:359:
+/* Merge the @merged dictionary into @dest. The dictionaries are expected to be
total: 2 errors, 2 warnings, 153 lines checked
Patch 2/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/11 Checking commit 49a23ce11213 (keyval: introduce keyval_parse_into)
WARNING: line over 80 characters
#28: FILE: include/qemu/option.h:150:
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
WARNING: line over 80 characters
#50: FILE: util/keyval.c:510:
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
total: 0 errors, 2 warnings, 78 lines checked
Patch 3/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/11 Checking commit 9fe2b23c1acd (vl: switch -M parsing to keyval)
WARNING: line over 80 characters
#206: FILE: softmmu/vl.c:1592:
+ error_setg(&local_err, "No machine specified, and there is no default");
WARNING: line over 80 characters
#219: FILE: softmmu/vl.c:1598:
+ error_append_hint(&local_err, "Use -machine help to list supported machines\n");
WARNING: line over 80 characters
#256: FILE: softmmu/vl.c:1639:
+ error_setg(errp, "Conflict between '%s' and '%s'", ent->key, new_key);
WARNING: line over 80 characters
#300: FILE: softmmu/vl.c:1663:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", value,
WARNING: line over 80 characters
#307: FILE: softmmu/vl.c:1670:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kvm-shadow-mem", value,
WARNING: line over 80 characters
#314: FILE: softmmu/vl.c:1677:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kernel-irqchip", value,
WARNING: line over 80 characters
#316: FILE: softmmu/vl.c:1679:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), "kernel-irqchip", value,
ERROR: line over 90 characters
#338: FILE: softmmu/vl.c:1804:
+ object_set_properties_from_keyval(OBJECT(current_machine), qdict, false, &error_fatal);
ERROR: line over 90 characters
#350: FILE: softmmu/vl.c:1834:
+ semihosting_arg_fallback(current_machine->kernel_filename, current_machine->kernel_cmdline);
ERROR: code indent should never use tabs
#414: FILE: softmmu/vl.c:2128:
+^I * Cannot merge string-valued and type-safe dictionaries, so JSON$
ERROR: code indent should never use tabs
#415: FILE: softmmu/vl.c:2129:
+^I * is not accepted yet for -M.$
ERROR: code indent should never use tabs
#416: FILE: softmmu/vl.c:2130:
+^I */$
ERROR: line over 90 characters
#526: FILE: softmmu/vl.c:3248:
+ keyval_parse_into(machine_opts_dict, optarg, "type", &help, &error_fatal);
total: 6 errors, 7 warnings, 536 lines checked
Patch 4/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/11 Checking commit 610f27167f8f (qemu-option: remove now-dead code)
6/11 Checking commit b48436688841 (machine: move dies from X86MachineState to CpuTopology)
7/11 Checking commit a25c706e0639 (machine: move common smp_parse code to caller)
8/11 Checking commit c40ee9c046fb (machine: add error propagation to mc->smp_parse)
9/11 Checking commit 9f9960617bab (machine: pass QAPI struct to mc->smp_parse)
WARNING: line over 80 characters
#96: FILE: hw/i386/pc.c:713:
+static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
total: 0 errors, 1 warnings, 134 lines checked
Patch 9/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/11 Checking commit 4041654d0ebc (machine: reject -smp dies!=1 for non-PC machines)
11/11 Checking commit 7860c7588061 (machine: add smp compound property)
ERROR: line over 90 characters
#236: FILE: softmmu/vl.c:3316:
+ machine_parse_property_opt(qemu_find_opts("smp-opts"), "smp", optarg, &error_fatal);
WARNING: line over 80 characters
#249: FILE: tests/qtest/numa-test.c:28:
+ cli = make_cli(data, "-machine smp.cpus=8 -numa node,nodeid=0,memdev=ram,cpus=0-3 "
WARNING: line over 80 characters
#258: FILE: tests/qtest/numa-test.c:45:
+ cli = make_cli(data, "-machine smp.cpus=8 -numa node,memdev=ram -numa node");
ERROR: line over 90 characters
#285: FILE: tests/qtest/numa-test.c:127:
+ cli = make_cli(data, "-cpu pentium -machine smp.cpus=8,smp.sockets=2,smp.cores=2,smp.threads=2 "
total: 2 errors, 2 warnings, 284 lines checked
Patch 11/11 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20210617155308.928754-1-pbonzini@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 02/11] keyval: introduce keyval_merge
2021-06-17 15:52 ` [PATCH v2 02/11] keyval: introduce keyval_merge Paolo Bonzini
@ 2021-06-18 7:42 ` Markus Armbruster
0 siblings, 0 replies; 14+ messages in thread
From: Markus Armbruster @ 2021-06-18 7:42 UTC (permalink / raw
To: Paolo Bonzini; +Cc: qemu-devel
Paolo Bonzini <pbonzini@redhat.com> writes:
> This patch introduces a function that merges two keyval-produced
> (or keyval-like) QDicts. It can be used to emulate the behavior of
> .merge_lists = true QemuOpts groups, merging -readconfig sections and
> command-line options in a single QDict, and also to implement -set.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> include/qemu/option.h | 1 +
> tests/unit/test-keyval.c | 58 ++++++++++++++++++++++++++++++++
> util/keyval.c | 71 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 130 insertions(+)
>
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index f73e0dc7d9..d89c66145a 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -149,5 +149,6 @@ QemuOptsList *qemu_opts_append(QemuOptsList *dst, QemuOptsList *list);
>
> QDict *keyval_parse(const char *params, const char *implied_key,
> bool *help, Error **errp);
> +void keyval_merge(QDict *old, const QDict *new, Error **errp);
>
> #endif
> diff --git a/tests/unit/test-keyval.c b/tests/unit/test-keyval.c
> index e20c07cf3e..af0581ae6c 100644
> --- a/tests/unit/test-keyval.c
> +++ b/tests/unit/test-keyval.c
> @@ -747,6 +747,61 @@ static void test_keyval_visit_any(void)
> visit_free(v);
> }
>
> +static void test_keyval_merge_dict(void)
> +{
> + QDict *first = keyval_parse("opt1=abc,opt2.sub1=def,opt2.sub2=ghi,opt3=xyz",
> + NULL, NULL, &error_abort);
> + QDict *second = keyval_parse("opt1=ABC,opt2.sub2=GHI,opt2.sub3=JKL",
> + NULL, NULL, &error_abort);
> + QDict *combined = keyval_parse("opt1=ABC,opt2.sub1=def,opt2.sub2=GHI,opt2.sub3=JKL,opt3=xyz",
> + NULL, NULL, &error_abort);
> + Error *err = NULL;
> +
> + keyval_merge(first, second, &err);
> + g_assert(!err);
> + g_assert(qobject_is_equal(QOBJECT(combined), QOBJECT(first)));
> + qobject_unref(first);
> + qobject_unref(second);
> + qobject_unref(combined);
> +}
> +
> +static void test_keyval_merge_list(void)
> +{
> + QDict *first = keyval_parse("opt1.0=abc,opt2.0=xyz",
> + NULL, NULL, &error_abort);
> + QDict *second = keyval_parse("opt1.0=def",
> + NULL, NULL, &error_abort);
> + QDict *combined = keyval_parse("opt1.0=abc,opt1.1=def,opt2.0=xyz",
> + NULL, NULL, &error_abort);
> + Error *err = NULL;
> +
> + keyval_merge(first, second, &err);
> + g_assert(!err);
> + g_assert(qobject_is_equal(QOBJECT(combined), QOBJECT(first)));
> + qobject_unref(first);
> + qobject_unref(second);
> + qobject_unref(combined);
> +}
> +
> +static void test_keyval_merge_conflict(void)
> +{
> + QDict *first = keyval_parse("opt2=ABC",
> + NULL, NULL, &error_abort);
> + QDict *second = keyval_parse("opt2.sub1=def,opt2.sub2=ghi",
> + NULL, NULL, &error_abort);
> + QDict *third = qdict_clone_shallow(first);
> + Error *err = NULL;
> +
> + keyval_merge(first, second, &err);
> + error_free_or_abort(&err);
> + keyval_merge(second, third, &err);
> + error_free_or_abort(&err);
> +
> + qobject_unref(first);
> + qobject_unref(second);
> + qobject_unref(third);
> +}
> +
> int main(int argc, char *argv[])
> {
> g_test_init(&argc, &argv, NULL);
> @@ -760,6 +815,9 @@ int main(int argc, char *argv[])
> g_test_add_func("/keyval/visit/optional", test_keyval_visit_optional);
> g_test_add_func("/keyval/visit/alternate", test_keyval_visit_alternate);
> g_test_add_func("/keyval/visit/any", test_keyval_visit_any);
> + g_test_add_func("/keyval/merge/dict", test_keyval_merge_dict);
> + g_test_add_func("/keyval/merge/list", test_keyval_merge_list);
> + g_test_add_func("/keyval/merge/conflict", test_keyval_merge_conflict);
> g_test_run();
> return 0;
> }
> diff --git a/util/keyval.c b/util/keyval.c
> index be34928813..8006c5df20 100644
> --- a/util/keyval.c
> +++ b/util/keyval.c
> @@ -310,6 +310,77 @@ static char *reassemble_key(GSList *key)
> return g_string_free(s, FALSE);
> }
>
> +/*
> + * Recursive worker for keyval_merge. @str is the path that led to the
> + * current dictionary, to be used for error messages. It is modified
> + * internally but restored before the function returns.
> + */
Slight reformatting to better blend in with other comments in this file:
/*
* Recursive worker for keyval_merge
* @str is the path that led to the current dictionary, to be used for
* error messages. It is modified internally but restored before the
* function returns.
*/
> +static void keyval_do_merge(QDict *dest, const QDict *merged, GString *str, Error **errp)
> +{
> + size_t save_len = str->len;
> + const QDictEntry *ent;
> + QObject *old_value;
> +
> + for (ent = qdict_first(merged); ent; ent = qdict_next(merged, ent)) {
> + old_value = qdict_get(dest, ent->key);
> + if (old_value) {
> + if (qobject_type(old_value) != qobject_type(ent->value)) {
> + error_setg(errp, "Parameter '%s%s' used inconsistently", str->str, ent->key);
Long line, break after the string literal.
> + return;
> + } else if (qobject_type(ent->value) == QTYPE_QDICT) {
> + /* Merge sub-dictionaries. */
> + g_string_append(str, ent->key);
> + g_string_append_c(str, '.');
> + keyval_do_merge(qobject_to(QDict, old_value),
> + qobject_to(QDict, ent->value),
> + str, errp);
> + g_string_truncate(str, save_len);
> + continue;
> + } else if (qobject_type(ent->value) == QTYPE_QLIST) {
> + /* Append to old list. */
> + QList *old = qobject_to(QList, old_value);
> + QList *new = qobject_to(QList, ent->value);
> + const QListEntry *item;
> + QLIST_FOREACH_ENTRY(new, item) {
> + qobject_ref(item->value);
> + qlist_append_obj(old, item->value);
> + }
> + continue;
> + } else {
> + assert(qobject_type(ent->value) == QTYPE_QSTRING);
> + }
> + }
> +
> + qobject_ref(ent->value);
> + qdict_put_obj(dest, ent->key, ent->value);
> + }
> +}
> +
> +/* Merge the @merged dictionary into @dest. The dictionaries are expected to be
> + * returned by the keyval parser, and therefore the only expected scalar type
> + * is the * string. In case the same path is present in both @dest and @merged,
> + * the semantics are as follows:
> + *
> + * - lists are concatenated
> + *
> + * - dictionaries are merged recursively
> + *
> + * - for scalar values, @merged wins
> + *
> + * In case an error is reported, @dest may already have been modified.
> + *
> + * This function can be used to implement semantics analogous to QemuOpts's
> + * .merge_lists = true case, or to implement -set for options backed by QDicts.
> + */
Contents is already lovely. Now let's tidy up the formatting:
/*
* Merge the @merged dictionary into @dest.
*
* The dictionaries are expected to be returned by the keyval parser,
* and therefore the only expected scalar type is the * string. In
* case the same path is present in both @dest and @merged, the
* semantics are as follows:
*
* - lists are concatenated
*
* - dictionaries are merged recursively
*
* - for scalar values, @merged wins
*
* In case an error is reported, @dest may already have been modified.
*
* This function can be used to implement semantics analogous to
* QemuOpts's .merge_lists = true case, or to implement -set for
* options backed by QDicts.
*/
Let's mention where this fails to be analogous. Perhaps:
* Note: while QemuOpts is commonly used so that repeated keys
* overwrite ("last one wins"), it can also be used so that repeated
* keys build up a list. keyval_merge() can be analogous to the
* former usage, but not the latter.
> +void keyval_merge(QDict *dest, const QDict *merged, Error **errp)
> +{
> + GString *str;
> +
> + str = g_string_new("");
> + keyval_do_merge(dest, merged, str, errp);
> + g_string_free(str, TRUE);
> +}
> +
> /*
> * Listify @cur recursively.
> * Replace QDicts whose keys are all valid list indexes by QLists.
Since I'm asking only for minor improvements:
Reviewed-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-06-18 7:44 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-06-17 15:52 [PATCH v2 00/11] vl: compound properties for machines Paolo Bonzini
2021-06-17 15:52 ` [PATCH v2 01/11] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
2021-06-17 15:52 ` [PATCH v2 02/11] keyval: introduce keyval_merge Paolo Bonzini
2021-06-18 7:42 ` Markus Armbruster
2021-06-17 15:53 ` [PATCH v2 03/11] keyval: introduce keyval_parse_into Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 04/11] vl: switch -M parsing to keyval Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 05/11] qemu-option: remove now-dead code Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 06/11] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 07/11] machine: move common smp_parse code to caller Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 08/11] machine: add error propagation to mc->smp_parse Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 09/11] machine: pass QAPI struct " Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 10/11] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
2021-06-17 15:53 ` [PATCH v2 11/11] machine: add smp compound property Paolo Bonzini
2021-06-17 16:21 ` [PATCH v2 00/11] vl: compound properties for machines no-reply
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.