* [PATCH 01/14] remove -writeconfig
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 02/14] qemu-config: parse configuration files to a QDict Paolo Bonzini
` (13 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, armbru
Like -set and -readconfig, it would not really be too hard to
extend -writeconfig to parsing mechanisms other than QemuOpts.
However, the uses of -writeconfig are substantially more
limited, as it is generally easier to write the configuration
by hand in the first place. In addition, -writeconfig does
not even try to detect cases where it prints incorrect
syntax (for example if values have a quote in them, since
qemu_config_parse does not support any kind of escaping.
Just remove it.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/system/deprecated.rst | 7 ------
docs/system/removed-features.rst | 6 +++++
include/qemu/config-file.h | 1 -
qemu-options.hx | 8 ++----
softmmu/vl.c | 20 ---------------
util/qemu-config.c | 42 --------------------------------
6 files changed, 8 insertions(+), 76 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index f9169077ae..36d0b24633 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -96,13 +96,6 @@ library enabled as a cryptography provider.
Neither the ``nettle`` library, or the built-in cryptography provider are
supported on FIPS enabled hosts.
-``-writeconfig`` (since 6.0)
-'''''''''''''''''''''''''''''
-
-The ``-writeconfig`` option is not able to serialize the entire contents
-of the QEMU command line. It is thus considered a failed experiment
-and deprecated, with no current replacement.
-
Userspace local APIC with KVM (x86, since 6.0)
''''''''''''''''''''''''''''''''''''''''''''''
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index c21e6fa5ee..85a988ce2d 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -126,6 +126,12 @@ devices. Drives the board doesn't pick up can no longer be used with
This option was undocumented and not used in the field.
Use `-device usb-ccid`` instead.
+``-writeconfig`` (since 6.1)
+'''''''''''''''''''''''''''''
+
+The ``-writeconfig`` option was not able to serialize the entire contents
+of the QEMU command line. It is thus considered a failed experiment
+and removed without a replacement.
QEMU Machine Protocol (QMP) commands
------------------------------------
diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
index 8d3e53ae4d..9a44d2a77b 100644
--- a/include/qemu/config-file.h
+++ b/include/qemu/config-file.h
@@ -10,7 +10,6 @@ void qemu_add_opts(QemuOptsList *list);
void qemu_add_drive_opts(QemuOptsList *list);
int qemu_global_option(const char *str);
-void qemu_config_write(FILE *fp);
int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname,
Error **errp);
diff --git a/qemu-options.hx b/qemu-options.hx
index acd8b4f6f9..317d33ea11 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4395,18 +4395,14 @@ SRST
ERST
DEF("readconfig", HAS_ARG, QEMU_OPTION_readconfig,
- "-readconfig <file>\n", QEMU_ARCH_ALL)
+ "-readconfig <file>\n"
+ " read config file\n", QEMU_ARCH_ALL)
SRST
``-readconfig file``
Read device configuration from file. This approach is useful when
you want to spawn QEMU process with many command line options but
you don't want to exceed the command line character limit.
ERST
-DEF("writeconfig", HAS_ARG, QEMU_OPTION_writeconfig,
- "-writeconfig <file>\n"
- " read/write config file (deprecated)\n", QEMU_ARCH_ALL)
-SRST
-ERST
DEF("no-user-config", 0, QEMU_OPTION_nouserconfig,
"-no-user-config\n"
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 11ac3750d8..c7653fe788 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -3388,26 +3388,6 @@ void qemu_init(int argc, char **argv, char **envp)
}
display_remote++;
break;
- case QEMU_OPTION_writeconfig:
- {
- FILE *fp;
- warn_report("-writeconfig is deprecated and will go away without a replacement");
- if (strcmp(optarg, "-") == 0) {
- fp = stdout;
- } else {
- fp = fopen(optarg, "w");
- if (fp == NULL) {
- error_report("open %s: %s", optarg,
- strerror(errno));
- exit(1);
- }
- }
- qemu_config_write(fp);
- if (fp != stdout) {
- fclose(fp);
- }
- break;
- }
case QEMU_OPTION_qtest:
qtest_chrdev = optarg;
break;
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 670bd6ebca..9bd86aab1b 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -307,48 +307,6 @@ void qemu_add_opts(QemuOptsList *list)
abort();
}
-struct ConfigWriteData {
- QemuOptsList *list;
- FILE *fp;
-};
-
-static int config_write_opt(void *opaque, const char *name, const char *value,
- Error **errp)
-{
- struct ConfigWriteData *data = opaque;
-
- fprintf(data->fp, " %s = \"%s\"\n", name, value);
- return 0;
-}
-
-static int config_write_opts(void *opaque, QemuOpts *opts, Error **errp)
-{
- struct ConfigWriteData *data = opaque;
- const char *id = qemu_opts_id(opts);
-
- if (id) {
- fprintf(data->fp, "[%s \"%s\"]\n", data->list->name, id);
- } else {
- fprintf(data->fp, "[%s]\n", data->list->name);
- }
- qemu_opt_foreach(opts, config_write_opt, data, NULL);
- fprintf(data->fp, "\n");
- return 0;
-}
-
-void qemu_config_write(FILE *fp)
-{
- struct ConfigWriteData data = { .fp = fp };
- QemuOptsList **lists = vm_config_groups;
- int i;
-
- fprintf(fp, "# qemu config file\n\n");
- for (i = 0; lists[i] != NULL; i++) {
- data.list = lists[i];
- qemu_opts_foreach(data.list, config_write_opts, &data, NULL);
- }
-}
-
/* Returns number of config groups on success, -errno on error */
int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
{
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 02/14] qemu-config: parse configuration files to a QDict
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
2021-05-13 16:28 ` [PATCH 01/14] remove -writeconfig Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 03/14] vl: plumb keyval-based options into -set and -readconfig Paolo Bonzini
` (12 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, armbru
Change the parser to put the values into a QDict and pass them
to a callback. qemu_config_parse's QemuOpts creation is
itself turned into a callback function.
This is useful for -readconfig to support keyval-based options;
getting a QDict from the parser removes a roundtrip from
QDict to QemuOpts and then back to QDict.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/qemu/config-file.h | 6 ++-
softmmu/vl.c | 4 +-
util/qemu-config.c | 91 +++++++++++++++++++++++++-------------
3 files changed, 68 insertions(+), 33 deletions(-)
diff --git a/include/qemu/config-file.h b/include/qemu/config-file.h
index 9a44d2a77b..217a909053 100644
--- a/include/qemu/config-file.h
+++ b/include/qemu/config-file.h
@@ -1,6 +1,7 @@
#ifndef QEMU_CONFIG_FILE_H
#define QEMU_CONFIG_FILE_H
+typedef void QEMUConfigCB(const char *group, QDict *qdict, void *opaque, Error **errp);
QemuOptsList *qemu_find_opts(const char *group);
QemuOptsList *qemu_find_opts_err(const char *group, Error **errp);
@@ -13,7 +14,10 @@ int qemu_global_option(const char *str);
int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname,
Error **errp);
-int qemu_read_config_file(const char *filename, Error **errp);
+/* A default callback for qemu_read_config_file(). */
+void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp);
+
+int qemu_read_config_file(const char *filename, QEMUConfigCB *f, Error **errp);
/* Parse QDict options as a replacement for a config file (allowing multiple
enumerated (0..(n-1)) configuration "sections") */
diff --git a/softmmu/vl.c b/softmmu/vl.c
index c7653fe788..598d064a8e 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2121,7 +2121,7 @@ static void qemu_read_default_config_file(Error **errp)
int ret;
g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf");
- ret = qemu_read_config_file(file, errp);
+ ret = qemu_read_config_file(file, qemu_config_do_parse, errp);
if (ret < 0) {
if (ret == -ENOENT) {
error_free(*errp);
@@ -3370,7 +3370,7 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_plugin_opt_parse(optarg, &plugin_list);
break;
case QEMU_OPTION_readconfig:
- qemu_read_config_file(optarg, &error_fatal);
+ qemu_read_config_file(optarg, qemu_config_do_parse, &error_fatal);
break;
case QEMU_OPTION_spice:
olist = qemu_find_opts_err("spice", NULL);
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 9bd86aab1b..22eebca3a8 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -308,19 +308,19 @@ void qemu_add_opts(QemuOptsList *list)
}
/* Returns number of config groups on success, -errno on error */
-int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
+static int qemu_config_foreach(FILE *fp, QEMUConfigCB *cb, void *opaque,
+ const char *fname, Error **errp)
{
- char line[1024], group[64], id[64], arg[64], value[1024];
+ char line[1024], prev_group[64], group[64], arg[64], value[1024];
Location loc;
- QemuOptsList *list = NULL;
Error *local_err = NULL;
- QemuOpts *opts = NULL;
+ QDict *qdict = NULL;
int res = -EINVAL, lno = 0;
int count = 0;
loc_push_none(&loc);
while (fgets(line, sizeof(line), fp) != NULL) {
- loc_set_file(fname, ++lno);
+ ++lno;
if (line[0] == '\n') {
/* skip empty lines */
continue;
@@ -329,39 +329,39 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error *
/* comment */
continue;
}
- if (sscanf(line, "[%63s \"%63[^\"]\"]", group, id) == 2) {
- /* group with id */
- list = find_list(lists, group, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
+ if (line[0] == '[') {
+ QDict *prev = qdict;
+ if (sscanf(line, "[%63s \"%63[^\"]\"]", group, value) == 2) {
+ qdict = qdict_new();
+ qdict_put_str(qdict, "id", value);
+ count++;
+ } else if (sscanf(line, "[%63[^]]]", group) == 1) {
+ qdict = qdict_new();
+ count++;
}
- opts = qemu_opts_create(list, id, 1, NULL);
- count++;
- continue;
- }
- if (sscanf(line, "[%63[^]]]", group) == 1) {
- /* group without id */
- list = find_list(lists, group, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
- goto out;
+ if (qdict != prev) {
+ if (prev) {
+ cb(prev_group, prev, opaque, &local_err);
+ qobject_unref(prev);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ goto out;
+ }
+ }
+ strcpy(prev_group, group);
+ continue;
}
- opts = qemu_opts_create(list, NULL, 0, &error_abort);
- count++;
- continue;
}
+ loc_set_file(fname, lno);
value[0] = '\0';
if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2 ||
sscanf(line, " %63s = \"\"", arg) == 1) {
/* arg = value */
- if (opts == NULL) {
+ if (qdict == NULL) {
error_setg(errp, "no group defined");
goto out;
}
- if (!qemu_opt_set(opts, arg, value, errp)) {
- goto out;
- }
+ qdict_put_str(qdict, arg, value);
continue;
}
error_setg(errp, "parse error");
@@ -375,10 +375,41 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error *
res = count;
out:
loc_pop(&loc);
+ if (qdict) {
+ cb(group, qdict, opaque, errp);
+ qobject_unref(qdict);
+ }
return res;
}
-int qemu_read_config_file(const char *filename, Error **errp)
+void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp)
+{
+ QemuOptsList **lists = opaque;
+ const char *id = qdict_get_try_str(qdict, "id");
+ QemuOptsList *list;
+ QemuOpts *opts;
+
+ list = find_list(lists, group, errp);
+ if (!list) {
+ return;
+ }
+
+ opts = qemu_opts_create(list, id, 1, errp);
+ if (!opts) {
+ return;
+ }
+ if (id) {
+ qdict_del(qdict, "id");
+ }
+ qemu_opts_absorb_qdict(opts, qdict, errp);
+}
+
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
+{
+ return qemu_config_foreach(fp, qemu_config_do_parse, lists, fname, errp);
+}
+
+int qemu_read_config_file(const char *filename, QEMUConfigCB *cb, Error **errp)
{
FILE *f = fopen(filename, "r");
int ret;
@@ -388,7 +419,7 @@ int qemu_read_config_file(const char *filename, Error **errp)
return -errno;
}
- ret = qemu_config_parse(f, vm_config_groups, filename, errp);
+ ret = qemu_config_foreach(f, cb, vm_config_groups, filename, errp);
fclose(f);
return ret;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 03/14] vl: plumb keyval-based options into -set and -readconfig
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
2021-05-13 16:28 ` [PATCH 01/14] remove -writeconfig Paolo Bonzini
2021-05-13 16:28 ` [PATCH 02/14] qemu-config: parse configuration files to a QDict Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 04/14] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
` (11 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, armbru
Let -set and -readconfig support parsing command line options with
keyval either QemuOpts. The underlying data structure is QDict
for command line options that opt into keyval-based parsing.
This patch introduces a keyval_merge function that merges two
keyval-produced (or keyval-like) QDicts. Right now it is used for -set,
which is slightly heavyweight; later, it will be used also to emulate
the behavior of .merge_lists = true QemuOpts groups, merging -readconfig
sections and command-line options in a single QDict.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
include/block/qdict.h | 2 -
include/qapi/qmp/qdict.h | 3 +
include/qemu/option.h | 1 +
softmmu/vl.c | 146 +++++++++++++++++++++++++++++++++------
tests/unit/test-keyval.c | 56 +++++++++++++++
util/keyval.c | 47 +++++++++++++
6 files changed, 232 insertions(+), 23 deletions(-)
diff --git a/include/block/qdict.h b/include/block/qdict.h
index d8cb502d7d..ced2acfb92 100644
--- a/include/block/qdict.h
+++ b/include/block/qdict.h
@@ -20,8 +20,6 @@ void qdict_join(QDict *dest, QDict *src, bool overwrite);
void qdict_extract_subqdict(QDict *src, QDict **dst, const char *start);
void qdict_array_split(QDict *src, QList **dst);
int qdict_array_entries(QDict *src, const char *subqdict);
-QObject *qdict_crumple(const QDict *src, Error **errp);
-void qdict_flatten(QDict *qdict);
typedef struct QDictRenames {
const char *from;
diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h
index 9934539c1b..d5b5430e21 100644
--- a/include/qapi/qmp/qdict.h
+++ b/include/qapi/qmp/qdict.h
@@ -64,4 +64,7 @@ const char *qdict_get_try_str(const QDict *qdict, const char *key);
QDict *qdict_clone_shallow(const QDict *src);
+QObject *qdict_crumple(const QDict *src, Error **errp);
+void qdict_flatten(QDict *qdict);
+
#endif /* QDICT_H */
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/softmmu/vl.c b/softmmu/vl.c
index 598d064a8e..d60813770b 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -121,6 +121,7 @@
#include "qapi/qapi-commands-misc.h"
#include "qapi/qapi-visit-qom.h"
#include "qapi/qapi-commands-ui.h"
+#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qerror.h"
#include "sysemu/iothread.h"
#include "qemu/guest-random.h"
@@ -140,6 +141,14 @@ typedef struct ObjectOption {
QTAILQ_ENTRY(ObjectOption) next;
} ObjectOption;
+typedef struct KeyvalConfigEntry {
+ QDict *dict;
+ bool from_json;
+ QTAILQ_ENTRY(KeyvalConfigEntry) next;
+} KeyvalConfigEntry;
+
+typedef QTAILQ_HEAD(, KeyvalConfigEntry) KeyvalConfigGroup;
+
static const char *cpu_option;
static const char *mem_path;
static const char *incoming;
@@ -2115,13 +2124,98 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
return 0;
}
+/*
+ * Return whether configuration group @group is stored in QemuOpts or
+ * in a list of QDicts.
+ */
+static bool is_qemuopts_group(const char *group)
+{
+ return true;
+}
+
+/*
+ * Return a pointer to a list of QDicts, used to store options for
+ * "-set GROUP.*".
+ */
+static KeyvalConfigGroup *qemu_config_list(const char *group)
+{
+ return NULL;
+}
+
+/*
+ * Return a pointer to a QDict inside the list starting at @head,
+ * used to store options for "-GROUP id=...".
+ */
+static KeyvalConfigEntry *qemu_find_config(KeyvalConfigGroup *head, const char *id)
+{
+ KeyvalConfigEntry *e;
+ assert(id);
+ QTAILQ_FOREACH(e, head, next) {
+ if (g_strcmp0(qdict_get_try_str(e->dict, "id"), id) == 0) {
+ return e;
+ }
+ }
+ return NULL;
+}
+
+static void qemu_record_config_group(const char *group, QDict *dict, bool from_json, Error **errp)
+{
+ KeyvalConfigGroup *head;
+
+ head = qemu_config_list(group);
+ if (head) {
+ KeyvalConfigEntry *e = g_new(KeyvalConfigEntry, 1);
+ e->dict = dict;
+ e->from_json = from_json;
+ QTAILQ_INSERT_HEAD(head, e, next);
+ } else {
+ abort();
+ }
+}
+
+static void qemu_set_qdict_option(QDict *dict, const char *key, const char *value,
+ Error **errp)
+{
+ QDict *merge_dict;
+
+ merge_dict = qdict_new();
+ qdict_put_str(merge_dict, key, value);
+ keyval_merge(dict, merge_dict, errp);
+ qobject_unref(merge_dict);
+}
+
+/*
+ * Parse non-QemuOpts config file groups, pass the rest to
+ * qemu_config_do_parse.
+ */
+static void qemu_parse_config_group(const char *group, QDict *qdict,
+ void *opaque, Error **errp)
+{
+ QObject *crumpled;
+ if (is_qemuopts_group(group)) {
+ qemu_config_do_parse(group, qdict, opaque, errp);
+ return;
+ }
+
+ crumpled = qdict_crumple(qdict, errp);
+ if (!crumpled) {
+ return;
+ }
+ if (qobject_type(crumpled) != QTYPE_QDICT) {
+ assert(qobject_type(crumpled) == QTYPE_QLIST);
+ error_setg(errp, "Lists cannot be at top level of a configuration section");
+ return;
+ }
+ qemu_record_config_group(group, qobject_to(QDict, crumpled), false, errp);
+}
+
static void qemu_read_default_config_file(Error **errp)
{
ERRP_GUARD();
int ret;
g_autofree char *file = get_relocated_path(CONFIG_QEMU_CONFDIR "/qemu.conf");
- ret = qemu_read_config_file(file, qemu_config_do_parse, errp);
+ ret = qemu_read_config_file(file, qemu_parse_config_group, errp);
if (ret < 0) {
if (ret == -ENOENT) {
error_free(*errp);
@@ -2130,37 +2224,48 @@ static void qemu_read_default_config_file(Error **errp)
}
}
-static int qemu_set_option(const char *str)
+static void qemu_set_option(const char *str, Error **errp)
{
- Error *local_err = NULL;
char group[64], id[64], arg[64];
QemuOptsList *list;
QemuOpts *opts;
+ KeyvalConfigGroup *head;
int rc, offset;
rc = sscanf(str, "%63[^.].%63[^.].%63[^=]%n", group, id, arg, &offset);
if (rc < 3 || str[offset] != '=') {
- error_report("can't parse: \"%s\"", str);
- return -1;
+ error_setg(errp, "can't parse: \"%s\"", str);
+ return;
}
- list = qemu_find_opts(group);
- if (list == NULL) {
- return -1;
+ head = qemu_config_list(group);
+ if (head) {
+ KeyvalConfigEntry *e = qemu_find_config(head, id);
+ if (!e) {
+ goto not_found;
+ }
+ if (e->from_json) {
+ error_setg(errp, "cannot use -set with %s \"%s\" passed as JSON", group, id);
+ return;
+ }
+ qemu_set_qdict_option(e->dict, arg, str + offset + 1, errp);
+ return;
}
- opts = qemu_opts_find(list, id);
- if (!opts) {
- error_report("there is no %s \"%s\" defined",
- list->name, id);
- return -1;
+ list = qemu_find_opts_err(group, errp);
+ if (list) {
+ opts = qemu_opts_find(list, id);
+ if (!opts) {
+ goto not_found;
+ }
+ qemu_opt_set(opts, arg, str + offset + 1, errp);
+ return;
}
- if (!qemu_opt_set(opts, arg, str + offset + 1, &local_err)) {
- error_report_err(local_err);
- return -1;
- }
- return 0;
+ return;
+
+not_found:
+ error_setg(errp, "there is no %s \"%s\" defined", group, id);
}
static void user_register_global_props(void)
@@ -2737,8 +2842,7 @@ void qemu_init(int argc, char **argv, char **envp)
}
break;
case QEMU_OPTION_set:
- if (qemu_set_option(optarg) != 0)
- exit(1);
+ qemu_set_option(optarg, &error_fatal);
break;
case QEMU_OPTION_global:
if (qemu_global_option(optarg) != 0)
@@ -3370,7 +3474,7 @@ void qemu_init(int argc, char **argv, char **envp)
qemu_plugin_opt_parse(optarg, &plugin_list);
break;
case QEMU_OPTION_readconfig:
- qemu_read_config_file(optarg, qemu_config_do_parse, &error_fatal);
+ qemu_read_config_file(optarg, qemu_parse_config_group, &error_fatal);
break;
case QEMU_OPTION_spice:
olist = qemu_find_opts_err("spice", NULL);
diff --git a/tests/unit/test-keyval.c b/tests/unit/test-keyval.c
index e20c07cf3e..254b51e98c 100644
--- a/tests/unit/test-keyval.c
+++ b/tests/unit/test-keyval.c
@@ -747,6 +747,59 @@ static void test_keyval_visit_any(void)
visit_free(v);
}
+static void test_keyval_merge_success(void)
+{
+ QDict *old = keyval_parse("opt1=abc,opt2.sub1=def,opt2.sub2=ghi,opt3=xyz",
+ NULL, NULL, &error_abort);
+ QDict *new = 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(old, new, &err);
+ g_assert(!err);
+ g_assert(qobject_is_equal(QOBJECT(combined), QOBJECT(old)));
+ qobject_unref(old);
+ qobject_unref(new);
+ qobject_unref(combined);
+}
+
+static void test_keyval_merge_list(void)
+{
+ QDict *old = keyval_parse("opt1.0=abc,opt2.0=xyz",
+ NULL, NULL, &error_abort);
+ QDict *new = 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(old, new, &err);
+ g_assert(!err);
+ g_assert(qobject_is_equal(QOBJECT(combined), QOBJECT(old)));
+ qobject_unref(old);
+ qobject_unref(new);
+ qobject_unref(combined);
+}
+
+static void test_keyval_merge_conflict(void)
+{
+ QDict *old = keyval_parse("opt2.sub1=def,opt2.sub2=ghi",
+ NULL, NULL, &error_abort);
+ QDict *new = keyval_parse("opt2=ABC",
+ NULL, NULL, &error_abort);
+ Error *err = NULL;
+
+ keyval_merge(new, old, &err);
+ error_free_or_abort(&err);
+ keyval_merge(old, new, &err);
+ error_free_or_abort(&err);
+
+ qobject_unref(old);
+ qobject_unref(new);
+}
+
int main(int argc, char *argv[])
{
g_test_init(&argc, &argv, NULL);
@@ -760,6 +813,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/success", test_keyval_merge_success);
+ 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..0797f36e1d 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -310,6 +310,53 @@ static char *reassemble_key(GSList *key)
return g_string_free(s, FALSE);
}
+/* Merge two dictionaries. */
+static void keyval_do_merge(QDict *old, const QDict *new, GString *str, Error **errp)
+{
+ size_t save_len = str->len;
+ const QDictEntry *ent;
+ QObject *old_value;
+
+ for (ent = qdict_first(new); ent; ent = qdict_next(new, ent)) {
+ old_value = qdict_get(old, 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;
+ }
+ }
+
+ qobject_ref(ent->value);
+ qdict_put_obj(old, ent->key, ent->value);
+ }
+}
+
+void keyval_merge(QDict *old, const QDict *new, Error **errp)
+{
+ GString *str = g_string_new("");
+ keyval_do_merge(old, new, str, errp);
+ g_string_free(str, TRUE);
+}
+
/*
* Listify @cur recursively.
* Replace QDicts whose keys are all valid list indexes by QLists.
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 04/14] qom: export more functions for use with non-UserCreatable objects
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (2 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 03/14] vl: plumb keyval-based options into -set and -readconfig Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 05/14] keyval: introduce keyval_parse_into Paolo Bonzini
` (10 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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.
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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 05/14] keyval: introduce keyval_parse_into
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (3 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 04/14] qom: export more functions for use with non-UserCreatable objects Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 06/14] vl: switch -M parsing to keyval Paolo Bonzini
` (9 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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.
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 0797f36e1d..1ffd6e1204 100644
--- a/util/keyval.c
+++ b/util/keyval.c
@@ -478,13 +478,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;
@@ -493,7 +494,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;
@@ -503,15 +503,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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 06/14] vl: switch -M parsing to keyval
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (4 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 05/14] keyval: introduce keyval_parse_into Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 07/14] qemu-option: remove now-dead code Paolo Bonzini
` (8 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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 | 303 ++++++++++++++++++++++++---------------------------
1 file changed, 141 insertions(+), 162 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index d60813770b..6b670fb301 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -153,6 +153,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;
@@ -243,21 +245,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",
@@ -506,16 +493,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;
@@ -823,33 +800,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"
@@ -1540,33 +1490,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;
@@ -1619,32 +1567,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;
-
- loc_push_none(&loc);
-
- opts = qemu_get_machine_opts();
- qemu_opts_loc_restore(opts);
+ MachineClass *machine_class;
+ Error *local_err = NULL;
- optarg = qemu_opt_get(opts, "type");
if (optarg) {
- machine_class = machine_parse(optarg, machines);
- }
-
- 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);
+ 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");
+ }
}
- 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;
}
@@ -1663,42 +1610,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;
+ value = qdict_get_try_str(qdict, "accel");
+ if (value) {
+ accelerators = g_strdup(value);
+ qdict_del(qdict, "accel");
}
- if (g_str_equal(qom_name, "igd-passthru")) {
- object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), qom_name, value,
+
+ value = qdict_get_try_str(qdict, "igd-passthru");
+ if (value) {
+ object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", value,
false);
- return 0;
+ qdict_del(qdict, "igd-passthru");
}
- if (g_str_equal(qom_name, "kvm-shadow-mem")) {
- object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value,
+
+ value = qdict_get_try_str(qdict, "kvm-shadow-mem");
+ if (value) {
+ object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kvm-shadow-mem", value,
false);
- return 0;
+ qdict_del(qdict, "kvm-shadow-mem");
}
- if (g_str_equal(qom_name, "kernel-irqchip")) {
- object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), qom_name, value,
+
+ 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"), qom_name, value,
+ object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), "kernel-irqchip", value,
false);
- return 0;
+ qdict_del(qdict, "kernel-irqchip");
}
-
- return object_parse_property_opt(opaque, name, value, "type", errp);
}
static void object_option_foreach_add(bool (*type_opt_predicate)(const char *))
@@ -1810,16 +1785,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;
@@ -1848,10 +1821,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);
}
}
@@ -1898,8 +1869,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();
@@ -2067,16 +2037,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),
@@ -2107,8 +2075,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);
}
}
@@ -2130,6 +2102,9 @@ static int global_init_func(void *opaque, QemuOpts *opts, Error **errp)
*/
static bool is_qemuopts_group(const char *group)
{
+ if (g_str_equal(group, "machine")) {
+ return false;
+ }
return true;
}
@@ -2168,6 +2143,13 @@ static void qemu_record_config_group(const char *group, QDict *dict, bool from_j
e->dict = dict;
e->from_json = from_json;
QTAILQ_INSERT_HEAD(head, e, next);
+ } 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();
}
@@ -2325,13 +2307,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;
@@ -2419,12 +2399,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) {
@@ -2747,7 +2726,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);
@@ -2788,6 +2766,7 @@ void qemu_init(int argc, char **argv, char **envp)
}
}
+ machine_opts_dict = qdict_new();
if (userconfig) {
qemu_read_default_config_file(&error_fatal);
}
@@ -2877,8 +2856,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;
@@ -2902,16 +2880,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);
@@ -3021,7 +2999,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;
@@ -3290,17 +3268,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);
@@ -3327,12 +3308,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:
@@ -3351,12 +3330,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");
@@ -3593,7 +3570,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();
/*
@@ -3626,7 +3603,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();
@@ -3634,12 +3611,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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 07/14] qemu-option: remove now-dead code
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (5 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 06/14] vl: switch -M parsing to keyval Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 08/14] vl: switch -accel parsing to keyval Paolo Bonzini
` (7 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 08/14] vl: switch -accel parsing to keyval
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (6 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 07/14] qemu-option: remove now-dead code Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 09/14] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
` (6 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, armbru
Switch from QemuOpts to keyval. This enables compound options
for accelerators as well as passing the options as JSON, using
for example -accel '{"accel":"kvm"}'.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
accel/accel-softmmu.c | 6 ++
include/qemu/accel.h | 1 +
softmmu/vl.c | 156 ++++++++++++++++++++++--------------------
3 files changed, 87 insertions(+), 76 deletions(-)
diff --git a/accel/accel-softmmu.c b/accel/accel-softmmu.c
index 50fa5acaa4..37717a3757 100644
--- a/accel/accel-softmmu.c
+++ b/accel/accel-softmmu.c
@@ -30,6 +30,12 @@
#include "accel-softmmu.h"
+bool accel_print_class_properties(const char *opt_name)
+{
+ g_autofree char *class_name = g_strdup_printf(ACCEL_CLASS_NAME("%s"), opt_name);
+ return type_print_class_properties(class_name);
+}
+
int accel_init_machine(AccelState *accel, MachineState *ms)
{
AccelClass *acc = ACCEL_GET_CLASS(accel);
diff --git a/include/qemu/accel.h b/include/qemu/accel.h
index 4f4c283f6f..2c712d3846 100644
--- a/include/qemu/accel.h
+++ b/include/qemu/accel.h
@@ -70,6 +70,7 @@ AccelClass *accel_find(const char *opt_name);
AccelState *current_accel(void);
void accel_init_interfaces(AccelClass *ac);
+bool accel_print_class_properties(const char *opt_name);
#ifndef CONFIG_USER_ONLY
int accel_init_machine(AccelState *accel, MachineState *ms);
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 6b670fb301..de844f08d7 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -155,6 +155,7 @@ static const char *incoming;
static const char *loadvm;
static const char *accelerators;
static QDict *machine_opts_dict;
+static KeyvalConfigGroup accel_opts_list;
static QTAILQ_HEAD(, ObjectOption) object_opts = QTAILQ_HEAD_INITIALIZER(object_opts);
static ram_addr_t maxram_size;
static uint64_t ram_slots;
@@ -245,20 +246,6 @@ static QemuOptsList qemu_option_rom_opts = {
},
};
-static QemuOptsList qemu_accel_opts = {
- .name = "accel",
- .implied_opt_name = "accel",
- .head = QTAILQ_HEAD_INITIALIZER(qemu_accel_opts.head),
- .desc = {
- /*
- * no elements => accept any
- * sanity checking will happen later
- * when setting accelerator properties
- */
- { }
- },
-};
-
static QemuOptsList qemu_boot_opts = {
.name = "boot-opts",
.implied_opt_name = "order",
@@ -1595,21 +1582,6 @@ static MachineClass *select_machine(QDict *qdict, Error **errp)
return machine_class;
}
-static int object_parse_property_opt(Object *obj,
- const char *name, const char *value,
- const char *skip, Error **errp)
-{
- if (g_str_equal(name, skip)) {
- return 0;
- }
-
- if (!object_property_parse(obj, name, value, errp)) {
- return -1;
- }
-
- return 0;
-}
-
/* *Non*recursively replace underscores with dashes in QDict keys. */
static void keyval_dashify(QDict *qdict, Error **errp)
{
@@ -2102,7 +2074,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, "machine")) {
+ if (g_str_equal(group, "machine") ||
+ g_str_equal(group, "accel")) {
return false;
}
return true;
@@ -2114,6 +2087,9 @@ static bool is_qemuopts_group(const char *group)
*/
static KeyvalConfigGroup *qemu_config_list(const char *group)
{
+ if (g_str_equal(group, "accel")) {
+ return &accel_opts_list;
+ }
return NULL;
}
@@ -2155,6 +2131,26 @@ static void qemu_record_config_group(const char *group, QDict *dict, bool from_j
}
}
+static QDict *qemu_parse_config_option(const char *group, const char *arg,
+ bool *help, Error **errp)
+{
+ QDict *args;
+
+ *help = false;
+ if (arg[0] == '{') {
+ args = (QDict*) qobject_from_json(arg, errp);
+ if (args) {
+ qemu_record_config_group(group, args, true, &error_abort);
+ }
+ } else {
+ args = keyval_parse(arg, group, help, errp);
+ if (args) {
+ qemu_record_config_group(group, args, false, &error_abort);
+ }
+ }
+ return args;
+}
+
static void qemu_set_qdict_option(QDict *dict, const char *key, const char *value,
Error **errp)
{
@@ -2262,17 +2258,9 @@ static int do_configure_icount(void *opaque, QemuOpts *opts, Error **errp)
return 0;
}
-static int accelerator_set_property(void *opaque,
- const char *name, const char *value,
- Error **errp)
-{
- return object_parse_property_opt(opaque, name, value, "accel", errp);
-}
-
-static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
+static bool do_configure_accelerator(KeyvalConfigEntry *e)
{
- bool *p_init_failed = opaque;
- const char *acc = qemu_opt_get(opts, "accel");
+ const char *acc = qdict_get_try_str(e->dict, "accel");
AccelClass *ac = accel_find(acc);
AccelState *accel;
int ret;
@@ -2281,38 +2269,36 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp)
qtest_with_kvm = g_str_equal(acc, "kvm") && qtest_chrdev != NULL;
if (!ac) {
- *p_init_failed = true;
if (!qtest_with_kvm) {
error_report("invalid accelerator %s", acc);
}
- return 0;
+ return true;
}
accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac)));
object_apply_compat_props(OBJECT(accel));
- qemu_opt_foreach(opts, accelerator_set_property,
- accel,
- &error_fatal);
+ qdict_del(e->dict, "accel");
+ object_set_properties_from_keyval(OBJECT(accel), e->dict, e->from_json, &error_fatal);
ret = accel_init_machine(accel, current_machine);
if (ret < 0) {
- *p_init_failed = true;
if (!qtest_with_kvm || ret != -ENOENT) {
- error_report("failed to initialize %s: %s", acc, strerror(-ret));
+ error_report("failed to initialize %s: %s", ac->name, strerror(-ret));
}
- return 0;
+ return true;
}
- return 1;
+ return false;
}
static void configure_accelerators(const char *progname)
{
bool init_failed = false;
+ KeyvalConfigEntry *accel;
qemu_opts_foreach(qemu_find_opts("icount"),
do_configure_icount, NULL, &error_fatal);
- if (QTAILQ_EMPTY(&qemu_accel_opts.head)) {
+ if (QTAILQ_EMPTY(&accel_opts_list)) {
char **accel_list, **tmp;
if (accelerators == NULL) {
@@ -2345,7 +2331,9 @@ static void configure_accelerators(const char *progname)
* such as "-machine accel=tcg,,thread=single".
*/
if (accel_find(*tmp)) {
- qemu_opts_parse_noisily(qemu_find_opts("accel"), *tmp, true);
+ QDict *qdict = qdict_new();
+ qdict_put_str(qdict, "accel", *tmp);
+ qemu_record_config_group("accel", qdict, true, &error_abort);
} else {
init_failed = true;
error_report("invalid accelerator %s", *tmp);
@@ -2359,8 +2347,14 @@ static void configure_accelerators(const char *progname)
}
}
- if (!qemu_opts_foreach(qemu_find_opts("accel"),
- do_configure_accelerator, &init_failed, &error_fatal)) {
+ QTAILQ_FOREACH_REVERSE(accel, &accel_opts_list, next) {
+ init_failed = do_configure_accelerator(accel);
+ if (current_accel()) {
+ break;
+ }
+ }
+
+ if (!current_accel()) {
if (!init_failed) {
error_report("no accelerator found");
}
@@ -2378,6 +2372,27 @@ static void configure_accelerators(const char *progname)
}
}
+static void list_accelerators(void)
+{
+ printf("Accelerators supported in QEMU binary:\n");
+ GSList *el, *accel_list = object_class_get_list(TYPE_ACCEL,
+ false);
+ for (el = accel_list; el; el = el->next) {
+ gchar *typename = g_strdup(object_class_get_name(
+ OBJECT_CLASS(el->data)));
+ /* omit qtest which is used for tests only */
+ if (g_strcmp0(typename, ACCEL_CLASS_NAME("qtest")) &&
+ g_str_has_suffix(typename, ACCEL_CLASS_SUFFIX)) {
+ gchar **optname = g_strsplit(typename,
+ ACCEL_CLASS_SUFFIX, 0);
+ printf("%s\n", optname[0]);
+ g_strfreev(optname);
+ }
+ g_free(typename);
+ }
+ g_slist_free(accel_list);
+}
+
static void create_default_memdev(MachineState *ms, const char *path)
{
Object *obj;
@@ -2702,7 +2717,7 @@ void qmp_x_exit_preconfig(Error **errp)
void qemu_init(int argc, char **argv, char **envp)
{
QemuOpts *opts;
- QemuOpts *icount_opts = NULL, *accel_opts = NULL;
+ QemuOpts *icount_opts = NULL;
QemuOptsList *olist;
int optind;
const char *optarg;
@@ -2726,7 +2741,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_accel_opts);
qemu_add_opts(&qemu_mem_opts);
qemu_add_opts(&qemu_smp_opts);
qemu_add_opts(&qemu_boot_opts);
@@ -3283,30 +3297,20 @@ void qemu_init(int argc, char **argv, char **envp)
break;
}
case QEMU_OPTION_accel:
- accel_opts = qemu_opts_parse_noisily(qemu_find_opts("accel"),
- optarg, true);
- optarg = qemu_opt_get(accel_opts, "accel");
- if (!optarg || is_help_option(optarg)) {
- printf("Accelerators supported in QEMU binary:\n");
- GSList *el, *accel_list = object_class_get_list(TYPE_ACCEL,
- false);
- for (el = accel_list; el; el = el->next) {
- gchar *typename = g_strdup(object_class_get_name(
- OBJECT_CLASS(el->data)));
- /* omit qtest which is used for tests only */
- if (g_strcmp0(typename, ACCEL_CLASS_NAME("qtest")) &&
- g_str_has_suffix(typename, ACCEL_CLASS_SUFFIX)) {
- gchar **optname = g_strsplit(typename,
- ACCEL_CLASS_SUFFIX, 0);
- printf("%s\n", optname[0]);
- g_strfreev(optname);
+ {
+ bool help;
+ QDict *args;
+
+ args = qemu_parse_config_option("accel", optarg, &help, &error_fatal);
+ if (help) {
+ const char *type = qdict_get_try_str(args, "accel");
+ if (!type || !accel_print_class_properties(type)) {
+ list_accelerators();
}
- g_free(typename);
+ exit(0);
}
- g_slist_free(accel_list);
- exit(0);
+ break;
}
- break;
case QEMU_OPTION_usb:
qdict_put_str(machine_opts_dict, "usb", "on");
break;
--
2.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 09/14] machine: move dies from X86MachineState to CpuTopology
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (7 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 08/14] vl: switch -accel parsing to keyval Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 10/14] machine: move common smp_parse code to caller Paolo Bonzini
` (5 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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.
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 1bf0e687b9..79efae89ce 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -969,6 +969,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 8cfaf216e7..466e57acbe 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -708,8 +708,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);
@@ -765,7 +763,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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 10/14] machine: move common smp_parse code to caller
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (8 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 09/14] machine: move dies from X86MachineState to CpuTopology Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 11/14] machine: add error propagation to mc->smp_parse Paolo Bonzini
` (4 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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 | 112 ++++++++++++++++++++++----------------------
hw/i386/pc.c | 116 +++++++++++++++++++++-------------------------
2 files changed, 110 insertions(+), 118 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 79efae89ce..7e57c287b0 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -740,67 +740,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);
-
- /* 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);
- }
-
- 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);
+ 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);
+ }
- 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.max_cpus =
+ qemu_opt_get_number(opts, "maxcpus", cpus);
- ms->smp.cpus = cpus;
- ms->smp.cores = cores;
- ms->smp.threads = threads;
- ms->smp.sockets = sockets;
+ if (ms->smp.max_cpus < cpus) {
+ error_report("maxcpus must be equal to or greater than smp");
+ exit(1);
}
- if (ms->smp.cpus > 1) {
- Error *blocker = NULL;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
- replay_add_blocker(blocker);
+ 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)
@@ -1134,7 +1126,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) {
@@ -1150,6 +1144,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 466e57acbe..2942ddf0f9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -708,69 +708,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);
-
- /* 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);
- }
-
- 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);
+ 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);
}
-
- ms->smp.cpus = cpus;
- ms->smp.cores = cores;
- ms->smp.threads = threads;
- ms->smp.sockets = sockets;
- ms->smp.dies = dies;
- }
-
- if (ms->smp.cpus > 1) {
- Error *blocker = NULL;
- error_setg(&blocker, QERR_REPLAY_NOT_SUPPORTED, "smp");
- replay_add_blocker(blocker);
- }
+ } 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);
+ }
+
+ 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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 11/14] machine: add error propagation to mc->smp_parse
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (9 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 10/14] machine: move common smp_parse code to caller Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:28 ` [PATCH 12/14] machine: pass QAPI struct " Paolo Bonzini
` (3 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, armbru
Clean up the smp_parse functions to use Error** instead of exiting.
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 7e57c287b0..70c297a7de 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -738,7 +738,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);
@@ -765,28 +765,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;
@@ -1125,9 +1125,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 2942ddf0f9..1760e94ff5 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -706,7 +706,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);
@@ -734,28 +734,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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 12/14] machine: pass QAPI struct to mc->smp_parse
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (10 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 11/14] machine: add error propagation to mc->smp_parse Paolo Bonzini
@ 2021-05-13 16:28 ` Paolo Bonzini
2021-05-13 16:29 ` [PATCH 13/14] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
` (2 subsequent siblings)
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:28 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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.
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 70c297a7de..7daca0b86a 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -738,12 +738,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) {
@@ -753,8 +753,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) {
@@ -772,8 +771,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");
@@ -1128,7 +1126,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 1760e94ff5..14ed30bb78 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -706,13 +706,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) {
@@ -722,8 +722,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) {
@@ -741,8 +740,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 6e90d463fc..3a6a21fd01 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1274,3 +1274,30 @@
##
{ 'event': 'MEM_UNPLUG_ERROR',
'data': { 'device': 'str', 'msg': 'str' } }
+
+##
+# @SMPConfiguration:
+#
+# Schema for CPU topology configuration.
+#
+# @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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 13/14] machine: reject -smp dies!=1 for non-PC machines
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (11 preceding siblings ...)
2021-05-13 16:28 ` [PATCH 12/14] machine: pass QAPI struct " Paolo Bonzini
@ 2021-05-13 16:29 ` Paolo Bonzini
2021-05-13 16:29 ` [PATCH 14/14] machine: add smp compound property Paolo Bonzini
2021-05-13 16:57 ` [PATCH 00/14] vl: compound properties for machines and accelerators no-reply
14 siblings, 0 replies; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:29 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, armbru
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 7daca0b86a..55e878fc3e 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -745,6 +745,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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH 14/14] machine: add smp compound property
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (12 preceding siblings ...)
2021-05-13 16:29 ` [PATCH 13/14] machine: reject -smp dies!=1 for non-PC machines Paolo Bonzini
@ 2021-05-13 16:29 ` Paolo Bonzini
2021-05-17 14:58 ` Igor Mammedov
2021-05-13 16:57 ` [PATCH 00/14] vl: compound properties for machines and accelerators no-reply
14 siblings, 1 reply; 17+ messages in thread
From: Paolo Bonzini @ 2021-05-13 16:29 UTC (permalink / raw)
To: qemu-devel; +Cc: yang.zhong, berrange, ehabkost, 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 +++++++++++++++++++++-------------------
softmmu/vl.c | 33 +++++++++---
tests/qtest/numa-test.c | 22 ++++----
3 files changed, 95 insertions(+), 68 deletions(-)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 55e878fc3e..f33c9ce78c 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"
@@ -797,6 +798,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);
@@ -836,6 +888,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);
@@ -1124,56 +1182,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/softmmu/vl.c b/softmmu/vl.c
index de844f08d7..555385e64d 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1504,6 +1504,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;
@@ -1796,6 +1815,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)
@@ -2039,9 +2064,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.
@@ -3325,10 +3347,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.26.2
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH 14/14] machine: add smp compound property
2021-05-13 16:29 ` [PATCH 14/14] machine: add smp compound property Paolo Bonzini
@ 2021-05-17 14:58 ` Igor Mammedov
0 siblings, 0 replies; 17+ messages in thread
From: Igor Mammedov @ 2021-05-17 14:58 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: yang.zhong, armbru, berrange, qemu-devel, ehabkost
On Thu, 13 May 2021 12:29:01 -0400
Paolo Bonzini <pbonzini@redhat.com> wrote:
> 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 +++++++++++++++++++++-------------------
> softmmu/vl.c | 33 +++++++++---
> tests/qtest/numa-test.c | 22 ++++----
> 3 files changed, 95 insertions(+), 68 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index 55e878fc3e..f33c9ce78c 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"
> @@ -797,6 +798,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,
> + };
why did you choose to set all has_foo to true?
> + 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);
> @@ -836,6 +888,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);
> @@ -1124,56 +1182,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/softmmu/vl.c b/softmmu/vl.c
> index de844f08d7..555385e64d 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -1504,6 +1504,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;
>
> @@ -1796,6 +1815,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)
> @@ -2039,9 +2064,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.
> @@ -3325,10 +3347,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 "
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH 00/14] vl: compound properties for machines and accelerators
2021-05-13 16:28 [PATCH 00/14] vl: compound properties for machines and accelerators Paolo Bonzini
` (13 preceding siblings ...)
2021-05-13 16:29 ` [PATCH 14/14] machine: add smp compound property Paolo Bonzini
@ 2021-05-13 16:57 ` no-reply
14 siblings, 0 replies; 17+ messages in thread
From: no-reply @ 2021-05-13 16:57 UTC (permalink / raw)
To: pbonzini; +Cc: yang.zhong, armbru, berrange, qemu-devel, ehabkost
Patchew URL: https://patchew.org/QEMU/20210513162901.1310239-1-pbonzini@redhat.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Message-id: 20210513162901.1310239-1-pbonzini@redhat.com
Subject: [PATCH 00/14] vl: compound properties for machines and accelerators
=== 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/20210513162901.1310239-1-pbonzini@redhat.com -> patchew/20210513162901.1310239-1-pbonzini@redhat.com
Switched to a new branch 'test'
f11df8b machine: add smp compound property
056a1f3 machine: reject -smp dies!=1 for non-PC machines
f9a9ed1 machine: pass QAPI struct to mc->smp_parse
ccdb728 machine: add error propagation to mc->smp_parse
36dc3c3 machine: move common smp_parse code to caller
c547aa6 machine: move dies from X86MachineState to CpuTopology
4fa41d4 vl: switch -accel parsing to keyval
51375fd qemu-option: remove now-dead code
a91c274 vl: switch -M parsing to keyval
3bafa79 keyval: introduce keyval_parse_into
237070b qom: export more functions for use with non-UserCreatable objects
06ecbc4 vl: plumb keyval-based options into -set and -readconfig
105227f qemu-config: parse configuration files to a QDict
0e653b3 remove -writeconfig
=== OUTPUT BEGIN ===
1/14 Checking commit 0e653b38c051 (remove -writeconfig)
2/14 Checking commit 105227fb9596 (qemu-config: parse configuration files to a QDict)
WARNING: line over 80 characters
#31: FILE: include/qemu/config-file.h:4:
+typedef void QEMUConfigCB(const char *group, QDict *qdict, void *opaque, Error **errp);
WARNING: line over 80 characters
#41: FILE: include/qemu/config-file.h:18:
+void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp);
WARNING: line over 80 characters
#65: FILE: softmmu/vl.c:3372:
+ qemu_read_config_file(optarg, qemu_config_do_parse, &error_fatal);
WARNING: line over 80 characters
#173: FILE: util/qemu-config.c:385:
+void qemu_config_do_parse(const char *group, QDict *qdict, void *opaque, Error **errp)
WARNING: line over 80 characters
#195: FILE: util/qemu-config.c:407:
+int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname, Error **errp)
total: 0 errors, 5 warnings, 170 lines checked
Patch 2/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
3/14 Checking commit 06ecbc4ad0ba (vl: plumb keyval-based options into -set and -readconfig)
WARNING: line over 80 characters
#117: FILE: softmmu/vl.c:2148:
+static KeyvalConfigEntry *qemu_find_config(KeyvalConfigGroup *head, const char *id)
ERROR: line over 90 characters
#129: FILE: softmmu/vl.c:2160:
+static void qemu_record_config_group(const char *group, QDict *dict, bool from_json, Error **errp)
WARNING: line over 80 characters
#144: FILE: softmmu/vl.c:2175:
+static void qemu_set_qdict_option(QDict *dict, const char *key, const char *value,
WARNING: line over 80 characters
#174: FILE: softmmu/vl.c:2205:
+ error_setg(errp, "Lists cannot be at top level of a configuration section");
WARNING: line over 80 characters
#223: FILE: softmmu/vl.c:2247:
+ error_setg(errp, "cannot use -set with %s \"%s\" passed as JSON", group, id);
WARNING: line over 80 characters
#272: FILE: softmmu/vl.c:3476:
+ qemu_read_config_file(optarg, qemu_parse_config_group, &error_fatal);
ERROR: line over 90 characters
#290: 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
#359: FILE: util/keyval.c:314:
+static void keyval_do_merge(QDict *old, const QDict *new, GString *str, Error **errp)
ERROR: line over 90 characters
#369: FILE: util/keyval.c:324:
+ error_setg(errp, "Parameter '%s%s' used inconsistently", str->str, ent->key);
total: 3 errors, 6 warnings, 344 lines checked
Patch 3/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
4/14 Checking commit 237070ba2b31 (qom: export more functions for use with non-UserCreatable objects)
5/14 Checking commit 3bafa79a7738 (keyval: introduce keyval_parse_into)
WARNING: line over 80 characters
#26: FILE: include/qemu/option.h:150:
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
WARNING: line over 80 characters
#48: FILE: util/keyval.c:486:
+QDict *keyval_parse_into(QDict *qdict, const char *params, const char *implied_key,
total: 0 errors, 2 warnings, 78 lines checked
Patch 5/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/14 Checking commit a91c274e99f7 (vl: switch -M parsing to keyval)
WARNING: line over 80 characters
#206: FILE: softmmu/vl.c:1586:
+ error_setg(&local_err, "No machine specified, and there is no default");
WARNING: line over 80 characters
#219: FILE: softmmu/vl.c:1592:
+ error_append_hint(&local_err, "Use -machine help to list supported machines\n");
WARNING: line over 80 characters
#256: FILE: softmmu/vl.c:1633:
+ error_setg(errp, "Conflict between '%s' and '%s'", ent->key, new_key);
WARNING: line over 80 characters
#300: FILE: softmmu/vl.c:1657:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("xen"), "igd-passthru", value,
WARNING: line over 80 characters
#307: FILE: softmmu/vl.c:1664:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kvm-shadow-mem", value,
WARNING: line over 80 characters
#314: FILE: softmmu/vl.c:1671:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("kvm"), "kernel-irqchip", value,
WARNING: line over 80 characters
#316: FILE: softmmu/vl.c:1673:
+ object_register_sugar_prop(ACCEL_CLASS_NAME("whpx"), "kernel-irqchip", value,
ERROR: line over 90 characters
#338: FILE: softmmu/vl.c:1794:
+ object_set_properties_from_keyval(OBJECT(current_machine), qdict, false, &error_fatal);
ERROR: line over 90 characters
#350: FILE: softmmu/vl.c:1824:
+ semihosting_arg_fallback(current_machine->kernel_filename, current_machine->kernel_cmdline);
ERROR: code indent should never use tabs
#414: FILE: softmmu/vl.c:2147:
+^I * Cannot merge string-valued and type-safe dictionaries, so JSON$
ERROR: code indent should never use tabs
#415: FILE: softmmu/vl.c:2148:
+^I * is not accepted yet for -M.$
ERROR: code indent should never use tabs
#416: FILE: softmmu/vl.c:2149:
+^I */$
ERROR: line over 90 characters
#526: FILE: softmmu/vl.c:3277:
+ keyval_parse_into(machine_opts_dict, optarg, "type", &help, &error_fatal);
total: 6 errors, 7 warnings, 536 lines checked
Patch 6/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/14 Checking commit 51375fdf61c2 (qemu-option: remove now-dead code)
8/14 Checking commit 4fa41d4803b8 (vl: switch -accel parsing to keyval)
WARNING: line over 80 characters
#29: FILE: accel/accel-softmmu.c:35:
+ g_autofree char *class_name = g_strdup_printf(ACCEL_CLASS_NAME("%s"), opt_name);
ERROR: "(foo*)" should be "(foo *)"
#134: FILE: softmmu/vl.c:2140:
+ args = (QDict*) qobject_from_json(arg, errp);
WARNING: line over 80 characters
#187: FILE: softmmu/vl.c:2279:
+ object_set_properties_from_keyval(OBJECT(accel), e->dict, e->from_json, &error_fatal);
WARNING: line over 80 characters
#194: FILE: softmmu/vl.c:2284:
+ error_report("failed to initialize %s: %s", ac->name, strerror(-ret));
WARNING: line over 80 characters
#315: FILE: softmmu/vl.c:3303:
+ args = qemu_parse_config_option("accel", optarg, &help, &error_fatal);
total: 1 errors, 4 warnings, 285 lines checked
Patch 8/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/14 Checking commit c547aa618036 (machine: move dies from X86MachineState to CpuTopology)
10/14 Checking commit 36dc3c38ee59 (machine: move common smp_parse code to caller)
11/14 Checking commit ccdb728e4ee8 (machine: add error propagation to mc->smp_parse)
12/14 Checking commit f9a9ed171915 (machine: pass QAPI struct to mc->smp_parse)
WARNING: line over 80 characters
#95: FILE: hw/i386/pc.c:709:
+static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
total: 0 errors, 1 warnings, 133 lines checked
Patch 12/14 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
13/14 Checking commit 056a1f3633ba (machine: reject -smp dies!=1 for non-PC machines)
14/14 Checking commit f11df8b988ce (machine: add smp compound property)
ERROR: line over 90 characters
#223: FILE: softmmu/vl.c:3349:
+ machine_parse_property_opt(qemu_find_opts("smp-opts"), "smp", optarg, &error_fatal);
WARNING: line over 80 characters
#236: 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
#245: 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
#272: 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, 277 lines checked
Patch 14/14 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/20210513162901.1310239-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] 17+ messages in thread