Re: [PATCH 08/31] configure: ensure dependency for cross-compile setup
Il lun 25 set 2023, 18:45 Alex Bennée ha scritto: > Paolo Bonzini writes: > > On 9/25/23 16:48, Alex Bennée wrote: > >> echo "HOST_GDB_SUPPORTS_ARCH=y" >> "$config_target_mak" > >> fi > >> + echo "$config_target_mak: configure" >> Makefile.prereqs > > > > This in practice is not adding anything; if "configure" changes then > > Makefile's dependency on config-host.mak will trigger a configure > > rerun anyway. > > > > If you want to add it, you should also add it for other config-*.mak > > files. However, I'd remove this line and just change > > > > -# 1. ensure config-host.mak is up-to-date > > +# 1. ensure config-host.mak is up-to-date. All other config-*.mak > > +# files for subdirectories will be updated as well. > > Peter ran into a mismatch between config-host.mak and > tests/tcg/foo/config-target.mak in his build system so it didn't get > picked up at one point. > But what is the rule that the new dependency is going to trigger? As far as I can see there is no rule to regenerate the $config_target_mak files, and also no rule to regenerate configure; the only effect of a change to configure will be rerunning the script, but that's triggered by the existing config-host.mak rule. Paolo > > > > in the Makefile. > > > > Paolo > > > -- > Alex Bennée > Virtualisation Tech Lead @ Linaro > >
Re: [PATCH 08/31] configure: ensure dependency for cross-compile setup
On 9/25/23 16:48, Alex Bennée wrote: If we update configure we should make sure we regenerate all the compiler details. We should also ensure those details are upto date before building the TCG tests. Signed-off-by: Alex Bennée --- configure | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configure b/configure index e83872571d..a95e0f5767 100755 --- a/configure +++ b/configure @@ -1788,6 +1788,8 @@ for target in $target_list; do echo "HOST_GDB_SUPPORTS_ARCH=y" >> "$config_target_mak" fi + echo "$config_target_mak: configure" >> Makefile.prereqs This in practice is not adding anything; if "configure" changes then Makefile's dependency on config-host.mak will trigger a configure rerun anyway. If you want to add it, you should also add it for other config-*.mak files. However, I'd remove this line and just change -# 1. ensure config-host.mak is up-to-date +# 1. ensure config-host.mak is up-to-date. All other config-*.mak +# files for subdirectories will be updated as well. in the Makefile. Paolo
Re: [PATCH 05/31] tests/docker: make docker engine choice entirely configure driven
On 9/25/23 16:48, Alex Bennée wrote: Since 0b1a649047 (tests/docker: use direct RUNC call to build containers) we ended up with the potential for the remaining docker.py script calls to deviate from the direct RUNC calls. Fix this by dropping the use of ENGINE in the makefile and rely entirely on what we detect at configure time. Signed-off-by: Alex Bennée --- configure | 1 - tests/docker/Makefile.include | 7 ++- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/configure b/configure index e08127045d..707132a3ae 100755 --- a/configure +++ b/configure @@ -1694,7 +1694,6 @@ if test -n "$gdb_bin"; then fi if test "$container" != no; then -echo "ENGINE=$container" >> $config_host_mak echo "RUNC=$runc" >> $config_host_mak fi echo "SUBDIRS=$subdirs" >> $config_host_mak diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index dfabafab92..035d272be9 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -17,8 +17,7 @@ endif DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY)) RUNC ?= docker -ENGINE ?= auto This was used when using docker-* from the source directory. What about changing it to: RUNC ?= $(if $(shell command -v docker), docker, podman) No complaint on removing ENGINE= though. Paolo -DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(ENGINE) +DOCKER_SCRIPT=$(SRC_PATH)/tests/docker/docker.py --engine $(RUNC) CUR_TIME := $(shell date +%Y-%m-%d-%H.%M.%S.) DOCKER_SRC_COPY := $(BUILD_DIR)/docker-src.$(CUR_TIME) @@ -158,7 +157,7 @@ $(foreach i,$(filter-out $(DOCKER_PARTIAL_IMAGES),$(DOCKER_IMAGES)), \ ) docker: - @echo 'Build QEMU and run tests inside Docker or Podman containers' + @echo 'Build QEMU and run tests inside $(RUNC) containers' @echo @echo 'Available targets:' @echo @@ -198,8 +197,6 @@ docker: @echo 'EXECUTABLE=Include executable in image.' @echo 'EXTRA_FILES=" [... ]"' @echo ' Include extra files in image.' - @echo 'ENGINE=auto/docker/podman' - @echo ' Specify which container engine to run.' @echo 'REGISTRY=url Cache builds from registry (default:$(DOCKER_REGISTRY))' docker-help: docker
Re: [PATCH 00/18] RFC: Remove deprecated audio features
On 4/25/22 10:21, Martin Kletzander wrote: I wanted to deal with https://bugzilla.redhat.com/2043498 and I got a suggesstion that removing deprecated features could actually make it easier to propagate the error. In the end (last patch) it turns out the error is still just reported with error_fatal, so it probably is not really needed, but I really wanted to dig into QEMU more and learn some of the internals for quite some time now. So I used the opportunity. The one-liner ended up being an 18 patch series which was, for someone who has just one commit in QEMU codebase, a pretty challenging task. Although I tried my best to do things properly, I am not sure whether I handled everything correctly, hence the RFC. Rebased and queued what's left... It took a while. :) Paolo Martin Kletzander (18): hw/audio: Remove -soundhw support hw/input/tsc210x: Extract common init code into new function hw/audio: Simplify hda audio init hw/audio/lm4549: Add errp error reporting to init function tests/qtest: Specify audiodev= and -audiodev ui/vnc: Require audiodev= Introduce machine's default-audiodev property audio: Add easy dummy audio initialiser hw/display/xlnx_dp.c: Add audiodev property hw/input/tsc210x.c: Support machine-default audiodev with fallback hw/arm: Support machine-default audiodev with fallback hw/ppc: Support machine-default audiodev with fallback audio: Make AUD_register_card fallible and require audiodev= audio: Require AudioState in AUD_add_capture audio: Be more strict during audio backend initialisation audio: Remove legacy audio environment variables and options audio: Remove unused can_be_default audio/spiceaudio: Fail initialisation when not using spice audio/alsaaudio.c | 1 - audio/audio.c | 204 +++ audio/audio.h | 5 +- audio/audio_int.h | 1 - audio/audio_legacy.c | 555 -- audio/coreaudio.m | 1 - audio/dbusaudio.c | 1 - audio/dsoundaudio.c | 1 - audio/jackaudio.c | 1 - audio/meson.build | 1 - audio/noaudio.c | 1 - audio/ossaudio.c | 1 - audio/paaudio.c | 1 - audio/sdlaudio.c | 1 - audio/spiceaudio.c| 3 +- audio/wavaudio.c | 1 - docs/about/deprecated.rst | 24 - docs/about/removed-features.rst | 27 + docs/qdev-device-use.txt | 21 +- docs/replay.txt | 2 +- hw/arm/integratorcp.c | 8 +- hw/arm/musicpal.c | 8 +- hw/arm/omap2.c| 11 +- hw/arm/realview.c | 3 + hw/arm/spitz.c| 10 +- hw/arm/versatilepb.c | 3 + hw/arm/vexpress.c | 3 + hw/arm/xlnx-zcu102.c | 4 + hw/arm/z2.c | 12 +- hw/audio/ac97.c | 9 +- hw/audio/adlib.c | 9 +- hw/audio/cs4231a.c| 8 +- hw/audio/es1370.c | 8 +- hw/audio/gus.c| 6 +- hw/audio/hda-codec.c | 37 +- hw/audio/intel-hda.c | 25 +- hw/audio/intel-hda.h | 2 +- hw/audio/lm4549.c | 7 +- hw/audio/lm4549.h | 3 +- hw/audio/meson.build | 1 - hw/audio/pcspk.c | 15 +- hw/audio/pl041.c | 2 +- hw/audio/sb16.c | 9 +- hw/audio/soundhw.c| 177 -- hw/audio/wm8750.c | 5 +- hw/core/machine.c | 23 + hw/display/xlnx_dp.c | 12 +- hw/input/tsc210x.c| 79 ++- hw/ppc/prep.c | 4 + hw/usb/dev-audio.c| 5 +- include/hw/audio/soundhw.h| 15 - include/hw/boards.h | 1 + qemu-options.hx | 37 -- .../codeconverter/test_regexps.py | 1 - softmmu/qdev-monitor.c| 2 - softmmu/vl.c | 10 - tests/qtest/ac97-test.c |
Re: [RFC 4/6] migration: Deprecate -incoming
On Thu, Jun 22, 2023 at 5:26 PM Peter Xu wrote: > PS: we may want to postpone this to be later than migration_object_init(), > when/if there's a real patch. Yes, that's true. > > > The only incompatibility is for people who are using "," in an URI, > > > which is rare and only an issue for the "exec" protocol. > > If we worry on breaking anyone, we can apply the keyval parsing only when > !exec. Not sure whether it matters a huge lot.. No, I don't think it does. > > Aha, that makes sense. And will allow us to deprecate/remove the > > --global migration.* stuff. > > We may still need a way to set the caps/params for src qemu?.. The source can use migrate_set_parameters normally, since migration has to be started from the monitor anyway. Paolo
Re: [RFC 4/6] migration: Deprecate -incoming
On 6/22/23 10:52, Juan Quintela wrote:
User friendliness.
The problem is that if you use more than two channels with multifd, on
the incoming side, you need to do:
You're sacrificing user-friendliness for the 99.99% that don't use
multifd, for an error (i.e. it's not even fixing the issue) for the
0.01% that use multifd. That's not user-friendly.
- migrate_set_parameter multifd-channels 16
- migrate_incoming
The issue is not how many features the command line has, but how
they're implemented.
Or if they are confusing for the user?
Anyone using multifd is not a typical user anyway.
If they're just QMP wrappers and as such they're self-contained in
softmmu/vl.c, that's fine.
In fact, even for parameters, we could use keyval to parse "-incoming"
What is keyval?
util/keyval.c and include/qemu/keyval.h. It parses a list of key=value
pairs into a QDict. Once you have removed the "source" key from the
QDict you can use a visitor to parse the rest into a
MigrateSetParameters. See the handling of QEMU_OPTION_audio, it could
be something like
case QEMU_OPTION_incoing: {
Visitor *v;
MigrateSetParameters *incoming_params = NULL;
QDict *dict = keyval_parse(optarg, "source", NULL,
_fatal);
if (incoming) {
if (qdict_haskey(dict, "source")) {
error_setg(_fatal, "Parameter 'source' is
duplicate");
}
} else {
if (!qdict_haskey(dict, "source")) {
error_setg(_fatal, "Parameter 'source' is
missing");
}
runstate_set(RUN_STATE_INMIGRATE);
incoming = g_strdup(qdict_get_str(dict, "source"));
qdict_del(dict, "source");
}
v = qobject_input_visitor_new_keyval(QOBJECT(dict));
qobject_unref(dict);
visit_type_MigrateSetParameters(v, NULL,
_params, _fatal);
visit_free(v);
qmp_migration_set_parameters(incoming_params,
_fatal);
qapi_free_MigrateSetParameters(incoming_params);
}
For example "-incoming [source=]tcp:foo,multifd-channels=16" would
desugar to
migrate_set_parameter multifd-channels 16
migrate_incoming tcp:foo
The only incompatibility is for people who are using "," in an URI,
which is rare and only an issue for the "exec" protocol.
Paolo
and
set the parameters in the same place as above. That would remove the need
for "-global migration".
Could you elaborate?
The other option that I can think of is changing the error messages for
migrate_check_parameters() and give instructions that you can't set
multifd channels once that you have started incoming migration.
Explaining there to use migrate_incoming command?
Later, Juan.
Re: [RFC 4/6] migration: Deprecate -incoming
On 6/21/23 09:08, Thomas Huth wrote:
if (strcmp(incoming, "defer") != 0) {
+ warn_report("-incoming %s is deprecated, use -incoming
defer and "
+ " set the uri with migrate-incoming.",
incoming);
qmp_migrate_incoming(incoming, _err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ",
incoming);
Could we maybe keep at least the smallest set of necessary parameters
around? I'm often doing a "-incoming tcp:0:1234" for doing quick sanity
checks with migration, not caring about other migration parameters, so
if that could continue to work, that would be very appreciated.
Yeah, this is throwing the baby out with the bathwater.
Paolo
Re: [RFC 4/6] migration: Deprecate -incoming
On 6/12/23 22:51, Juan Quintela wrote:
Shall we just leave it there? Or is deprecating it helps us in any form?
See the patches two weeks ago when people complained that lisen(.., num)
was too low. And there are other parameters that work the same way
(that I convenientely had forgotten). So the easiest way to get things
right is to use "defer" always. Using -incoming "uri" should only be
for people that "know what they are doing", so we had to ways to do it:
- review all migration options and see which ones work without defer
and document it
- deprecate everything that is not defer.
"-incoming " is literally the same as running "migrate-incoming"
as the first thing on the monitor:
if (incoming) {
Error *local_err = NULL;
if (strcmp(incoming, "defer") != 0) {
qmp_migrate_incoming(incoming, _err);
if (local_err) {
error_reportf_err(local_err, "-incoming %s: ", incoming);
exit(1);
}
}
} else if (autostart) {
qmp_cont(NULL);
}
It's the only piece of code which distinguishes "-incoming defer" from
"-incoming ".
So I'm not sure what the problem would be with keeping it? The issue is
not how many features the command line has, but how they're implemented.
If they're just QMP wrappers and as such they're self-contained in
softmmu/vl.c, that's fine.
In fact, even for parameters, we could use keyval to parse "-incoming" and
set the parameters in the same place as above. That would remove the need
for "-global migration".
Paolo
Re: [PATCH v2 10/10] hmp: Deprecate 'singlestep' member of StatusInfo
On 4/4/23 16:24, Peter Maydell wrote: I think on balance I would go for: * remove (ie deprecate-and-drop) 'singlestep' from the QMP struct, rather than merely renaming it * if anybody comes along and says they want to do this via QMP, implement Paolo's idea of putting the accelerator object somewhere they can get at it and use qom-get/qom-set on it [My guess is this is very unlikely: nobody's complained so far that QMP doesn't permit setting 'singlestep'; and wanting read without write seems even more marginal.] * keep the HMP commands, but have both read and write directly talk to the accel object. I favour splitting the 'read' part out into its own 'info one-insn-per-tb', for consistency (then 'info status' matches the QMP query-status) I think the read part could be added to 'info jit'. Paolo
Re: [PATCH v2 10/10] hmp: Deprecate 'singlestep' member of StatusInfo
On 4/4/23 11:17, Peter Maydell wrote: I don't want to add a QMP interface for writing it unless there's somebody who actually has a use case for it. We could place the accelerator at a well-known QOM path such as /machine/accel, and then qom-set can already be used to implement such an interface. Not that you have to do it---just an argument against adding a QMP version of singlestep/one-insn-per-tb. Paolo
Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems
On 2/17/23 11:47, Daniel P. Berrangé wrote: On Fri, Feb 17, 2023 at 11:36:41AM +0100, Markus Armbruster wrote: I feel the discussion petered out without a conclusion. I don't think letting the status quo win by inertia is a good outcome here. Which 32-bit hosts are still useful, and why? Which 32-bit hosts does Linux still provide KVM support for. All except ARM: MIPS, x86, PPC and RISC-V. I would like to remove x86, but encountered some objections. MIPS, nobody is really using it I think. So that leaves PPC and RISC-V. If any, is there an EOL date for Linux 32-bit KVM support ? No, and I don't think there's going to be one. Paolo
[PATCH v2 4/7] tests: convert x86_64 tests to query-cpus-fast
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
convert the JSON output for x86 tests to the new format, and drop the
"halted" field from the expected output as it is not available anymore.
The CPU properties were obtained from the query-hotpluggable-cpus output
in tests/qemumonitorjsondata. CPU, thread_id, and qom_path are renamed
respectively to cpu-index, qom-path and thread-id, while nip and halted
are removed.
Signed-off-by: Paolo Bonzini
---
...json-cpuinfo-x86-basic-pluggable-cpus.json | 65 ---
...nitorjson-cpuinfo-x86-basic-pluggable.data | 5 --
...onitorjson-cpuinfo-x86-node-full-cpus.json | 16 ++---
...qemumonitorjson-cpuinfo-x86-node-full.data | 2 -
tests/qemumonitorjsontest.c | 4 +-
5 files changed, 48 insertions(+), 44 deletions(-)
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
index 7a4973195c..c4250dae1d 100644
---
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
+++
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
@@ -3,47 +3,62 @@
{
"arch": "x86",
"current": true,
- "CPU": 0,
- "qom_path": "/machine/unattached/device[0]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518291
+ "cpu-index": 0,
+ "props": {
+"core-id": 0,
+"thread-id": 0,
+"socket-id": 0
+ },
+ "qom-path": "/machine/unattached/device[0]",
+ "thread-id": 518291
},
{
"arch": "x86",
"current": false,
- "CPU": 1,
- "qom_path": "/machine/unattached/device[2]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518292
+ "cpu-index": 1,
+ "props": {
+"core-id": 1,
+"thread-id": 0,
+"socket-id": 0
+ },
+ "qom-path": "/machine/unattached/device[2]",
+ "thread-id": 518292
},
{
"arch": "x86",
"current": false,
- "CPU": 2,
- "qom_path": "/machine/unattached/device[3]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518294
+ "cpu-index": 2,
+ "props": {
+"core-id": 1,
+"thread-id": 1,
+"socket-id": 0
+ },
+ "qom-path": "/machine/unattached/device[3]",
+ "thread-id": 518294
},
{
"arch": "x86",
"current": false,
- "CPU": 3,
- "qom_path": "/machine/unattached/device[4]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518295
+ "cpu-index": 3,
+ "props": {
+"core-id": 0,
+"thread-id": 0,
+"socket-id": 1
+ },
+ "qom-path": "/machine/unattached/device[4]",
+ "thread-id": 518295
},
{
"arch": "x86",
"current": false,
- "CPU": 4,
- "qom_path": "/machine/unattached/device[5]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518296
+ "cpu-index": 4,
+ "props": {
+"core-id": 0,
+"thread-id": 1,
+"socket-id": 1
+ },
+ "qom-path": "/machine/unattached/device[5]",
+ "thread-id": 518296
}
],
"id": "libvirt-22"
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
index 9a1788d947..93cefb9dd2 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
@@ -7,7 +7,6 @@
type='qemu64-x86_64-cpu'
qom_path='/machine/unattached/device[0]'
topology: socket='0' core='0' thread='0' vcpus='1'
-halted
[vcpu libvirt-id='1']
online=yes
hotpluggable=no
@@ -17,7 +16,6 @@
type='qemu64-x86_64-cpu'
qom_path='/machine/unattached/device[2]'
topology: socket='0' core='0' thread='1' vcpus='
[PATCH v2 5/7] tests: remove query-cpus tests
All tests now use query-cpus-fast. Since the QEMU driver will lose
support for query-cpus soon, go ahead and remove support for testing
it.
Signed-off-by: Paolo Bonzini
---
tests/qemumonitorjsontest.c | 117 +---
1 file changed, 15 insertions(+), 102 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 0001df7bb3..9d8a315103 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1291,7 +1291,6 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct
qemuMonitorQueryCpusEntr
static int
testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(qemuMonitorTest *test,
struct
qemuMonitorQueryCpusEntry *expect,
- bool fast,
size_t num)
{
struct qemuMonitorQueryCpusEntry *cpudata = NULL;
@@ -1300,7 +1299,7 @@
testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(qemuMonitorTest *test,
int ret = -1;
if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test),
- , , true, fast) < 0)
+ , , true, true) < 0)
goto cleanup;
if (ncpudata != num) {
@@ -1326,72 +1325,6 @@
testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(qemuMonitorTest *test,
}
-static int
-testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *opaque)
-{
-const testGenericData *data = opaque;
-virDomainXMLOption *xmlopt = data->xmlopt;
-struct qemuMonitorQueryCpusEntry expect_slow[] = {
-{0, 17622, (char *) "/machine/unattached/device[0]", true},
-{1, 17624, (char *) "/machine/unattached/device[1]", true},
-{2, 17626, (char *) "/machine/unattached/device[2]", true},
-{3, 17628, NULL, true},
-};
-g_autoptr(qemuMonitorTest) test = NULL;
-
-if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema)))
-return -1;
-
-qemuMonitorTestSkipDeprecatedValidation(test, true);
-
-if (qemuMonitorTestAddItem(test, "query-cpus",
- "{"
- "\"return\": ["
- "{"
- "\"current\": true,"
- "\"CPU\": 0,"
- "\"qom_path\":
\"/machine/unattached/device[0]\","
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17622"
- "},"
- "{"
- "\"current\": false,"
- "\"CPU\": 1,"
- "\"qom_path\":
\"/machine/unattached/device[1]\","
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17624"
- "},"
- "{"
- "\"current\": false,"
- "\"CPU\": 2,"
- "\"qom_path\":
\"/machine/unattached/device[2]\","
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17626"
- "},"
- "{"
- "\"current\": false,"
- "\"CPU\": 3,"
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17628"
- "}"
- "],"
- "\"id\": \"libvirt-7\""
- "
[PATCH v2 7/7] qemu: deprecate query-cpus-fast capability
All supported versions of QEMU have the command.
Signed-off-by: Paolo Bonzini
---
src/qemu/qemu_capabilities.c | 1 -
src/qemu/qemu_capabilities.h | 2 +-
tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 -
tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 -
tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 -
tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 -
tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 -
tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.2.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.2.0.riscv64.xml | 1 -
tests/qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 -
tests/qemucapabilitiesdata/caps_5.2.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.0.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 -
tests/qemucapabilitiesdata/caps_6.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.1.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.2.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 -
tests/qemucapabilitiesdata/caps_6.2.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_7.0.0.aarch64.xml | 1 -
tests/qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 -
tests/qemucapabilitiesdata/caps_7.0.0.x86_64.xml | 1 -
tests/qemucapabilitiesdata/caps_7.1.0.x86_64.xml | 1 -
tests/qemuhotplugtest.c | 2 --
38 files changed, 1 insertion(+), 39 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c2c55f4800..ca1f32beca 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -468,7 +468,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"qcow2-luks", /* QEMU_CAPS_QCOW2_LUKS */
"pcie-pci-bridge", /* QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE */
"seccomp-blacklist", /* X_QEMU_CAPS_SECCOMP_BLACKLIST */
- "query-cpus-fast", /* QEMU_CAPS_QUERY_CPUS_FAST */
+ "query-cpus-fast", /* X_QEMU_CAPS_QUERY_CPUS_FAST */
"disk-write-cache", /* QEMU_CAPS_DISK_WRITE_CACHE */
/* 290 */
@@ -1212,7 +1212,6 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-cpu-model-expansion", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION },
{ "query-cpu-definitions", QEMU_CAPS_QUERY_CPU_DEFINITIONS },
{ "query-named-block-nodes", QEMU_CAPS_QUERY_NAMED_BLOCK_NODES },
-{ "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST },
{ "qom-list-properties", QEMU_CAPS_QOM_LIST_PROPERTIES },
{ "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL },
{ "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8f3090e2ce..20b1034ca5 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -447,7 +447,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */
QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE, /* -device pcie-pci-bridge */
X_QEMU_CAPS_SECCOMP_BLACKLIST, /* -sandbox.elevateprivileges */
-QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */
+X_QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */
QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache
param */
/* 290 */
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
index 6d4cc74a4d..d31168271b 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml
@@ -97,7 +97,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
index b5cee8476c..240bf4e7e8 100644
--- a/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
+++ b/tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml
@@ -137,7 +137,6 @@
-
diff --git a/tests/qemucapabilitiesdata/caps_4.0.0.aar
[PATCH v2 2/7] tests: drop "-fast" from query-cpus-fast tests
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
remove the "-fast" suffix from both x86-full-fast and s390-fast.
Signed-off-by: Paolo Bonzini
---
...-fast-cpus.json => qemumonitorjson-cpuinfo-s390-cpus.json} | 0
...hotplug.json => qemumonitorjson-cpuinfo-s390-hotplug.json} | 0
...uinfo-s390-fast.data => qemumonitorjson-cpuinfo-s390.data} | 0
...t-cpus.json => qemumonitorjson-cpuinfo-x86-full-cpus.json} | 0
...lug.json => qemumonitorjson-cpuinfo-x86-full-hotplug.json} | 0
...6-full-fast.data => qemumonitorjson-cpuinfo-x86-full.data} | 0
tests/qemumonitorjsontest.c | 4 ++--
7 files changed, 2 insertions(+), 2 deletions(-)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-cpus.json
=> qemumonitorjson-cpuinfo-s390-cpus.json} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-hotplug.json =>
qemumonitorjson-cpuinfo-s390-hotplug.json} (100%)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast.data =>
qemumonitorjson-cpuinfo-s390.data} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-x86-full-fast-cpus.json =>
qemumonitorjson-cpuinfo-x86-full-cpus.json} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
=> qemumonitorjson-cpuinfo-x86-full-hotplug.json} (100%)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-x86-full-fast.data
=> qemumonitorjson-cpuinfo-x86-full.data} (100%)
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-cpus.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-cpus.json
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-hotplug.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-hotplug.json
diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390.data
similarity index 100%
rename from tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390.data
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
rename to
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data
similarity index 100%
rename from tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 54ff240e97..d0a12d14e0 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -3115,8 +3115,8 @@ mymain(void)
DO_TEST_CPU_DATA("ecx");
DO_TEST_CPU_INFO("x86-basic-pluggable", 8);
+DO_TEST_CPU_INFO_FAST("x86-full", 11);
DO_TEST_CPU_INFO("x86-node-full", 8);
-DO_TEST_CPU_INFO_FAST("x86-full-fast", 11);
DO_TEST_CPU_INFO_FAST("x86-dies", 16);
DO_TEST_CPU_INFO("ppc64-basic", 24);
@@ -3125,7 +3125,7 @@ mymain(void)
DO_TEST_CPU_INFO("ppc64-hotplug-4", 24);
DO_TEST_CPU_INFO("ppc64-no-threads", 16);
-DO_TEST_CPU_INFO_FAST("s390-fast", 2);
+DO_TEST_CPU_INFO_FAST("s390", 2);
#define DO_TEST_BLOCK_NODE_DETECT(testname) \
do { \
--
2.37.1
[PATCH v2 6/7] qemu: remove support for query-cpus
The query-cpus-fast command was introduced in 2.12, therefore
query-cpus is never used on supported versions of QEMU. Remove
the logic to parse its output, as well as the parameters to
choose between the two commands.
Signed-off-by: Paolo Bonzini
---
src/qemu/qemu_domain.c | 22 --
src/qemu/qemu_monitor.c | 29 ++--
src/qemu/qemu_monitor.h | 6 ++---
src/qemu/qemu_monitor_json.c | 44 +++-
src/qemu/qemu_monitor_json.h | 3 +--
tests/qemumonitorjsontest.c | 4 ++--
6 files changed, 30 insertions(+), 78 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 69e0c9e217..bb25266959 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9618,22 +9618,18 @@ qemuDomainRefreshVcpuInfo(virQEMUDriver *driver,
size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
size_t i, j;
bool hotplug;
-bool fast;
bool validTIDs = true;
int rc;
int ret = -1;
hotplug = qemuDomainSupportsNewVcpuHotplug(vm);
-fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
- QEMU_CAPS_QUERY_CPUS_FAST);
-
-VIR_DEBUG("Maxvcpus %zu hotplug %d fast query %d", maxvcpus, hotplug,
fast);
+VIR_DEBUG("Maxvcpus %zu hotplug %d", maxvcpus, hotplug);
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), , maxvcpus,
- hotplug, fast);
+ hotplug);
qemuDomainObjExitMonitor(vm);
@@ -9641,7 +9637,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriver *driver,
goto cleanup;
/*
- * The query-cpus[-fast] commands return information
+ * The query-cpus-fast commands return information
* about the vCPUs, including the OS level PID that
* is executing the vCPU.
*
@@ -9766,7 +9762,6 @@ qemuDomainRefreshVcpuHalted(virQEMUDriver *driver,
size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
g_autoptr(virBitmap) haltedmap = NULL;
size_t i;
-bool fast;
/* Not supported currently for TCG, see qemuDomainRefreshVcpuInfo */
if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU)
@@ -9774,21 +9769,14 @@ qemuDomainRefreshVcpuHalted(virQEMUDriver *driver,
/* The halted state is interesting only on s390(x). On other platforms
* the data would be stale at the time when it would be used.
- * Calling qemuMonitorGetCpuHalted() can adversely affect the running
- * VM's performance unless QEMU supports query-cpus-fast.
*/
-if (!ARCH_IS_S390(vm->def->os.arch) ||
-!virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
-QEMU_CAPS_QUERY_CPUS_FAST))
+if (!ARCH_IS_S390(vm->def->os.arch))
return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
-fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
- QEMU_CAPS_QUERY_CPUS_FAST);
-haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus,
-fast);
+haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus);
qemuDomainObjExitMonitor(vm);
if (!haltedmap)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 0c0b07d4a5..9d20acdc11 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1565,16 +1565,16 @@ qemuMonitorGetCPUInfoLegacy(struct
qemuMonitorQueryCpusEntry *cpuentries,
*
* This function stitches together data retrieved via query-hotpluggable-cpus
* which returns entities on the hotpluggable level (which may describe more
- * than one guest logical vcpu) with the output of query-cpus (or
- * query-cpus-fast), having an entry per enabled guest logical vcpu.
+ * than one guest logical vcpu) with the output of query-cpus-fast,
+ * having an entry per enabled guest logical vcpu.
*
* query-hotpluggable-cpus conveys following information:
* - topology information and number of logical vcpus this entry creates
* - device type name of the entry that needs to be used when hotplugging
* - qom path in qemu which can be used to map the entry against
- * query-cpus[-fast]
+ * query-cpus-fast
*
- * query-cpus[-fast] conveys following information:
+ * query-cpus-fast conveys following information:
* - thread id of a given guest logical vcpu
* - order in which the vcpus were inserted
* - qom path to allow mapping the two together
@@ -1609,7 +1609,7 @@ qemuMonitorGetCPUInfoHotplug(struct
qemuMonitorQueryHotpluggableCpusEntry *hotpl
for (i = 0; i < nhotplugvcpus; i++)
totalvcpus += hotplugvcpus[i].vcpus;
-/* trim '/thread...' suffix from the data returned by query-cpus[-fast] */
+/* trim '/thread...' suffix from the data returned by query-cpus-fast */
for (i =
[PATCH v2 0/7] qemu: remove support for query-cpus
The query-cpus-fast command was introduced in 2.12, therefore query-cpus
is never used on supported versions of QEMU. Remove the logic to parse
its output, as well as the parameters to choose between the two commands.
Since most tests were still mocking the query-cpus command, the expected
results of QEMU monitor commands have to be converted as well.
Thanks,
Paolo
v1->v2: do not query capability, regenerate test output [Peter]
hopefully patch 3/7 is not mangled [Pavel]
Paolo Bonzini (7):
tests: remove duplicate cpuinfo test
tests: drop "-fast" from query-cpus-fast tests
tests: convert ppc64 tests to query-cpus-fast
tests: convert x86_64 tests to query-cpus-fast
tests: remove query-cpus tests
qemu: remove support for query-cpus
qemu: deprecate query-cpus-fast capability
src/qemu/qemu_capabilities.c | 1 -
src/qemu/qemu_capabilities.h | 2 +-
src/qemu/qemu_domain.c| 22 +-
src/qemu/qemu_monitor.c | 29 +-
src/qemu/qemu_monitor.h | 6 +-
src/qemu/qemu_monitor_json.c | 44 +--
src/qemu/qemu_monitor_json.h | 3 +-
.../qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 1 -
.../caps_3.1.0.x86_64.xml | 1 -
.../caps_4.0.0.aarch64.xml| 1 -
.../qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 1 -
.../caps_4.0.0.riscv32.xml| 1 -
.../caps_4.0.0.riscv64.xml| 1 -
.../qemucapabilitiesdata/caps_4.0.0.s390x.xml | 1 -
.../caps_4.0.0.x86_64.xml | 1 -
.../caps_4.1.0.x86_64.xml | 1 -
.../caps_4.2.0.aarch64.xml| 1 -
.../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 -
.../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 -
.../caps_4.2.0.x86_64.xml | 1 -
.../caps_5.0.0.aarch64.xml| 1 -
.../qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 1 -
.../caps_5.0.0.riscv64.xml| 1 -
.../caps_5.0.0.x86_64.xml | 1 -
.../qemucapabilitiesdata/caps_5.1.0.sparc.xml | 1 -
.../caps_5.1.0.x86_64.xml | 1 -
.../caps_5.2.0.aarch64.xml| 1 -
.../qemucapabilitiesdata/caps_5.2.0.ppc64.xml | 1 -
.../caps_5.2.0.riscv64.xml| 1 -
.../qemucapabilitiesdata/caps_5.2.0.s390x.xml | 1 -
.../caps_5.2.0.x86_64.xml | 1 -
.../caps_6.0.0.aarch64.xml| 1 -
.../qemucapabilitiesdata/caps_6.0.0.s390x.xml | 1 -
.../caps_6.0.0.x86_64.xml | 1 -
.../caps_6.1.0.x86_64.xml | 1 -
.../caps_6.2.0.aarch64.xml| 1 -
.../qemucapabilitiesdata/caps_6.2.0.ppc64.xml | 1 -
.../caps_6.2.0.x86_64.xml | 1 -
.../caps_7.0.0.aarch64.xml| 1 -
.../qemucapabilitiesdata/caps_7.0.0.ppc64.xml | 1 -
.../caps_7.0.0.x86_64.xml | 1 -
.../caps_7.1.0.x86_64.xml | 1 -
tests/qemuhotplugtest.c | 2 -
...umonitorjson-cpuinfo-ppc64-basic-cpus.json | 64 ++---
...itorjson-cpuinfo-ppc64-hotplug-1-cpus.json | 176 ++--
...itorjson-cpuinfo-ppc64-hotplug-2-cpus.json | 264 ++
...itorjson-cpuinfo-ppc64-hotplug-4-cpus.json | 264 ++
...mumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 -
...torjson-cpuinfo-ppc64-no-threads-cpus.json | 88 +++---
...=> qemumonitorjson-cpuinfo-s390-cpus.json} | 0
...qemumonitorjson-cpuinfo-s390-hotplug.json} | 0
...data => qemumonitorjson-cpuinfo-s390.data} | 0
...json-cpuinfo-x86-basic-pluggable-cpus.json | 65 +++--
...nitorjson-cpuinfo-x86-basic-pluggable.data | 5 -
...qemumonitorjson-cpuinfo-x86-full-cpus.json | 154 +-
...onitorjson-cpuinfo-x86-full-fast-cpus.json | 126 -
...torjson-cpuinfo-x86-full-fast-hotplug.json | 115
...qemumonitorjson-cpuinfo-x86-full-fast.data | 109
...onitorjson-cpuinfo-x86-node-full-cpus.json | 16 +-
...qemumonitorjson-cpuinfo-x86-node-full.data | 2 -
tests/qemumonitorjsontest.c | 102 +--
61 files changed, 626 insertions(+), 1076 deletions(-)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-cpus.json
=> qemumonitorjson-cpuinfo-s390-cpus.json} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-hotplug.json =>
qemumonitorjson-cpuinfo-s390-hotplug.json} (100%)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast.data =>
qemumonitorjson-cpuinfo-s390.data} (100%)
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjso
[PATCH v2 1/7] tests: remove duplicate cpuinfo test
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
remove the query-cpus version of the x86-full test.
Signed-off-by: Paolo Bonzini
---
...qemumonitorjson-cpuinfo-x86-full-cpus.json | 104
...umonitorjson-cpuinfo-x86-full-hotplug.json | 115 --
.../qemumonitorjson-cpuinfo-x86-full.data | 109 -
tests/qemumonitorjsontest.c | 1 -
4 files changed, 329 deletions(-)
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
deleted file mode 100644
index 16f5cc41dc..00
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
+++ /dev/null
@@ -1,104 +0,0 @@
-{
- "return": [
-{
- "arch": "x86",
- "current": true,
- "CPU": 0,
- "qom_path": "/machine/unattached/device[0]",
- "pc": 1042686,
- "halted": false,
- "thread_id": 895040
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 1,
- "qom_path": "/machine/peripheral/vcpu1",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895056
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 2,
- "qom_path": "/machine/peripheral/vcpu2",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895057
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 3,
- "qom_path": "/machine/peripheral/vcpu3",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895058
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 4,
- "qom_path": "/machine/peripheral/vcpu4",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895059
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 5,
- "qom_path": "/machine/peripheral/vcpu5",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895060
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 6,
- "qom_path": "/machine/peripheral/vcpu6",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895061
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 7,
- "qom_path": "/machine/peripheral/vcpu7",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895062
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 8,
- "qom_path": "/machine/peripheral/vcpu8",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895063
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 9,
- "qom_path": "/machine/peripheral/vcpu9",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895064
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 10,
- "qom_path": "/machine/peripheral/vcpu10",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895065
-}
- ],
- "id": "libvirt-52"
-}
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
deleted file mode 100644
index aff5aa3c9b..00
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
+++ /dev/null
@@ -1,115 +0,0 @@
-{
- "return": [
-{
- "props": {
-"core-id": 0,
-"thread-id": 0,
-"socket-id": 10
- },
- "vcpus-count": 1,
- "qom-path":
[PATCH v2 3/7] tests: convert ppc64 tests to query-cpus-fast
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
convert the JSON output for PPC tests to the new format, and drop the
"halted" field from the expected output as it is not available anymore.
The CPU properties were obtained from the query-hotpluggable-cpus output
in tests/qemumonitorjsondata. CPU, thread_id, and qom_path are renamed
respectively to cpu-index, qom-path and thread-id, while nip and halted
are removed.
Signed-off-by: Paolo Bonzini
---
...umonitorjson-cpuinfo-ppc64-basic-cpus.json | 64 ++---
...itorjson-cpuinfo-ppc64-hotplug-1-cpus.json | 176 ++--
...itorjson-cpuinfo-ppc64-hotplug-2-cpus.json | 264 ++
...itorjson-cpuinfo-ppc64-hotplug-4-cpus.json | 264 ++
...mumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 -
...torjson-cpuinfo-ppc64-no-threads-cpus.json | 88 +++---
tests/qemumonitorjsontest.c | 10 +-
7 files changed, 461 insertions(+), 413 deletions(-)
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
index 27a3d8b89f..d1d3406958 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
@@ -3,74 +3,58 @@
{
"arch": "ppc",
"current": true,
- "CPU": 0,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[0]",
- "halted": false,
- "thread_id": 21925
+ "cpu-index": 0,
+ "qom-path": "/machine/unattached/device[1]/thread[0]",
+ "thread-id": 21925
},
{
"arch": "ppc",
"current": false,
- "CPU": 1,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[1]",
- "halted": false,
- "thread_id": 21926
+ "cpu-index": 1,
+ "qom-path": "/machine/unattached/device[1]/thread[1]",
+ "thread-id": 21926
},
{
"arch": "ppc",
"current": false,
- "CPU": 2,
- "nip": -4611686018422360608,
- "qom_path": "/machine/unattached/device[1]/thread[2]",
- "halted": false,
- "thread_id": 21927
+ "cpu-index": 2,
+ "qom-path": "/machine/unattached/device[1]/thread[2]",
+ "thread-id": 21927
},
{
"arch": "ppc",
"current": false,
- "CPU": 3,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[3]",
- "halted": false,
- "thread_id": 21928
+ "cpu-index": 3,
+ "qom-path": "/machine/unattached/device[1]/thread[3]",
+ "thread-id": 21928
},
{
"arch": "ppc",
"current": false,
- "CPU": 4,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[4]",
- "halted": false,
- "thread_id": 21930
+ "cpu-index": 4,
+ "qom-path": "/machine/unattached/device[1]/thread[4]",
+ "thread-id": 21930
},
{
"arch": "ppc",
"current": false,
- "CPU": 5,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[5]",
- "halted": false,
- "thread_id": 21931
+ "cpu-index": 5,
+ "qom-path": "/machine/unattached/device[1]/thread[5]",
+ "thread-id": 21931
},
{
"arch": "ppc",
"current": false,
- "CPU": 6,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[6]",
- "halted": false,
- "thread_id": 21932
+ "cpu-index": 6,
+ "qom-path": "/machine/unattached/device[1]/thread[6]",
+ "thread-id": 21932
},
{
"arch": "ppc",
"current": false,
- "CPU": 7,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[7]",
- "halted": false,
- &q
Re: [PATCH 3/7] tests: convert ppc64 tests to query-cpus-fast
On 8/8/22 16:44, Pavel Hrdina wrote:
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads-cpus.json
index 31a3905e43..0ee53ae471 100644
---
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads-cpus.json
+++
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-no-threads-cpus.json
@@ -3,74 +3,82 @@
{
"arch": "ppc",
"current": true,
- "CPU": 0,
- "nip": -46116860184chine/unattached/device[2]/thread[0]",
+ "thread-id": 35233
},
This hunk doesn't look correct and doesn't apply on top of current
master.
Weird, this is not in the file that I have sent. git send-email or
sendmail bug?!?
Paolo
[PATCH 5/7] tests: remove query-cpus tests
All tests now use query-cpus-fast. Since the QEMU driver will lose
support for query-cpus soon, go ahead and remove support for testing
it.
Signed-off-by: Paolo Bonzini
---
tests/qemumonitorjsontest.c | 117 +---
1 file changed, 15 insertions(+), 102 deletions(-)
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 0001df7bb3..9d8a315103 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1291,7 +1291,6 @@ testQemuMonitorJSONqemuMonitorJSONQueryCPUsEqual(struct
qemuMonitorQueryCpusEntr
static int
testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(qemuMonitorTest *test,
struct
qemuMonitorQueryCpusEntry *expect,
- bool fast,
size_t num)
{
struct qemuMonitorQueryCpusEntry *cpudata = NULL;
@@ -1300,7 +1299,7 @@
testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(qemuMonitorTest *test,
int ret = -1;
if (qemuMonitorJSONQueryCPUs(qemuMonitorTestGetMonitor(test),
- , , true, fast) < 0)
+ , , true, true) < 0)
goto cleanup;
if (ncpudata != num) {
@@ -1326,72 +1325,6 @@
testQEMUMonitorJSONqemuMonitorJSONQueryCPUsHelper(qemuMonitorTest *test,
}
-static int
-testQemuMonitorJSONqemuMonitorJSONQueryCPUs(const void *opaque)
-{
-const testGenericData *data = opaque;
-virDomainXMLOption *xmlopt = data->xmlopt;
-struct qemuMonitorQueryCpusEntry expect_slow[] = {
-{0, 17622, (char *) "/machine/unattached/device[0]", true},
-{1, 17624, (char *) "/machine/unattached/device[1]", true},
-{2, 17626, (char *) "/machine/unattached/device[2]", true},
-{3, 17628, NULL, true},
-};
-g_autoptr(qemuMonitorTest) test = NULL;
-
-if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema)))
-return -1;
-
-qemuMonitorTestSkipDeprecatedValidation(test, true);
-
-if (qemuMonitorTestAddItem(test, "query-cpus",
- "{"
- "\"return\": ["
- "{"
- "\"current\": true,"
- "\"CPU\": 0,"
- "\"qom_path\":
\"/machine/unattached/device[0]\","
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17622"
- "},"
- "{"
- "\"current\": false,"
- "\"CPU\": 1,"
- "\"qom_path\":
\"/machine/unattached/device[1]\","
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17624"
- "},"
- "{"
- "\"current\": false,"
- "\"CPU\": 2,"
- "\"qom_path\":
\"/machine/unattached/device[2]\","
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17626"
- "},"
- "{"
- "\"current\": false,"
- "\"CPU\": 3,"
- "\"pc\": -2130530478,"
- "\"halted\": true,"
- "\"thread_id\": 17628"
- "}"
- "],"
- "\"id\": \"libvirt-7\""
- "
[PATCH 6/7] qemu: remove support for query-cpus
The query-cpus-fast command was introduced in 2.12, therefore
query-cpus is never used on supported versions of QEMU. Remove
the logic to parse its output, as well as the parameters to
choose between the two commands.
Signed-off-by: Paolo Bonzini
---
src/qemu/qemu_domain.c | 22 --
src/qemu/qemu_monitor.c | 29 ++--
src/qemu/qemu_monitor.h | 6 ++---
src/qemu/qemu_monitor_json.c | 44 +++-
src/qemu/qemu_monitor_json.h | 3 +--
tests/qemumonitorjsontest.c | 4 ++--
6 files changed, 30 insertions(+), 78 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 69e0c9e217..bb25266959 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9618,22 +9618,18 @@ qemuDomainRefreshVcpuInfo(virQEMUDriver *driver,
size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
size_t i, j;
bool hotplug;
-bool fast;
bool validTIDs = true;
int rc;
int ret = -1;
hotplug = qemuDomainSupportsNewVcpuHotplug(vm);
-fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
- QEMU_CAPS_QUERY_CPUS_FAST);
-
-VIR_DEBUG("Maxvcpus %zu hotplug %d fast query %d", maxvcpus, hotplug,
fast);
+VIR_DEBUG("Maxvcpus %zu hotplug %d", maxvcpus, hotplug);
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
rc = qemuMonitorGetCPUInfo(qemuDomainGetMonitor(vm), , maxvcpus,
- hotplug, fast);
+ hotplug);
qemuDomainObjExitMonitor(vm);
@@ -9641,7 +9637,7 @@ qemuDomainRefreshVcpuInfo(virQEMUDriver *driver,
goto cleanup;
/*
- * The query-cpus[-fast] commands return information
+ * The query-cpus-fast commands return information
* about the vCPUs, including the OS level PID that
* is executing the vCPU.
*
@@ -9766,7 +9762,6 @@ qemuDomainRefreshVcpuHalted(virQEMUDriver *driver,
size_t maxvcpus = virDomainDefGetVcpusMax(vm->def);
g_autoptr(virBitmap) haltedmap = NULL;
size_t i;
-bool fast;
/* Not supported currently for TCG, see qemuDomainRefreshVcpuInfo */
if (vm->def->virtType == VIR_DOMAIN_VIRT_QEMU)
@@ -9774,21 +9769,14 @@ qemuDomainRefreshVcpuHalted(virQEMUDriver *driver,
/* The halted state is interesting only on s390(x). On other platforms
* the data would be stale at the time when it would be used.
- * Calling qemuMonitorGetCpuHalted() can adversely affect the running
- * VM's performance unless QEMU supports query-cpus-fast.
*/
-if (!ARCH_IS_S390(vm->def->os.arch) ||
-!virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
-QEMU_CAPS_QUERY_CPUS_FAST))
+if (!ARCH_IS_S390(vm->def->os.arch))
return 0;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
return -1;
-fast = virQEMUCapsGet(QEMU_DOMAIN_PRIVATE(vm)->qemuCaps,
- QEMU_CAPS_QUERY_CPUS_FAST);
-haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus,
-fast);
+haltedmap = qemuMonitorGetCpuHalted(qemuDomainGetMonitor(vm), maxvcpus);
qemuDomainObjExitMonitor(vm);
if (!haltedmap)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 0c0b07d4a5..9d20acdc11 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1565,16 +1565,16 @@ qemuMonitorGetCPUInfoLegacy(struct
qemuMonitorQueryCpusEntry *cpuentries,
*
* This function stitches together data retrieved via query-hotpluggable-cpus
* which returns entities on the hotpluggable level (which may describe more
- * than one guest logical vcpu) with the output of query-cpus (or
- * query-cpus-fast), having an entry per enabled guest logical vcpu.
+ * than one guest logical vcpu) with the output of query-cpus-fast,
+ * having an entry per enabled guest logical vcpu.
*
* query-hotpluggable-cpus conveys following information:
* - topology information and number of logical vcpus this entry creates
* - device type name of the entry that needs to be used when hotplugging
* - qom path in qemu which can be used to map the entry against
- * query-cpus[-fast]
+ * query-cpus-fast
*
- * query-cpus[-fast] conveys following information:
+ * query-cpus-fast conveys following information:
* - thread id of a given guest logical vcpu
* - order in which the vcpus were inserted
* - qom path to allow mapping the two together
@@ -1609,7 +1609,7 @@ qemuMonitorGetCPUInfoHotplug(struct
qemuMonitorQueryHotpluggableCpusEntry *hotpl
for (i = 0; i < nhotplugvcpus; i++)
totalvcpus += hotplugvcpus[i].vcpus;
-/* trim '/thread...' suffix from the data returned by query-cpus[-fast] */
+/* trim '/thread...' suffix from the data returned by query-cpus-fast */
for (i =
[PATCH 7/7] qemu: deprecate query-cpus-fast capability
All supported versions of QEMU have the command.
Signed-off-by: Paolo Bonzini
---
src/qemu/qemu_capabilities.c | 4 ++--
src/qemu/qemu_capabilities.h | 2 +-
tests/qemuhotplugtest.c | 2 --
3 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index c2c55f4800..1eb006e3ef 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -468,7 +468,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"qcow2-luks", /* QEMU_CAPS_QCOW2_LUKS */
"pcie-pci-bridge", /* QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE */
"seccomp-blacklist", /* X_QEMU_CAPS_SECCOMP_BLACKLIST */
- "query-cpus-fast", /* QEMU_CAPS_QUERY_CPUS_FAST */
+ "query-cpus-fast", /* X_QEMU_CAPS_QUERY_CPUS_FAST */
"disk-write-cache", /* QEMU_CAPS_DISK_WRITE_CACHE */
/* 290 */
@@ -1212,7 +1212,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ "query-cpu-model-expansion", QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION },
{ "query-cpu-definitions", QEMU_CAPS_QUERY_CPU_DEFINITIONS },
{ "query-named-block-nodes", QEMU_CAPS_QUERY_NAMED_BLOCK_NODES },
-{ "query-cpus-fast", QEMU_CAPS_QUERY_CPUS_FAST },
+{ "query-cpus-fast", X_QEMU_CAPS_QUERY_CPUS_FAST },
{ "qom-list-properties", QEMU_CAPS_QOM_LIST_PROPERTIES },
{ "blockdev-del", QEMU_CAPS_BLOCKDEV_DEL },
{ "query-current-machine", QEMU_CAPS_QUERY_CURRENT_MACHINE },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 8f3090e2ce..20b1034ca5 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -447,7 +447,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */
QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE, /* -device pcie-pci-bridge */
X_QEMU_CAPS_SECCOMP_BLACKLIST, /* -sandbox.elevateprivileges */
-QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */
+X_QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */
QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache
param */
/* 290 */
diff --git a/tests/qemuhotplugtest.c b/tests/qemuhotplugtest.c
index 3c9dac241a..133145a23a 100644
--- a/tests/qemuhotplugtest.c
+++ b/tests/qemuhotplugtest.c
@@ -443,8 +443,6 @@ testQemuHotplugCpuPrepare(const char *test,
priv = data->vm->privateData;
-virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_QUERY_CPUS_FAST);
-
if (data->modern)
virQEMUCapsSet(priv->qemuCaps, QEMU_CAPS_QUERY_HOTPLUGGABLE_CPUS);
--
2.37.1
[PATCH 3/7] tests: convert ppc64 tests to query-cpus-fast
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
convert the JSON output for PPC tests to the new format, and drop the
"halted" field from the expected output as it is not available anymore.
The CPU properties were obtained from the query-hotpluggable-cpus output
in tests/qemumonitorjsondata. CPU, thread_id, and qom_path are renamed
respectively to cpu-index, qom-path and thread-id, while nip and halted
are removed.
Signed-off-by: Paolo Bonzini
---
...umonitorjson-cpuinfo-ppc64-basic-cpus.json | 64 ++---
...itorjson-cpuinfo-ppc64-hotplug-1-cpus.json | 176 ++--
...itorjson-cpuinfo-ppc64-hotplug-2-cpus.json | 264 ++
...itorjson-cpuinfo-ppc64-hotplug-4-cpus.json | 264 ++
...mumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 -
...torjson-cpuinfo-ppc64-no-threads-cpus.json | 88 +++---
tests/qemumonitorjsontest.c | 10 +-
7 files changed, 461 insertions(+), 413 deletions(-)
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
index 27a3d8b89f..d1d3406958 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-ppc64-basic-cpus.json
@@ -3,74 +3,58 @@
{
"arch": "ppc",
"current": true,
- "CPU": 0,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[0]",
- "halted": false,
- "thread_id": 21925
+ "cpu-index": 0,
+ "qom-path": "/machine/unattached/device[1]/thread[0]",
+ "thread-id": 21925
},
{
"arch": "ppc",
"current": false,
- "CPU": 1,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[1]",
- "halted": false,
- "thread_id": 21926
+ "cpu-index": 1,
+ "qom-path": "/machine/unattached/device[1]/thread[1]",
+ "thread-id": 21926
},
{
"arch": "ppc",
"current": false,
- "CPU": 2,
- "nip": -4611686018422360608,
- "qom_path": "/machine/unattached/device[1]/thread[2]",
- "halted": false,
- "thread_id": 21927
+ "cpu-index": 2,
+ "qom-path": "/machine/unattached/device[1]/thread[2]",
+ "thread-id": 21927
},
{
"arch": "ppc",
"current": false,
- "CPU": 3,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[3]",
- "halted": false,
- "thread_id": 21928
+ "cpu-index": 3,
+ "qom-path": "/machine/unattached/device[1]/thread[3]",
+ "thread-id": 21928
},
{
"arch": "ppc",
"current": false,
- "CPU": 4,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[4]",
- "halted": false,
- "thread_id": 21930
+ "cpu-index": 4,
+ "qom-path": "/machine/unattached/device[1]/thread[4]",
+ "thread-id": 21930
},
{
"arch": "ppc",
"current": false,
- "CPU": 5,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[5]",
- "halted": false,
- "thread_id": 21931
+ "cpu-index": 5,
+ "qom-path": "/machine/unattached/device[1]/thread[5]",
+ "thread-id": 21931
},
{
"arch": "ppc",
"current": false,
- "CPU": 6,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[6]",
- "halted": false,
- "thread_id": 21932
+ "cpu-index": 6,
+ "qom-path": "/machine/unattached/device[1]/thread[6]",
+ "thread-id": 21932
},
{
"arch": "ppc",
"current": false,
- "CPU": 7,
- "nip": -4611686018426772172,
- "qom_path": "/machine/unattached/device[1]/thread[7]",
- "halted": false,
- &q
[PATCH 4/7] tests: convert x86_64 tests to query-cpus-fast
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
convert the JSON output for x86 tests to the new format, and drop the
"halted" field from the expected output as it is not available anymore.
The CPU properties were obtained from the query-hotpluggable-cpus output
in tests/qemumonitorjsondata. CPU, thread_id, and qom_path are renamed
respectively to cpu-index, qom-path and thread-id, while nip and halted
are removed.
Signed-off-by: Paolo Bonzini
---
...json-cpuinfo-x86-basic-pluggable-cpus.json | 65 ---
...nitorjson-cpuinfo-x86-basic-pluggable.data | 5 --
...onitorjson-cpuinfo-x86-node-full-cpus.json | 16 ++---
...qemumonitorjson-cpuinfo-x86-node-full.data | 2 -
tests/qemumonitorjsontest.c | 4 +-
5 files changed, 48 insertions(+), 44 deletions(-)
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
index 7a4973195c..c4250dae1d 100644
---
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
+++
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable-cpus.json
@@ -3,47 +3,62 @@
{
"arch": "x86",
"current": true,
- "CPU": 0,
- "qom_path": "/machine/unattached/device[0]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518291
+ "cpu-index": 0,
+ "props": {
+"core-id": 0,
+"thread-id": 0,
+"socket-id": 0
+ },
+ "qom-path": "/machine/unattached/device[0]",
+ "thread-id": 518291
},
{
"arch": "x86",
"current": false,
- "CPU": 1,
- "qom_path": "/machine/unattached/device[2]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518292
+ "cpu-index": 1,
+ "props": {
+"core-id": 1,
+"thread-id": 0,
+"socket-id": 0
+ },
+ "qom-path": "/machine/unattached/device[2]",
+ "thread-id": 518292
},
{
"arch": "x86",
"current": false,
- "CPU": 2,
- "qom_path": "/machine/unattached/device[3]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518294
+ "cpu-index": 2,
+ "props": {
+"core-id": 1,
+"thread-id": 1,
+"socket-id": 0
+ },
+ "qom-path": "/machine/unattached/device[3]",
+ "thread-id": 518294
},
{
"arch": "x86",
"current": false,
- "CPU": 3,
- "qom_path": "/machine/unattached/device[4]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518295
+ "cpu-index": 3,
+ "props": {
+"core-id": 0,
+"thread-id": 0,
+"socket-id": 1
+ },
+ "qom-path": "/machine/unattached/device[4]",
+ "thread-id": 518295
},
{
"arch": "x86",
"current": false,
- "CPU": 4,
- "qom_path": "/machine/unattached/device[5]",
- "pc": -2130415978,
- "halted": true,
- "thread_id": 518296
+ "cpu-index": 4,
+ "props": {
+"core-id": 0,
+"thread-id": 1,
+"socket-id": 1
+ },
+ "qom-path": "/machine/unattached/device[5]",
+ "thread-id": 518296
}
],
"id": "libvirt-22"
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
index 9a1788d947..93cefb9dd2 100644
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
+++ b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-basic-pluggable.data
@@ -7,7 +7,6 @@
type='qemu64-x86_64-cpu'
qom_path='/machine/unattached/device[0]'
topology: socket='0' core='0' thread='0' vcpus='1'
-halted
[vcpu libvirt-id='1']
online=yes
hotpluggable=no
@@ -17,7 +16,6 @@
type='qemu64-x86_64-cpu'
qom_path='/machine/unattached/device[2]'
topology: socket='0' core='0' thread='1' vcpus='
[PATCH 2/7] tests: drop "-fast" from query-cpus-fast tests
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
remove the "-fast" suffix from both x86-full-fast and s390-fast.
Signed-off-by: Paolo Bonzini
---
...-fast-cpus.json => qemumonitorjson-cpuinfo-s390-cpus.json} | 0
...hotplug.json => qemumonitorjson-cpuinfo-s390-hotplug.json} | 0
...uinfo-s390-fast.data => qemumonitorjson-cpuinfo-s390.data} | 0
...t-cpus.json => qemumonitorjson-cpuinfo-x86-full-cpus.json} | 0
...lug.json => qemumonitorjson-cpuinfo-x86-full-hotplug.json} | 0
...6-full-fast.data => qemumonitorjson-cpuinfo-x86-full.data} | 0
tests/qemumonitorjsontest.c | 4 ++--
7 files changed, 2 insertions(+), 2 deletions(-)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-cpus.json
=> qemumonitorjson-cpuinfo-s390-cpus.json} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-hotplug.json =>
qemumonitorjson-cpuinfo-s390-hotplug.json} (100%)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast.data =>
qemumonitorjson-cpuinfo-s390.data} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-x86-full-fast-cpus.json =>
qemumonitorjson-cpuinfo-x86-full-cpus.json} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
=> qemumonitorjson-cpuinfo-x86-full-hotplug.json} (100%)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-x86-full-fast.data
=> qemumonitorjson-cpuinfo-x86-full.data} (100%)
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-cpus.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-cpus.json
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-cpus.json
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-hotplug.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast-hotplug.json
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-hotplug.json
diff --git a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390.data
similarity index 100%
rename from tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390-fast.data
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-s390.data
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
similarity index 100%
rename from
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
rename to
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data
similarity index 100%
rename from tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data
rename to tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 54ff240e97..d0a12d14e0 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -3115,8 +3115,8 @@ mymain(void)
DO_TEST_CPU_DATA("ecx");
DO_TEST_CPU_INFO("x86-basic-pluggable", 8);
+DO_TEST_CPU_INFO_FAST("x86-full", 11);
DO_TEST_CPU_INFO("x86-node-full", 8);
-DO_TEST_CPU_INFO_FAST("x86-full-fast", 11);
DO_TEST_CPU_INFO_FAST("x86-dies", 16);
DO_TEST_CPU_INFO("ppc64-basic", 24);
@@ -3125,7 +3125,7 @@ mymain(void)
DO_TEST_CPU_INFO("ppc64-hotplug-4", 24);
DO_TEST_CPU_INFO("ppc64-no-threads", 16);
-DO_TEST_CPU_INFO_FAST("s390-fast", 2);
+DO_TEST_CPU_INFO_FAST("s390", 2);
#define DO_TEST_BLOCK_NODE_DETECT(testname) \
do { \
--
2.37.1
[PATCH 1/7] tests: remove duplicate cpuinfo test
All supported versions of QEMU include the query-cpus-fast QMP command.
In preparation for dropping support for the old "query-cpus" commands,
remove the query-cpus version of the x86-full test.
Signed-off-by: Paolo Bonzini
---
...qemumonitorjson-cpuinfo-x86-full-cpus.json | 104
...umonitorjson-cpuinfo-x86-full-hotplug.json | 115 --
.../qemumonitorjson-cpuinfo-x86-full.data | 109 -
tests/qemumonitorjsontest.c | 1 -
4 files changed, 329 deletions(-)
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full.data
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
deleted file mode 100644
index 16f5cc41dc..00
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-cpus.json
+++ /dev/null
@@ -1,104 +0,0 @@
-{
- "return": [
-{
- "arch": "x86",
- "current": true,
- "CPU": 0,
- "qom_path": "/machine/unattached/device[0]",
- "pc": 1042686,
- "halted": false,
- "thread_id": 895040
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 1,
- "qom_path": "/machine/peripheral/vcpu1",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895056
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 2,
- "qom_path": "/machine/peripheral/vcpu2",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895057
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 3,
- "qom_path": "/machine/peripheral/vcpu3",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895058
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 4,
- "qom_path": "/machine/peripheral/vcpu4",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895059
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 5,
- "qom_path": "/machine/peripheral/vcpu5",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895060
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 6,
- "qom_path": "/machine/peripheral/vcpu6",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895061
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 7,
- "qom_path": "/machine/peripheral/vcpu7",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895062
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 8,
- "qom_path": "/machine/peripheral/vcpu8",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895063
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 9,
- "qom_path": "/machine/peripheral/vcpu9",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895064
-},
-{
- "arch": "x86",
- "current": false,
- "CPU": 10,
- "qom_path": "/machine/peripheral/vcpu10",
- "pc": 4294967280,
- "halted": false,
- "thread_id": 895065
-}
- ],
- "id": "libvirt-52"
-}
diff --git
a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
b/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
deleted file mode 100644
index aff5aa3c9b..00
--- a/tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-hotplug.json
+++ /dev/null
@@ -1,115 +0,0 @@
-{
- "return": [
-{
- "props": {
-"core-id": 0,
-"thread-id": 0,
-"socket-id": 10
- },
- "vcpus-count": 1,
- "qom-path":
[PATCH 0/7] qemu: remove support for query-cpus
The query-cpus-fast command was introduced in 2.12, therefore query-cpus
is never used on supported versions of QEMU. Remove the logic to parse
its output, as well as the parameters to choose between the two commands.
Since most tests were still mocking the query-cpus command, the expected
results of QEMU monitor commands have to be converted as well.
Thanks,
Paolo
Paolo Bonzini (7):
tests: remove duplicate cpuinfo test
tests: drop "-fast" from query-cpus-fast tests
tests: convert ppc64 tests to query-cpus-fast
tests: convert x86_64 tests to query-cpus-fast
tests: remove query-cpus tests
qemu: remove support for query-cpus
qemu: deprecate query-cpus-fast capability
src/qemu/qemu_capabilities.c | 4 +-
src/qemu/qemu_capabilities.h | 2 +-
src/qemu/qemu_domain.c| 22 +-
src/qemu/qemu_monitor.c | 29 +-
src/qemu/qemu_monitor.h | 6 +-
src/qemu/qemu_monitor_json.c | 44 +--
src/qemu/qemu_monitor_json.h | 3 +-
tests/qemuhotplugtest.c | 2 -
...umonitorjson-cpuinfo-ppc64-basic-cpus.json | 64 ++---
...itorjson-cpuinfo-ppc64-hotplug-1-cpus.json | 176 ++--
...itorjson-cpuinfo-ppc64-hotplug-2-cpus.json | 264 ++
...itorjson-cpuinfo-ppc64-hotplug-4-cpus.json | 264 ++
...mumonitorjson-cpuinfo-ppc64-hotplug-4.data | 8 -
...torjson-cpuinfo-ppc64-no-threads-cpus.json | 88 +++---
...=> qemumonitorjson-cpuinfo-s390-cpus.json} | 0
...qemumonitorjson-cpuinfo-s390-hotplug.json} | 0
...data => qemumonitorjson-cpuinfo-s390.data} | 0
...json-cpuinfo-x86-basic-pluggable-cpus.json | 65 +++--
...nitorjson-cpuinfo-x86-basic-pluggable.data | 5 -
...qemumonitorjson-cpuinfo-x86-full-cpus.json | 154 +-
...onitorjson-cpuinfo-x86-full-fast-cpus.json | 126 -
...torjson-cpuinfo-x86-full-fast-hotplug.json | 115
...qemumonitorjson-cpuinfo-x86-full-fast.data | 109
...onitorjson-cpuinfo-x86-node-full-cpus.json | 16 +-
...qemumonitorjson-cpuinfo-x86-node-full.data | 2 -
tests/qemumonitorjsontest.c | 102 +--
26 files changed, 628 insertions(+), 1042 deletions(-)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-cpus.json
=> qemumonitorjson-cpuinfo-s390-cpus.json} (100%)
rename
tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast-hotplug.json =>
qemumonitorjson-cpuinfo-s390-hotplug.json} (100%)
rename tests/qemumonitorjsondata/{qemumonitorjson-cpuinfo-s390-fast.data =>
qemumonitorjson-cpuinfo-s390.data} (100%)
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-cpus.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast-hotplug.json
delete mode 100644
tests/qemumonitorjsondata/qemumonitorjson-cpuinfo-x86-full-fast.data
--
2.37.1
Re: [PATCH 3/3] qemu_driver: use qemuMonitorQueryStats to extract halt poll time
On 7/22/22 17:43, Martin Kletzander wrote: As mentioned before, all these failures do not have to exit the function, but rather fallback to the old way. You can even create two new functions for the new and old implementations and then call them from here to make the fallback easier to spot (and code). More precisely, they should just "continue;" to the next iteration of the for loop, like if (!success_obj || !fail_obj) continue; found = true; and then go fall back if found is false at the end of the loop. On the other hand, here: if (virJSONValueGetNumberUlong(success_obj, ) < 0) return 0; if (virJSONValueGetNumberUlong(fail_obj, ) < 0) return 0; I am not sure about falling back, because it is really an unexpected situation where the statistic exist but somehow the value cannot be parsed. Paolo I wanted to change this before pushing as well, but I feel like I'm changing too much of your code. And I also had to rebase this (although trivially). Would you mind just changing these few last things so that we can get it in before the rc0 freeze?
Re: [PATCH 2/3] ui: Switch "-display sdl" to use the QAPI parser
On 5/17/22 10:34, Thomas Huth wrote: This remains, and that's fine. One step at time. Not sure how we want to proceed with that in the long run, though ... IIRC clearly, Paolo once said that it doesn't really belong into "-display" anyway and should be handled with the separate "-vnc" option instead? Not me, Gerd (https://lore.kernel.org/all/20210825092023.81396-2-th...@redhat.com/T/#e8c4f826cc8ff48b9afad37703e11704137f540c8): Other way around, -display vnc should be dropped. -display is about local displays, and there can be only one instance. -vnc / -spice enable remote access, and this can be done in addition to a local display. not possible: -display gtk + -display sdl possible: -display gtk + -vnc -display gtk + -vnc + -spice -display none + -vnc + -spice For what it's worth, Libvirt uses both -vnc and -spice, so we might very well proceed with Gerd's idea. If we don't like -vnc and -spice then it may be possible to QOMify them and go with -object. Thanks, Paolo
Re: [PATCH 01/18] hw/audio: Remove -soundhw support
On 4/25/22 10:21, Martin Kletzander wrote: One thing I am not sure about is whether to keep the aliases of ac97 and es1370 in the qdev_alias_table. Signed-off-by: Martin Kletzander I agree that -soundhw is a bad option, but I think we should preserve something similarly easy to use. For example: -audio OPTIONS,model=XXX would expand to -audiodev OPTIONS,id=audiodev0 and then call the same init_isa/init_pci functions that already exist; except that those would add an audiodev=audiodev0 property. This way, one can use "-audio pa,model=hda". Later on we can add a hook in the machines for the case when no model is specified, but you don't need to do that now because it's backwards compatible. Let me post a sketch after lunch. Paolo --- docs/about/deprecated.rst | 9 - docs/about/removed-features.rst | 10 + docs/qdev-device-use.txt | 21 +-- docs/replay.txt | 2 +- hw/audio/ac97.c | 3 - hw/audio/adlib.c | 2 - hw/audio/cs4231a.c| 2 - hw/audio/es1370.c | 3 - hw/audio/gus.c| 2 - hw/audio/intel-hda.c | 21 --- hw/audio/meson.build | 1 - hw/audio/pcspk.c | 11 -- hw/audio/sb16.c | 3 - hw/audio/soundhw.c| 177 -- include/hw/audio/soundhw.h| 15 -- qemu-options.hx | 27 --- .../codeconverter/test_regexps.py | 1 - softmmu/qdev-monitor.c| 2 - softmmu/vl.c | 6 - 19 files changed, 19 insertions(+), 299 deletions(-) delete mode 100644 hw/audio/soundhw.c delete mode 100644 include/hw/audio/soundhw.h diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index cf02ef6821e4..7ba71ebd3435 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -39,15 +39,6 @@ should specify an ``audiodev=`` property. Additionally, when using vnc, you should specify an ``audiodev=`` property if you plan to transmit audio through the VNC protocol. -Creating sound card devices using ``-soundhw`` (since 5.1) -'' - -Sound card devices should be created using ``-device`` instead. The -names are the same for most devices. The exceptions are ``hda`` which -needs two devices (``-device intel-hda -device hda-duplex``) and -``pcspk`` which can be activated using ``-machine -pcspk-audiodev=``. - ``-chardev`` backend aliases ``tty`` and ``parport`` (since 6.0) diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst index 4b831ea29176..086ba3edb042 100644 --- a/docs/about/removed-features.rst +++ b/docs/about/removed-features.rst @@ -336,6 +336,16 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine. The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which should be used instead. +Creating sound card devices using ``-soundhw`` (removed in 7.1) +''' + +Sound card devices should be created using ``-device`` instead. The +names are the same for most devices. The exceptions are ``hda`` which +needs two devices (``-device intel-hda -device hda-duplex``) and +``pcspk`` which can be activated using ``-machine +pcspk-audiodev=``. And ``AC97`` and ``ES1370`` now have to be +specified in uppercase. + QEMU Machine Protocol (QMP) commands diff --git a/docs/qdev-device-use.txt b/docs/qdev-device-use.txt index 240888933482..30e7eaa3e66d 100644 --- a/docs/qdev-device-use.txt +++ b/docs/qdev-device-use.txt @@ -311,21 +311,16 @@ constraints. Host and guest part of audio devices have always been separate. -The old way to define guest audio devices is -soundhw C1,... +Host side (backend) is defined using -audiodev with a specific driver: -The new way is to define each guest audio device separately with --device. +spice +pa +none -Map from -soundhw sound card name to -device: - -ac97-device AC97 -cs4231a -device cs4231a,iobase=IOADDR,irq=IRQ,dma=DMA -es1370 -device ES1370 -gus -device gus,iobase=IOADDR,irq=IRQ,dma=DMA,freq=F -hda -device intel-hda,msi=MSI -device hda-duplex -sb16-device sb16,iobase=IOADDR,irq=IRQ,dma=DMA,dma16=DMA16,version=V -adlib not yet available with -device -pcspk not yet available with -device +And each guest audio device is then defined with -device with +audiodev=AUDIODEV_ID that refers to the audio backend above.
Re: [PATCH 06/18] ui/vnc: Require audiodev=
On 4/25/22 10:21, Martin Kletzander wrote:
@@ -4188,12 +4188,15 @@ void vnc_display_open(const char *id, Error **errp)
vd->ledstate = 0;
audiodev = qemu_opt_get(opts, "audiodev");
-if (audiodev) {
-vd->audio_state = audio_state_by_name(audiodev);
-if (!vd->audio_state) {
-error_setg(errp, "Audiodev '%s' not found", audiodev);
-goto fail;
-}
+if (!audiodev) {
+error_setg(errp, "Audiodev parameter for vnc required");
+goto fail;
+}
+
Wouldn't this break "-vnc :0"? You can just ignore the audio commands
if vd->audio_state is NULL.
Paolo
Re: [PATCH 0/2] domain, qemu: add support for clearing TSC on reset
On 3/25/22 16:35, Michal Prívozník wrote: I'm fixing all the small issues I've raised in review and pushing these. Reviewed-by: Michal Privoznik No, please don't! The property is not yet part of QEMU, this should have been tagged RFC. Sorry about the confusion. Paolo
[PATCH v2] meson: do not look for libparted if not necessary
libparted_dep is not used if -Dstorage_disk=disabled. Do
not bother looking for it if the disk storage backend was not
requested.
Signed-off-by: Paolo Bonzini
---
meson.build | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/meson.build b/meson.build
index eb13c7efa4..27c21d1d67 100644
--- a/meson.build
+++ b/meson.build
@@ -1013,7 +1013,7 @@ else
endif
libparted_version = '1.8.0'
-libparted_dep = dependency('libparted', version: '>=' + libparted_version,
required: false)
+libparted_dep = dependency('libparted', version: '>=' + libparted_version,
required: get_option('storage_disk'))
libpcap_version = '1.5.0'
if not get_option('libpcap').disabled()
--
2.35.1
[PATCH] meson: do not look for libdevmapper/libparted if not requested
devmapper_dep and libparted_dep are not used if -Dstorage_disk=disabled. Do
not bother looking for these libraries if the disk storage backend was not
requested.
Signed-off-by: Paolo Bonzini
---
meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index eb13c7efa4..9d1f90fc80 100644
--- a/meson.build
+++ b/meson.build
@@ -933,7 +933,7 @@ if curl_dep.found()
endif
devmapper_version = '1.0.0'
-devmapper_dep = dependency('devmapper', version: '>=' + devmapper_version,
required: false)
+devmapper_dep = dependency('devmapper', version: '>=' + devmapper_version,
required: get_option('storage_disk'))
if devmapper_dep.found()
conf.set('WITH_DEVMAPPER', 1)
endif
@@ -1013,7 +1013,7 @@ else
endif
libparted_version = '1.8.0'
-libparted_dep = dependency('libparted', version: '>=' + libparted_version,
required: false)
+libparted_dep = dependency('libparted', version: '>=' + libparted_version,
required: get_option('storage_disk'))
libpcap_version = '1.5.0'
if not get_option('libpcap').disabled()
--
2.35.1
[PATCH] meson: do not look for librbd/librados if not requested
rbd_dep is not used if -Dstorage_rbd=disabled. Do not bother looking for
the libraries that compose it if the rbd storage backend was not requested.
Signed-off-by: Paolo Bonzini
---
meson.build | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index 2c1d3bc64d..eb13c7efa4 100644
--- a/meson.build
+++ b/meson.build
@@ -1127,8 +1127,8 @@ parallels_sdk_dep = dependency('parallels-sdk', version:
'>=' + parallels_sdk_ve
pciaccess_version = '0.10.0'
pciaccess_dep = dependency('pciaccess', version: '>=' + pciaccess_version,
required: get_option('pciaccess'))
-rbd_dep = cc.find_library('rbd', required: false)
-rados_dep = cc.find_library('rados', required: false)
+rbd_dep = cc.find_library('rbd', required: get_option('storage_rbd'))
+rados_dep = cc.find_library('rados', required: get_option('storage_rbd'))
if rbd_dep.found() and not cc.has_function('rbd_get_features', dependencies:
rbd_dep)
rbd_dep = dependency('', required: false)
endif
--
2.35.1
[PATCH 2/2] qemu: add support for tsc.on_reboot element
QEMU 7.0.0 adds a new property tsc-clear-on-reset to x86 CPU, corresponding
to Libvirt's element. Plumb it in the validation,
command line handling and tests.
Signed-off-by: Paolo Bonzini
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 4 +++
src/qemu/qemu_validate.c | 36 +--
.../caps_7.0.0.x86_64.replies | 4 +++
.../caps_7.0.0.x86_64.xml | 1 +
...uency.args => cpu-tsc-clear-on-reset.args} | 2 +-
... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++--
...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +-
tests/qemuxml2argvtest.c | 2 ++
10 files changed, 44 insertions(+), 16 deletions(-)
copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args =>
cpu-tsc-clear-on-reset.args} (97%)
copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args =>
cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%)
copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml =>
cpu-tsc-clear-on-reset.xml} (95%)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 32980e7330..d22bbee70e 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -668,6 +668,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
/* 425 */
"blockdev.nbd.tls-hostname", /*
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME */
+ "x86-cpu.tsc-clear-on-reset", /*
QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET */
);
@@ -1775,6 +1776,7 @@ static struct virQEMUCapsStringFlags
virQEMUCapsObjectPropsMemoryBackendMemfd[]
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsMaxCPU[] = {
{ "unavailable-features", QEMU_CAPS_CPU_UNAVAILABLE_FEATURES },
{ "kvm-no-adjvtime", QEMU_CAPS_CPU_KVM_NO_ADJVTIME },
+{ "tsc-clear-on-reset", QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET },
{ "migratable", QEMU_CAPS_CPU_MIGRATABLE },
};
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 0a215a11d5..7aee73a725 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -643,6 +643,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for
syntax-check */
/* 425 */
QEMU_CAPS_BLOCKDEV_NBD_TLS_HOSTNAME, /* tls hostname can be overriden for
NBD clients */
+QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET, /* x86-cpu.tsc-clear-on-reset */
QEMU_CAPS_LAST /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c836799888..8ecede0ade 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6613,6 +6613,10 @@ qemuBuildCpuCommandLine(virCommand *cmd,
case VIR_DOMAIN_TIMER_NAME_TSC:
if (timer->frequency > 0)
virBufferAsprintf(, ",tsc-frequency=%llu",
timer->frequency);
+if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR)
+virBufferAddLit(, ",tsc-clear-on-reset=on");
+else if (timer->reboot == VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP)
+virBufferAddLit(, ",tsc-clear-on-reset=off");
break;
case VIR_DOMAIN_TIMER_NAME_ARMVTIMER:
switch (timer->tickpolicy) {
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index f27e480696..c4ab2976dc 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -396,10 +396,11 @@ static int
qemuValidateDomainDefClockTimers(const virDomainDef *def,
virQEMUCaps *qemuCaps)
{
+virDomainTimerDef *timer;
size_t i;
for (i = 0; i < def->clock.ntimers; i++) {
-virDomainTimerDef *timer = def->clock.timers[i];
+timer = def->clock.timers[i];
switch ((virDomainTimerNameType)timer->name) {
case VIR_DOMAIN_TIMER_NAME_PLATFORM:
@@ -409,20 +410,25 @@ qemuValidateDomainDefClockTimers(const virDomainDef *def,
return -1;
case VIR_DOMAIN_TIMER_NAME_TSC:
-case VIR_DOMAIN_TIMER_NAME_KVMCLOCK:
-case VIR_DOMAIN_TIMER_NAME_HYPERVCLOCK:
-if (!ARCH_IS_X86(def->os.arch) && timer->present ==
VIR_TRISTATE_BOOL_YES) {
+if (!ARCH_IS_X86(def->os.arch) && timer->present ==
VIR_TRISTATE_BOOL_YES)
+goto no_support;
+
+if (timer->reboot != VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT &&
+!virQEMUCapsGet(qemuCaps,
QEMU_CAPS_X86_CPU_TSC_CLEAR_ON_RESET)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Configuring the '%s' timer is not supported "
- "for virtType=%s arch=%s machine=%s guests"),
- virDomainTimerNameTy
[PATCH 1/2] domain: add tsc.on_reboot element
Some versions of Windows hang on reboot if their TSC value is greater
than 2^54. The workaround is to reset the TSC to a small value. Add
to the domain configuration an attribute for this. It can be used
by QEMU and in principle also by ESXi, which has a property called
monitor_control.enable_softResetClearTSC as well.
Signed-off-by: Paolo Bonzini
---
docs/formatdomain.rst | 4
src/conf/domain_conf.c| 22 ++
src/conf/domain_conf.h| 10 ++
src/conf/schemas/domaincommon.rng | 9 +
4 files changed, 45 insertions(+)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
index d188de4858..c9319e42ac 100644
--- a/docs/formatdomain.rst
+++ b/docs/formatdomain.rst
@@ -2241,6 +2241,10 @@ Windows, however, expects it to be in so called
'localtime'.
``frequency``
The ``frequency`` attribute is an unsigned integer specifying the
frequency at which ``name="tsc"`` runs.
+ ``on_reboot``
+ The ``on_reboot`` attribute controls how the ``name="tsc"`` timer behaves
+ when the VM is reset, and can be "default", "clear" or "keep". The reset
+ behavior of other timers is hardcoded, and depends on the type of timer.
``mode``
The ``mode`` attribute controls how the ``name="tsc"`` timer is managed,
and can be "auto", "native", "emulate", "paravirt", or "smpsafe". Other
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 153954a0b0..1893b8bbca 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1217,6 +1217,13 @@ VIR_ENUM_IMPL(virDomainTimerMode,
"smpsafe",
);
+VIR_ENUM_IMPL(virDomainTimerRebootMode,
+ VIR_DOMAIN_TIMER_REBOOT_MODE_LAST,
+ "default",
+ "keep",
+ "clear",
+);
+
VIR_ENUM_IMPL(virDomainStartupPolicy,
VIR_DOMAIN_STARTUP_POLICY_LAST,
"default",
@@ -12022,6 +12029,7 @@ virDomainTimerDefParseXML(xmlNodePtr node,
g_autofree char *tickpolicy = NULL;
g_autofree char *track = NULL;
g_autofree char *mode = NULL;
+g_autofree char *reboot = NULL;
def = g_new0(virDomainTimerDef, 1);
@@ -12080,6 +12088,15 @@ virDomainTimerDefParseXML(xmlNodePtr node,
}
}
+reboot = virXMLPropString(node, "on_reboot");
+if (reboot != NULL) {
+if ((def->reboot = virDomainTimerRebootModeTypeFromString(reboot)) <=
0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown timer reboot mode '%s'"), reboot);
+goto error;
+}
+}
+
catchup = virXPathNode("./catchup", ctxt);
if (catchup != NULL) {
ret = virXPathULong("string(./catchup/@threshold)", ctxt,
@@ -26151,6 +26168,11 @@ virDomainTimerDefFormat(virBuffer *buf,
virBufferAsprintf(, " mode='%s'",
virDomainTimerModeTypeToString(def->mode));
}
+
+if (def->reboot) {
+virBufferAsprintf(, " on_reboot='%s'",
+ virDomainTimerRebootModeTypeToString(def->mode));
+}
}
if (def->catchup.threshold > 0)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index b69abfa270..3618410f6c 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2408,6 +2408,14 @@ typedef enum {
VIR_DOMAIN_TIMER_MODE_LAST
} virDomainTimerModeType;
+typedef enum {
+VIR_DOMAIN_TIMER_REBOOT_MODE_DEFAULT = 0,
+VIR_DOMAIN_TIMER_REBOOT_MODE_KEEP,
+VIR_DOMAIN_TIMER_REBOOT_MODE_CLEAR,
+
+VIR_DOMAIN_TIMER_REBOOT_MODE_LAST
+} virDomainTimerRebootModeType;
+
typedef enum {
VIR_DOMAIN_CPU_PLACEMENT_MODE_STATIC = 0,
VIR_DOMAIN_CPU_PLACEMENT_MODE_AUTO,
@@ -2439,6 +2447,7 @@ struct _virDomainTimerDef {
/* frequency & mode are only valid for name='tsc' */
unsigned long long frequency; /* in Hz, unspecified = 0 */
int mode; /* enum virDomainTimerModeType */
+int reboot; /* enum virDomainTimerRebootModeType */
};
typedef enum {
@@ -4032,6 +4041,7 @@ VIR_ENUM_DECL(virDomainClockBasis);
VIR_ENUM_DECL(virDomainTimerName);
VIR_ENUM_DECL(virDomainTimerTrack);
VIR_ENUM_DECL(virDomainTimerTickpolicy);
+VIR_ENUM_DECL(virDomainTimerRebootMode);
VIR_ENUM_DECL(virDomainTimerMode);
VIR_ENUM_DECL(virDomainCpuPlacementMode);
diff --git a/src/conf/schemas/domaincommon.rng
b/src/conf/schemas/domaincommon.rng
index 9c1b64a644..10d0404889 100644
--- a/src/conf/schemas/domaincommon.rng
+++ b/src/conf/schemas/domaincommon.rng
@@ -1285,6 +1285,15 @@
+
+
+
+default
+clear
+keep
+
+
+
--
2.35.1
[PATCH 0/2] domain, qemu: add support for clearing TSC on reset
Some versions of Windows hang on reboot if their TSC value is greater
than 2^54. The workaround is to reset the TSC to a small value, and
is implemented by QEMU and ESXi. This series adds a domain attribute
for this, and implements it in QEMU.
Paolo
Paolo Bonzini (2):
domain: add tsc.on_reboot element
qemu: add support for tsc.on_reboot element
docs/formatdomain.rst | 4 +++
src/conf/domain_conf.c| 22
src/conf/domain_conf.h| 10 ++
src/conf/schemas/domaincommon.rng | 9 +
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 4 +++
src/qemu/qemu_validate.c | 36 +--
.../caps_7.0.0.x86_64.replies | 4 +++
.../caps_7.0.0.x86_64.xml | 1 +
...uency.args => cpu-tsc-clear-on-reset.args} | 2 +-
... cpu-tsc-clear-on-reset.x86_64-7.0.0.args} | 6 ++--
...equency.xml => cpu-tsc-clear-on-reset.xml} | 2 +-
tests/qemuxml2argvtest.c | 2 ++
14 files changed, 89 insertions(+), 16 deletions(-)
copy tests/qemuxml2argvdata/{cpu-tsc-frequency.args =>
cpu-tsc-clear-on-reset.args} (97%)
copy tests/qemuxml2argvdata/{virtio-rng-builtin.x86_64-latest.args =>
cpu-tsc-clear-on-reset.x86_64-7.0.0.args} (77%)
copy tests/qemuxml2argvdata/{cpu-tsc-frequency.xml =>
cpu-tsc-clear-on-reset.xml} (95%)
--
2.35.1
[PATCH] tests: add dependencies to meson declaration
Make sure that all tests are run after the helpers and mocks are
(re)built. This enables for example using "meson test" as the
command line passed to "git bisect run".
Signed-off-by: Paolo Bonzini
---
tests/meson.build | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/tests/meson.build b/tests/meson.build
index 8f92f2033f..fd78d8a2fd 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -191,8 +191,9 @@ test_file_wrapper_lib = static_library(
dependencies: [ tests_dep ],
)
+tests_deps = []
foreach mock : mock_libs
- shared_library(
+ tests_deps += shared_library(
mock['name'],
mock.get('sources', [ '@0@.c'.format(mock['name']) ]),
override_options: [
@@ -215,7 +216,7 @@ endforeach
# Must not link to any libvirt modules - libc only otherwise external
# libraries might unexpectedly leak file descriptors into commandhelper
# invalidating the test logic assumptions.
-executable(
+tests_deps += executable(
'commandhelper',
[ 'commandhelper.c' ],
dependencies: [
@@ -227,7 +228,7 @@ executable(
)
# This is a fake SSH we use from virnetsockettest
-executable(
+tests_deps += executable(
'ssh',
[ 'ssh.c' ],
dependencies: [
@@ -592,7 +593,7 @@ foreach data : tests
# default meson timeout
timeout = 30
endif
- test(data['name'], test_bin, env: tests_env, timeout: timeout)
+ test(data['name'], test_bin, env: tests_env, timeout: timeout, depends:
tests_deps)
endforeach
--
2.35.1
Re: [PATCH] deprecation: x86 default machine types
On 3/2/22 18:42, Daniel P. Berrangé wrote: Overall I'm just not seeing enough benefit to justify the disruption we'll cause by making this change to existing system emulator binaries. I agree. Paolo
Re: Call for GSoC and Outreachy project ideas for summer 2022
On 2/21/22 10:36, Michal Prívozník wrote: Indeed. Libvirt's participating on its own since 2016, IIRC. Since we're still in org acceptance phase we have some time to decide this, actually. We can do the final decision after participating orgs are announced. My gut feeling says that it's going to be more work on QEMU side which would warrant it to be on the QEMU ideas page. There are multiple projects that can be done on this topic, some QEMU-only, some Libvirt-only. For now I would announce the project on the Libvirt side (and focus on those projects) since you are comentoring. Paolo
Re: Call for GSoC and Outreachy project ideas for summer 2022
On 2/18/22 12:39, Michal Prívozník wrote: On 2/17/22 18:52, Paolo Bonzini wrote: I would like to co-mentor one or more projects about adding more statistics to Mark Kanda's newly-born introspectable statistics subsystem in QEMU (https://patchew.org/QEMU/20220215150433.2310711-1-mark.ka...@oracle.com/), for example integrating "info blockstats"; and/or, to add matching functionality to libvirt. However, I will only be available for co-mentoring unfortunately. I'm happy to offer my helping hand in this. I mean the libvirt part, since I am a libvirt developer. I believe this will be listed in QEMU's ideas list, right? Does Libvirt participate to GSoC as an independent organization this year? If not, I'll add it as a Libvirt project on the QEMU ideas list. Paolo
Re: Call for GSoC and Outreachy project ideas for summer 2022
On 1/28/22 16:47, Stefan Hajnoczi wrote: Dear QEMU, KVM, and rust-vmm communities, QEMU will apply for Google Summer of Code 2022 (https://summerofcode.withgoogle.com/) and has been accepted into Outreachy May-August 2022 (https://www.outreachy.org/). You can now submit internship project ideas for QEMU, KVM, and rust-vmm! If you have experience contributing to QEMU, KVM, or rust-vmm you can be a mentor. It's a great way to give back and you get to work with people who are just starting out in open source. Please reply to this email by February 21st with your project ideas. I would like to co-mentor one or more projects about adding more statistics to Mark Kanda's newly-born introspectable statistics subsystem in QEMU (https://patchew.org/QEMU/20220215150433.2310711-1-mark.ka...@oracle.com/), for example integrating "info blockstats"; and/or, to add matching functionality to libvirt. However, I will only be available for co-mentoring unfortunately. Paolo Good project ideas are suitable for remote work by a competent programmer who is not yet familiar with the codebase. In addition, they are: - Well-defined - the scope is clear - Self-contained - there are few dependencies - Uncontroversial - they are acceptable to the community - Incremental - they produce deliverables along the way Feel free to post ideas even if you are unable to mentor the project. It doesn't hurt to share the idea! I will review project ideas and keep you up-to-date on QEMU's acceptance into GSoC. Internship program details: - Paid, remote work open source internships - GSoC projects are 175 or 350 hours, Outreachy projects are 30 hrs/week for 12 weeks - Mentored by volunteers from QEMU, KVM, and rust-vmm - Mentors typically spend at least 5 hours per week during the coding period Changes since last year: GSoC now has 175 or 350 hour project sizes instead of 12 week full-time projects. GSoC will accept applicants who are not students, before it was limited to students. For more background on QEMU internships, check out this video: https://www.youtube.com/watch?v=xNVCX7YMUL8 Please let me know if you have any questions! Stefan
Re: [PATCH v1 12/12] target/riscv: Support virtual time context synchronization
On 11/20/21 23:34, Richard Henderson wrote:
On 11/20/21 8:46 AM, Yifei Jiang wrote:
const VMStateDescription vmstate_riscv_cpu = {
.name = "cpu",
.version_id = 3,
.minimum_version_id = 3,
+ .post_load = cpu_post_load,
.fields = (VMStateField[]) {
VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
VMSTATE_UINT64_ARRAY(env.fpr, RISCVCPU, 32),
@@ -211,6 +221,10 @@ const VMStateDescription vmstate_riscv_cpu = {
VMSTATE_UINT64(env.mtohost, RISCVCPU),
VMSTATE_UINT64(env.timecmp, RISCVCPU),
+ VMSTATE_UINT64(env.kvm_timer_time, RISCVCPU),
+ VMSTATE_UINT64(env.kvm_timer_compare, RISCVCPU),
+ VMSTATE_UINT64(env.kvm_timer_state, RISCVCPU),
+
VMSTATE_END_OF_LIST()
},
Can't alter VMStateDescription.fields without bumping version.
If this is really kvm-only state, consider placing it into a
subsection. But I worry about kvm-only state because ideally we'd be
able to migrate between tcg and kvm (if only for debugging).
Where is this state stored for TCG?
Paolo
Re: [PATCH 11/11] Deprecate stable non-JSON -device and -object
On 24/09/21 11:04, Kevin Wolf wrote: We want to switch both from QemuOpts to the keyval parser in the future, which results in some incompatibilities, mainly around list handling. Mark the non-JSON version of both as unstable syntax so that management tools switch to JSON and we can later make the change without breaking things. Maybe we need a different section for unstable syntaxes, rather than overloading deprecated.rst? Paolo
Re: [PULL 36/40] vl: switch -M parsing to keyval
On 22/07/21 10:19, Peter Krempa wrote: This patch breaks detection of certain machine options features in libvirt which were being detected from 'query-command-line-options'. I presume the change simply removed this from the output of query-command-line-options due to the historical cruft how the aforementioned command works. Unfortunately I didn't find any suitable replacement from what we are querying. Yep, there is already a patch queued for this. Paolo
Re: [PATCH 4/4] x86/tsx: Add cmdline tsx=fake to not clear CPUID bits RTM and HLE
On 07/07/21 20:23, Eduardo Habkost wrote:
On Wed, Jul 7, 2021 at 1:18 PM Jim Mattson wrote:
On Wed, Jul 7, 2021 at 10:08 AM Eduardo Habkost wrote:
On Wed, Jul 7, 2021 at 12:42 PM Jim Mattson wrote:
On Wed, Jul 7, 2021 at 8:09 AM Eduardo Habkost wrote:
CCing libvir-list, Jiri Denemark, Michal Privoznik, so they are aware
that the definition of "supported CPU features" will probably become a
bit more complex in the future.
Has there ever been a clear definition? Family, model, and stepping,
for instance: are these the only values supported? That would make
cross-platform migration impossible. What about the vendor string? Is
that the only value supported? That would make cross-vendor migration
impossible. For the maximum input value for basic CPUID information
(CPUID.0H:EAX), is that the only value supported, or is it the maximum
value supported? On the various individual feature bits, does a '1'
imply that '0' is also supported, or is '1' the only value supported?
What about the feature bits with reversed polarity (e.g.
CPUID.(EAX=07H,ECX=0):EBX.FDP_EXCPTN_ONLY[bit 6])?
This API has never made sense to me. I have no idea how to interpret
what it is telling me.
Is this about GET_SUPPORTED_CPUID, QEMU's query-cpu-model-expansion &
related commands, or the libvirt CPU APIs?
This is my ongoing rant about KVM_GET_SUPPORTED_CPUID.
I agree the definition is not clear. I have tried to enumerate below
what QEMU assumes about the return value of KVM_GET_SUPPORTED_CPUID.
These are a collection of workarounds and feature-specific rules that
are encoded in the kvm_arch_get_supported_cpuid()
x86_cpu_filter_features(), and cpu_x86_cpuid() functions in QEMU.
1. Passing through the returned values (unchanged) from
KVM_GET_SUPPORTED_CPUID to KVM_SET_CPUID is assumed to be always safe,
as long as the ability to save/resume VCPU state is not required.
(This is the behavior implemented by "-cpu host,migratable=off")
Right, this is basically the definition of KVM_GET_SUPPORTED_CPUID.
2. The safety of setting a bit to a different value requires specific
knowledge about the CPUID bit.
2.1. For a specific set of registers (see below), QEMU assumes it's
safe to set the bit to 0 when KVM_GET_SUPPORTED_CPUID returns 1.
2.2. For a few specific leaves (see below), there are more complex rules.
2.4. For all other leaves, QEMU doesn't use the return value of
KVM_GET_SUPPORTED_CPUID at all (AFAICS).
The CPUID leaves mentioned in 2.1 are:
CPUID[1].EDX
CPUID[1].ECX
CPUID[6].EAX
CPUID[EAX=7,ECX=0].EBX
- This unfortunately includes de-feature bits like FDP_EXCPTN_ONLY and
ZERO_FCS_FDS
CPUID[EAX=7,ECX=0].ECX
CPUID[EAX=7,ECX=0].EDX
CPUID[EAX=7,ECX=1].EAX
CPUID[EAX=0Dh,ECX=0].EAX
CPUID[EAX=0Dh,ECX=0].EDX
CPUID[EAX=0Dh,ECX=1].EAX
- Note that CPUID[0Dh] has additional logic to ensure XSAVE component
info on CPUID is consistent
CPUID[4001h].EAX
CPUID[4001h].EDX
CPUID[8001h].EDX
CPUID[8001h].ECX
CPUID[8007h].EDX
CPUID[8008h].EBX
CPUID[800Ah].EDX
CPUID[C001h].EDX
Plus all unknown leaves.
Some of the CPUID leaves mentioned in 2.2 are:
CPUID[1].ECX.HYPERVISOR[bit 31]
- Can be enabled unconditionally
CPUID[1].ECX.TSC_DEADLINE_TIMER[bit 24]
- Can be set to 1 if using the in-kernel irqchip and
KVM_CAP_TSC_DEADLINE_TIMER is enabled
CPUID[1].ECX.X2APIC[bit 21]
- Can be set to 1 if using the in-kernel irqchip
CPUID[1].ECX.MONITOR[bit 3]
- Can be set to 1 if KVM_X86_DISABLE_EXITS_MWAIT is enabled
Can always be set to 1, but only makes sense to do so if
KVM_X86_DISABLE_EXITS_MWAIT is enabled.
CPUID[6].EAX.ARAT[bit 2]
- Can be enabled unconditionally
CPUID[EAX=7,ECX=0].EDX.ARCH_CAPABILITIES
- Workaround for KVM bug in Linux v4.17-v4.20
CPUID[EAX=14h,ECX=0], CPUID{EAX=14h,ECX=1]
- Most bits must match the host, unless
CPUID[EAX=7,ECX=0].EBX.INTEL_PT[bit 25] is 0
CPUID[8001h].EDX
- AMD-specific feature flag aliases can be set based on CPUID[1].EDX
CPUID[4001h].EAX
- KVM_FEATURE_PV_UNHALT requires in-kernel irqchip
- KVM_FEATURE_MSI_EXT_DEST_ID requires split irqchip
CPUID[4001].EDX.KVM_HINTS_REALTIME
- Can be enabled unconditionally
This should apply to all of CPUID[4000_0001h].EDX in the future
Thanks Eduardo, this is a great start for kernel-side documentation!
I'll wrap it in a patch.
Paolo
Re: [PATCH v3 27/30] hmp: QAPIfy object_add
On 15/03/21 12:38, Dr. David Alan Gilbert wrote:
* Kevin Wolf (kw...@redhat.com) wrote:
Am 15.03.2021 um 10:39 hat Markus Armbruster geschrieben:
Paolo Bonzini writes:
On 13/03/21 14:28, Markus Armbruster wrote:
Kevin Wolf writes:
This switches the HMP command object_add from a QemuOpts-based parser to
user_creatable_add_from_str() which uses a keyval parser and enforces
the QAPI schema.
Apart from being a cleanup, this makes non-scalar properties and help
accessible. In order for help to be printed to the monitor instead of
stdout, the printf() calls in the help functions are changed to
qemu_printf().
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
Reviewed-by: Dr. David Alan Gilbert
---
monitor/hmp-cmds.c | 17 ++---
qom/object_interfaces.c | 11 ++-
hmp-commands.hx | 2 +-
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 3c88a4faef..652cf9ff21 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
+const char *options = qdict_get_str(qdict, "object");
Error *err = NULL;
-QemuOpts *opts;
-Object *obj = NULL;
-
-opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, );
-if (err) {
-goto end;
-}
-obj = user_creatable_add_opts(opts, );
-qemu_opts_del(opts);
-
-end:
+user_creatable_add_from_str(options, );
hmp_handle_error(mon, err);
-
-if (obj) {
-object_unref(obj);
-}
}
Doesn't this break the list-valued properties (Memdev member host-nodes,
NumaNodeOptions member cpus) exactly the same way that made us keep
QemuOpts for qemu-system-FOO -object?
Yes, it does. I guess it can just be documented, unlike for the command
line?
Maybe. Judgement call, not mine to make.
Do people create such objects in HMP? I figure we don't really know.
Educated guess?
If you try, how does it break? Is it confusing? Can you show an
example?
(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=0
Error: Invalid parameter type for 'host-nodes', expected: array
(qemu) object_add memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes.0=0
(qemu)
HMP is not a stable interface, so changing the syntax didn't feel like a
problem to me. I doubt many people do HMP memory hotplug while setting a
specific NUMA policy, but it wouldn't change my assessment anyway. I
should have made this explicit in the commit message, though.
I'm OK for it to change, but yes I'd like to have the before/after
syntax listed somewhere as easy references for people confused.
I think we should try to improve the string-value QObject visitor to
also allow JSON values in some places, for example to allow
object_add
memory-backend-ram,id=mem,size=4G,policy=bind,host-nodes=[0,1,2,3]
Paolo
Re: [PATCH v3 27/30] hmp: QAPIfy object_add
On 13/03/21 14:28, Markus Armbruster wrote:
Kevin Wolf writes:
This switches the HMP command object_add from a QemuOpts-based parser to
user_creatable_add_from_str() which uses a keyval parser and enforces
the QAPI schema.
Apart from being a cleanup, this makes non-scalar properties and help
accessible. In order for help to be printed to the monitor instead of
stdout, the printf() calls in the help functions are changed to
qemu_printf().
Signed-off-by: Kevin Wolf
Acked-by: Peter Krempa
Reviewed-by: Eric Blake
Reviewed-by: Dr. David Alan Gilbert
---
monitor/hmp-cmds.c | 17 ++---
qom/object_interfaces.c | 11 ++-
hmp-commands.hx | 2 +-
3 files changed, 9 insertions(+), 21 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 3c88a4faef..652cf9ff21 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1670,24 +1670,11 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
void hmp_object_add(Monitor *mon, const QDict *qdict)
{
+const char *options = qdict_get_str(qdict, "object");
Error *err = NULL;
-QemuOpts *opts;
-Object *obj = NULL;
-
-opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, );
-if (err) {
-goto end;
-}
-obj = user_creatable_add_opts(opts, );
-qemu_opts_del(opts);
-
-end:
+user_creatable_add_from_str(options, );
hmp_handle_error(mon, err);
-
-if (obj) {
-object_unref(obj);
-}
}
Doesn't this break the list-valued properties (Memdev member host-nodes,
NumaNodeOptions member cpus) exactly the same way that made us keep
QemuOpts for qemu-system-FOO -object?
Yes, it does. I guess it can just be documented, unlike for the command
line?
Paolo
Re: [PATCH v3 22/30] qom: Factor out user_creatable_process_cmdline()
On 13/03/21 09:41, Markus Armbruster wrote: Observation, not objection: 1. QMP core parses JSON text into QObject, passes to generated marshaller. 2. Marshaller converts QObject to ObjectOptions with the QObject input visitor, passes to qmp_object_add(). 3. qmp_object_add() wraps around user_creatable_add_qapi(). 4. user_creatable_add_qapi() converts right back to QObject with the QObject output visitor. It splits the result into qom_type, id and the rest, and passes all three to user_creatable_add_type(). 5. user_creatable_add_type() performs a virtual visit with the QObject input visitor. The outermost object it visits itself, its children it visits by calling object_property_set(). I sure hope we wouldn't write it this way from scratch:) All problems in computer science ca be solved by adding another level of indirection, except those that can be solved by adding two more levels of indirection. I think your patch is a reasonable step towards a QOM that is at peace with QAPI. But there's plenty of work left. Absolutely, see https://wiki.qemu.org/Features/QOM-QAPI_integration for some brainstorming about it. Paolo
Re: [PATCH v3 26/30] qemu-img: Use user_creatable_process_cmdline() for --object
On 13/03/21 08:40, Markus Armbruster wrote:
+if (!user_creatable_add_from_str(optarg, _err)) {
+if (local_err) {
+error_report_err(local_err);
+exit(2);
+} else {
+/* Help was printed */
+exit(EXIT_SUCCESS);
+}
+}
+break;
}
-} break;
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
Why is this one different? The others all call
user_creatable_process_cmdline().
It's to exit with status code 2 instead of 1.
Paolo
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
On 12/03/21 10:17, Andrew Jones wrote: On Fri, Mar 12, 2021 at 10:01:43AM +0100, Paolo Bonzini wrote: On 12/03/21 09:48, Andrew Jones wrote: I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ... Could also pre-expand all of these and list them individually. That seems worse (in general) because in a lot of cases you don't care; the basic query-accels output should match the argument to "-accel". For these special subtypes, what's the property/state that indicates it when just using '-accel kvm' on the command line? Because if this qmp list matches the '-accel' parameter list, then qtest and other qmp clients may need to query that other information too, in order for this accel query to be useful. And, do we need an accel-specific qmp query for it? Or, is that information already available? It depends. On PPC (if I remember/understand correctly) only pseries supports both HV and PR, while all other machines only support KVM-PR. So in that case it's a kvm-type machine property that is defined only for the pseries machine. On MIPS instead there's no option and VZ always wins over TE. I think it could be made an option on -accel, but I'm not familiar with MIPS machine types. Something like "name: 'kvm', types: ['book3s-hv', 'pr']" would work nicely for KVM-PPC, and likewise for MIPS. Paolo
Re: [PATCH 1/6] accel: Introduce 'query-accels' QMP command
On 12/03/21 09:48, Andrew Jones wrote: I think we definitely need the additional data section here: For KVM on POWER, it would be good to know whether it's KVM-HV or KVM-PR, for KVM on MIPS it would be good to know whether it's KVM_VM_MIPS_VZ or KVM_VM_MIPS_TE, for KVM on x86 whether it's the AMD flavor or Intel, ... Could also pre-expand all of these and list them individually. That seems worse (in general) because in a lot of cases you don't care; the basic query-accels output should match the argument to "-accel". Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 12/03/21 09:14, Markus Armbruster wrote:
Paolo Bonzini writes:
On 11/03/21 15:08, Markus Armbruster wrote:
I would rather keep the OptsVisitor here. Do the same check for JSON
syntax that you have in qobject_input_visitor_new_str, and whenever
you need to walk all -object arguments, use something like this:
typedef struct ObjectArgument {
const char *id;
QDict *json;/* or NULL for QemuOpts */
QSIMPLEQ_ENTRY(ObjectArgument) next;
}
I already had patches in my queue to store -object in a GSList of
dictionaries, changing it to use the above is easy enough.
I think I'd prefer following -display's precedence. See my reply to
Kevin for details.
Yeah, I got independently to the same conclusion and posted patches
for that. I was scared that visit_type_ObjectOptions was too much for
OptsVisitor but it seems to work...
We have reason to be scared. I'll try to cover this in my review.
Yes, it's a good reason to possibly even delay those 3 patches to 6.1.
Paolo
Re: [PATCH 05/14] migrate: remove QMP/HMP commands for speed, downtime and cache size
On 11/03/21 19:33, Daniel P. Berrangé wrote: On Thu, Mar 11, 2021 at 07:18:54PM +0100, Paolo Bonzini wrote: On 11/03/21 12:54, Dr. David Alan Gilbert wrote: * Daniel P. Berrangé (berra...@redhat.com) wrote: The generic 'migrate_set_parameters' command handle all types of param. Only the QMP commands were documented in the deprecations page, but the rationale for deprecating applies equally to HMP, and the replacements exist. Furthermore the HMP commands are just shims to the QMP commands, so removing the latter breaks the former unless they get re-implemented. Signed-off-by: Daniel P. Berrangé Yes OK; ouch that's going to break my 7 years of instinctive 'migrate_set_speed 10G' typing, but it's probably the right thing to do. migrate_set_speed should remain if it is not changed to have a sane default. Define sane ? The default is 1 Gib/s since: commit 7590a2ae091fde8bb72d5df93977ab9707e23242 Author: Laurent Vivier Date: Mon Sep 21 16:49:57 2020 +0200 migration: increase max-bandwidth to 128 MiB/s (1 Gib/s) Oh, I missed that! I was still thinking of the old 32 MiB/s value. Paolo
Re: [PATCH 05/14] migrate: remove QMP/HMP commands for speed, downtime and cache size
On 11/03/21 12:54, Dr. David Alan Gilbert wrote: * Daniel P. Berrangé (berra...@redhat.com) wrote: The generic 'migrate_set_parameters' command handle all types of param. Only the QMP commands were documented in the deprecations page, but the rationale for deprecating applies equally to HMP, and the replacements exist. Furthermore the HMP commands are just shims to the QMP commands, so removing the latter breaks the former unless they get re-implemented. Signed-off-by: Daniel P. Berrangé Yes OK; ouch that's going to break my 7 years of instinctive 'migrate_set_speed 10G' typing, but it's probably the right thing to do. migrate_set_speed should remain if it is not changed to have a sane default. Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 15:08, Markus Armbruster wrote:
I would rather keep the OptsVisitor here. Do the same check for JSON
syntax that you have in qobject_input_visitor_new_str, and whenever
you need to walk all -object arguments, use something like this:
typedef struct ObjectArgument {
const char *id;
QDict *json;/* or NULL for QemuOpts */
QSIMPLEQ_ENTRY(ObjectArgument) next;
}
I already had patches in my queue to store -object in a GSList of
dictionaries, changing it to use the above is easy enough.
I think I'd prefer following -display's precedence. See my reply to
Kevin for details.
Yeah, I got independently to the same conclusion and posted patches for
that. I was scared that visit_type_ObjectOptions was too much for
OptsVisitor but it seems to work...
Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 11:38, Markus Armbruster wrote:
Here's a differently terrible hack. We have
keyval_parse() visitor
optarg > QObject > QAPI type
Idea: hack the QObject. If we're working for -object, and QObject maps
key "qom-type" to value "memory-backend-ram", get the value of
host-nodes, and if it's a string, parse it into a list like the opts
visitor does, and put that back, replacing the string value.
Same for other uses of Memdev and NumaNodeOptions with -object, if they
exist.
This doesn't help with backwards compatibility, since keyval loses the
first of host-nodes=0,host-nodes=4.
I would rather keep the OptsVisitor here. Do the same check for JSON
syntax that you have in qobject_input_visitor_new_str, and whenever you
need to walk all -object arguments, use something like this:
typedef struct ObjectArgument {
const char *id;
QDict *json;/* or NULL for QemuOpts */
QSIMPLEQ_ENTRY(ObjectArgument) next;
}
I already had patches in my queue to store -object in a GSList of
dictionaries, changing it to use the above is easy enough.
Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 09:45, Kevin Wolf wrote: I think it's only patch 29 and 30 that we would have to drop, actually. Unfortunately, that still removes one of the most immediately useful features, which is non-scalar properties for -object in the system emulator. But of course, a lot better than not merging it at all. Who is going to include this series in the next pull request, Markus or myself? The time is ticking for soft freeze. QOM is probably the right subsystem, so that would be you. Or I can just merge it myself as long as everyone is fine with it. Eric has some minor comments that I think could be addressed while applying the series. Or should I send a v4 for that (and for dropping patches 29 and 30)? I'd say just send a pull request. :) Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 11/03/21 08:47, Peter Krempa wrote: And for an interleaved list: -object memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind Uhm, I doubt this works? I would have expected "host-nodes=1-2,,5,,7" instead. Paolo
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 10/03/21 18:30, Kevin Wolf wrote: Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben: On 10/03/21 15:22, Peter Krempa wrote: I've stumbled upon a regression with this patchset applied: error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -object memory-backend-ram,id=pc.ram,size=1048576000,host-nodes=0,policy=bind: Invalid parameter type for 'host-nodes', expected: array This is the magic conversion of "string containing a list of integers" to "list of integers". Ah, crap. This one wouldn't have been a problem when converting only object-add, and I trusted your audit that user creatable objects don't depend on any QemuOpts magic. I should have noticed it, too, of course, but during the convertion I didn't have QemuOpts in mind, only QOM and QAPI. Yeah, let's just drop the -object conversion for now. It will just remove a few patches. Who is going to include this series in the next pull request, Markus or myself? The time is ticking for soft freeze. Paolo I checked all object types, and it seems this is the only one that is affected. We have a second list in AuthZListProperties, but it contains structs, so it was never supported on the command line anyway. The relevant code is in qapi/string-input-visitor.c, but I'm not sure where to stick it in the keyval-based parsing flow (i.e. qobject_input_visitor_new_keyval). Markus, any ideas? The best I can think of at the moment would be adding a flag to the keyval parser that would enable the feature only for -object (and only in the system emulator, because memory-backend-ram doesn't exist in the tools): The keyval parser would create a list if multiple values are given for the same key. Some care needs to be taken to avoid mixing the magic list feature with the normal indexed list options. The QAPI schema would then use an alternate between 'int' and ['int'], with the the memory-backend-ram implementation changed accordingly. We could consider immediately deprecating the syntax and printing a warning in the keyval parser when it automatically creates a list from two values for a key, so that users don't start using this syntax instead of the normal list syntax in other places. We'd probably still leave the implementation around for -device and other users of the same magic. Kevin
Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add
On 10/03/21 15:22, Peter Krempa wrote: I've stumbled upon a regression with this patchset applied: error: internal error: process exited while connecting to monitor: qemu-system-x86_64: -object memory-backend-ram,id=pc.ram,size=1048576000,host-nodes=0,policy=bind: Invalid parameter type for 'host-nodes', expected: array This is the magic conversion of "string containing a list of integers" to "list of integers". The relevant code is in qapi/string-input-visitor.c, but I'm not sure where to stick it in the keyval-based parsing flow (i.e. qobject_input_visitor_new_keyval). Markus, any ideas?
Re: [PATCH v2 19/31] qapi/qom: QAPIfy object-add
On 01/03/21 12:54, Kevin Wolf wrote: Please add a check in object_property_add_child that the id is well formed (using the id_wellformed function). This is pre-existing, but it becomes a regression for -object later in the series. Are the conditions for internally called object_property_add_child() actually the same as for IDs specified by the user? For example, I seem to remember some array-ish properties with [] in their name which aren't allowed by id_wellformed(). Yes, you are right. The obvious place to affect only the external interfaces would be user_creatable_add_type(). Makes sense, thanks. Paolo
Re: [PATCH v2 19/31] qapi/qom: QAPIfy object-add
On 24/02/21 14:52, Kevin Wolf wrote: +v = qobject_output_visitor_new(); +visit_type_ObjectOptions(v, NULL, , _abort); +visit_complete(v, ); +visit_free(v); + +props = qobject_to(QDict, qobj); +qdict_del(props, "qom-type"); +qdict_del(props, "id"); + +v = qobject_input_visitor_new(QOBJECT(props)); +obj = user_creatable_add_type(ObjectType_str(options->qom_type), + options->id, props, v, errp); +object_unref(obj); Please add a check in object_property_add_child that the id is well formed (using the id_wellformed function). This is pre-existing, but it becomes a regression for -object later in the series. Thanks, Paolo
Re: [PATCH 04/14] softmmu: remove '-usbdevice' command line option
On 24/02/21 14:11, Daniel P. Berrangé wrote:
This was replaced by the '-device usb-DEV' option.
Signed-off-by: Daniel P. Berrangé
This is probably used in many tutorial as "-usbdevice tablet" (for
example https://wiki.gentoo.org/wiki/QEMU/Options).
Paolo
---
docs/system/deprecated.rst | 9 ---
docs/system/removed-features.rst | 9 +++
softmmu/vl.c | 42
3 files changed, 9 insertions(+), 51 deletions(-)
diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 611adf60f7..c577cc97c4 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -21,15 +21,6 @@ deprecated.
System emulator command line arguments
--
-``-usbdevice`` (since 2.10.0)
-'
-
-The ``-usbdevice DEV`` argument is now a synonym for setting
-the ``-device usb-DEV`` argument instead. The deprecated syntax
-would automatically enable USB support on the machine type.
-If using the new syntax, USB support must be explicitly
-enabled via the ``-machine usb=on`` argument.
-
``-drive file=json:{...{'driver':'file'}}`` (since 3.0)
'''
diff --git a/docs/system/removed-features.rst b/docs/system/removed-features.rst
index dc63581fe5..74d022babf 100644
--- a/docs/system/removed-features.rst
+++ b/docs/system/removed-features.rst
@@ -50,6 +50,15 @@ by the ``tls-authz`` and ``sasl-authz`` options.
The ``pretty=on|off`` switch has no effect for HMP monitors and
its use is rejected.
+``-usbdevice`` (removed in 6.0)
+'''
+
+The ``-usbdevice DEV`` argument was now a synonym for setting
+the ``-device usb-DEV`` argument instead. The removed syntax
+would automatically enable USB support on the machine type.
+When using the new syntax, USB support must be explicitly
+enabled via the ``-machine usb=on`` argument.
+
QEMU Machine Protocol (QMP) commands
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b219ce1f35..c31061cc09 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -743,34 +743,6 @@ static void configure_msg(QemuOpts *opts)
}
-/***/
-/* USB devices */
-
-static int usb_device_add(const char *devname)
-{
-USBDevice *dev = NULL;
-
-if (!machine_usb(current_machine)) {
-return -1;
-}
-
-dev = usbdevice_create(devname);
-if (!dev)
-return -1;
-
-return 0;
-}
-
-static int usb_parse(const char *cmdline)
-{
-int r;
-r = usb_device_add(cmdline);
-if (r < 0) {
-error_report("could not add USB device '%s'", cmdline);
-}
-return r;
-}
-
/***/
/* machine registration */
@@ -1267,7 +1239,6 @@ static void monitor_parse(const char *optarg, const char *mode, bool pretty)
struct device_config {
enum {
-DEV_USB, /* -usbdevice */
DEV_SERIAL,/* -serial*/
DEV_PARALLEL, /* -parallel */
DEV_DEBUGCON, /* -debugcon */
@@ -2484,12 +2455,6 @@ static void qemu_create_cli_devices(void)
qemu_opts_foreach(qemu_find_opts("fw_cfg"),
parse_fw_cfg, fw_cfg_find(), _fatal);
-/* init USB devices */
-if (machine_usb(current_machine)) {
-if (foreach_device_config(DEV_USB, usb_parse) < 0)
-exit(1);
-}
-
/* init generic devices */
rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
qemu_opts_foreach(qemu_find_opts("device"),
@@ -3182,13 +3147,6 @@ void qemu_init(int argc, char **argv, char **envp)
olist = qemu_find_opts("machine");
qemu_opts_parse_noisily(olist, "usb=on", false);
break;
-case QEMU_OPTION_usbdevice:
-error_report("'-usbdevice' is deprecated, please use "
- "'-device usb-...' instead");
-olist = qemu_find_opts("machine");
-qemu_opts_parse_noisily(olist, "usb=on", false);
-add_device_config(DEV_USB, optarg);
-break;
case QEMU_OPTION_device:
if (!qemu_opts_parse_noisily(qemu_find_opts("device"),
optarg, true)) {
Re: [PATCH 00/14] deprecations: remove many old deprecations
On 24/02/21 14:11, Daniel P. Berrangé wrote:
The following features have been deprecated for well over the 2
release cycle we promise
``-usbdevice`` (since 2.10.0)
``-drive file=3Djson:{...{'driver':'file'}}`` (since 3.0)
``-vnc acl`` (since 4.0.0)
``-mon ...,control=3Dreadline,pretty=3Don|off`` (since 4.1)
``migrate_set_downtime`` and ``migrate_set_speed`` (since 2.8.0)
``query-named-block-nodes`` result ``encryption_key_missing`` (since 2.10.0)
``query-block`` result ``inserted.encryption_key_missing`` (since 2.10.0)
``migrate-set-cache-size`` and ``query-migrate-cache-size`` (since 2.11.0)
``query-named-block-nodes`` and ``query-block`` result dirty-bitmaps[i].sta=
tus (ince 4.0)
``query-cpus`` (since 2.12.0)
``query-cpus-fast`` ``arch`` output member (since 3.0.0)
``query-events`` (since 4.0)
chardev client socket with ``wait`` option (since 4.0)
``acl_show``, ``acl_reset``, ``acl_policy``, ``acl_add``, ``acl_remove`` (s=
ince 4.0.0)
``ide-drive`` (since 4.2)
``scsi-disk`` (since 4.2)
AFAICT, libvirt has ceased to use all of these too.
No objections except possibly for -usbdevice.
Paolo
Re: A brief look at deprecating our JSON extensions over RFC 8259
On 23/02/21 10:06, Markus Armbruster wrote:
Markus, did you rebuild the qtests after disabling single-quoted
strings? "make check-qtest-x86_64" would have rebuilt them, but I'm
confused by the results.
I ran "make check" and looked at the failures:
Still confused?
Yes. What's the patch that you used to disable the single quotes, and
why didn't the patched parser choke on
response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
"'property': 'temperature' } }", id);
?
Paolo
Re: A brief look at deprecating our JSON extensions over RFC 8259
On 22/02/21 18:54, Daniel P. Berrangé wrote:
These are sent to QEMU as double-quoted strings (the single-quoted JSON is
parsed to get interpolation and printed back; commit 563890c7c7, "libqtest:
escape strings in QMP commands, fix leak", 2014-07-01). However, doing the
interpolation requires a parser that recognizes the single-quoted strings.
IMHO this is the wrong solution to the problem. Consider the equivalent
libvirt code that uses a standard JSON library underneath and has a high
level API to serialize args into the command
qemuMonitorJSONMakeCommand("qom-get",
"s:path", id,
"s:property", "temperature");
Of course this example is reasonably easy since it is a flat set of
arguments. Nested args get slightly more complicated, but still always
preferrable to doing string interpolation IMHO.
I don't disagree. I'm just stating why I wanted a clarification from Markus.
Paolo
Re: A brief look at deprecating our JSON extensions over RFC 8259
On 22/02/21 16:24, Daniel P. Berrangé wrote:
This problem isn't unique to QEMU. Any app using JSON from the
shell will have the tedium of quote escaping. JSON is incredibly
widespread and no other apps felt it neccessary to introduce single
quoting support, because the benefit doesn't outweigh the interop
problem it introduces.
The quotes were introduced for C code (and especially qtest), not for
the shell. We have something like
response = qmp("{ 'execute': 'qom-get', 'arguments': { 'path': %s, "
"'property': 'temperature' } }", id);
These are sent to QEMU as double-quoted strings (the single-quoted JSON
is parsed to get interpolation and printed back; commit 563890c7c7,
"libqtest: escape strings in QMP commands, fix leak", 2014-07-01).
However, doing the interpolation requires a parser that recognizes the
single-quoted strings.
Markus, did you rebuild the qtests after disabling single-quoted
strings? "make check-qtest-x86_64" would have rebuilt them, but I'm
confused by the results.
Paolo
Re: A brief look at deprecating our JSON extensions over RFC 8259
On 22/02/21 15:57, Markus Armbruster wrote: * The block layer's pseudo-protocol "json:" (which can get embedded in image headers) If it gets embedded in image headers, I don't think we'll be able to deprecate it ever. We'd need to keep a converter for old images, at which point it's simpler to keep the extensions. Paolo
Re: [PATCH] qemu: Replace deprecated short-form boolean options
On 03/02/21 09:40, Han Han wrote: Hi Paolo, I find there is no warning for the nolazy_refcounts option(qemu v5.2.0-1530-g74208cd252): $ qemu-img create /tmp/b.qcow2 -f qcow2 10M -o nolazy_refcounts Formatting '/tmp/b.qcow2', fmt=qcow2 cluster_size=65536 extended_l2=off compression_type=zlib size=10485760 lazy_refcounts=off refcount_bits=16 Could you please help to check if this short-form boolean option is missing in the commit "ccd3b3b811 qemu-option: warn for short-form boolean options" This is indeed a slightly different path that doesn't warn yet, but I suggest changing it anyway. It will work with all versions of QEMU. Paolo
Re: [PATCH] qemu: Replace deprecated short-form boolean options
On 26/01/21 04:55, Han Han wrote:
Since the commit ccd3b3b811 of QEMU, the short-form boolean options in
qemu cmdline like "server", "nowait", "disable-ticketing" are
deprecated
There are a few more:
1) -vnc password, -vnc tls, -vnc sasl:
if (graphics->data.vnc.auth.passwd || cfg->vncPassword)
virBufferAddLit(, ",password");
if (cfg->vncTLS) {
qemuDomainGraphicsPrivatePtr gfxPriv =
QEMU_DOMAIN_GRAPHICS_PRIVATE(graphics);
if (gfxPriv->tlsAlias) {
...
} else {
virBufferAddLit(, ",tls");
...
}
}
if (cfg->vncSASL) {
virBufferAddLit(, ",sasl");
if (cfg->vncSASLdir)
virCommandAddEnvPair(cmd, "SASL_CONF_PATH", cfg->vncSASLdir);
/* TODO: Support ACLs later */
}
"-vnc tls" is only used for old QEMU, but I think it's cleaner to change
it as well.
2) -chardev telnet
virBufferAsprintf(,
"socket,id=%s,host=%s,port=%s%s",
charAlias,
dev->data.tcp.host,
dev->data.tcp.service,
telnet ? ",telnet" : "");
3) -fsdev readonly:
if (fs->readonly)
virBufferAddLit(, ",readonly");
4) -spice sasl:
if (cfg->spiceSASL) {
virBufferAddLit(, "sasl,");
if (cfg->spiceSASLdir)
virCommandAddEnvPair(cmd, "SASL_CONF_PATH",
cfg->spiceSASLdir);
/* TODO: Support ACLs later */
}
5) qemu-img create:
if (info->features && info->format == VIR_STORAGE_FILE_QCOW2) {
if (virBitmapIsBitSet(info->features,
VIR_STORAGE_FILE_FEATURE_LAZY_REFCOUNTS)) {
if (STREQ_NULLABLE(info->compat, "0.10")) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("lazy_refcounts not supported with compat"
" level %s"),
info->compat);
return -1;
}
virBufferAddLit(, "lazy_refcounts,");
}
}
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 2d1f5ea5f5..97954bcc37 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -872,7 +872,7 @@ xenParseSxprChar(const char *value,
else
def->source->data.tcp.service = g_strdup(offset);
-if (offset2 && strstr(offset2, ",server"))
+if (offset2 && strstr(offset2, ",server=on"))
def->source->data.tcp.listen = true;
}
break;
@@ -924,7 +924,7 @@ xenParseSxprChar(const char *value,
def->source->data.nix.path = g_strdup(value);
if (offset != NULL &&
-strstr(offset, ",server") != NULL)
+strstr(offset, ",server=on") != NULL)
def->source->data.nix.listen = true;
}
break;
As far as I understand it, it is valid to start a domain with "xl" and
inspect it with "virsh dumpxml". So I wouldn't change this, as it
depends on whatever xl has placed in the value you are parsing.
Thanks,
Paolo
Re: [PATCH RFC 1/2] qemu: use "-accel" option to specify accelerator instead of "-machine"
On 11/01/21 10:38, Daniel P. Berrangé wrote: The "-machine" options for accelerators are legacy, the "-accel" options is a better mechanism. The following are the details: https://lore.kernel.org/qemu-devel/3aa73987-40e8-3619-0723-9f17f7385...@redhat.com/ This patch switch the option "-machine accel=xxx" to "-accel xxx" when specifying accelerator type once libvirt build QEMU command line. This is going to break use of the /usr/bin/qemu-kvm wrapper on Fedora because QEMU refuses to allow -accel combined with -machine $ /usr/bin/qemu-kvm -accel tc qemu-system-x86_64: The -accel and "-machine accel=" options are incompatible That script is useless, a symlink will do ever since QEMU 4.0. Fedora can and should get rid of it, I'll take a look. Is there any other distro that does the same? Libvirt cannot use newer KVM features with "-machine accel". Paolo
Re: [PATCH RFC 1/2] qemu: use "-accel" option to specify accelerator instead of "-machine"
On 11/01/21 10:27, Jiri Denemark wrote: We use a fallback for capabilities probing (in qemuProcessQMPLaunch) so in case the old -machine type,accel=... way is going to be deprecated, Most likely not. We've gotten better at making the "old ways" just syntactic sugar for the "new ways" without keeping messy code around the whole codebase, so deprecation is not always that appealing. Paolo we need to be able to detect whether multiple -accel options are allowed and use -machine otherwise.
Re: [PATCH RFC 1/2] qemu: use "-accel" option to specify accelerator instead of "-machine"
On 11/01/21 09:35, Ján Tomko wrote:
This unfortunately cannot be done unconditionally. You need to probe
for the availability of -accel, using something like
What are we probing for?
Existence of "-accel". "-accel" allows configuration of
accelerator-specific, machine-independent properties. In the past they
went in -machine but the two have been separate since 5.0 (with
backwards-compatible glue).
There were two phases of -accel support in QEMU:
- 2.9 to 4.2: only one -accel option supported; specifying a fallback
couldn't be done with -accel and required the older "-machine
accel=tcg:kvm" instead
- 5.0 or newer: multiple -accel options supported, e.g. "-accel tcg
-accel kvm"
and it would be possible to distinguish them, for example using QOM
properties. However Libvirt only ever specifies one accelerator so it
makes no difference.
That said, my example patch was wrong. It's
+{ "accel", NULL, QEMU_CAPS_ACCEL },
instead.
The minimal QEMU version supported by libvirt is 1.5.3 and I'm seeing
some test cases using -machine pc-i440fx-1.5,accel=tcg in our test
suite.
Indeed a fallback is needed, that was my review remark. :)
And I don't see any explicit use of -accel in this patch.
It's right in qemuBuildAccelCommandLine:
+virCommandAddArg(cmd, "-accel");
+
+switch ((virDomainVirtType)def->virtType) {
+case VIR_DOMAIN_VIRT_QEMU:
+virBufferAddLit(, "tcg");
+break;
Thanks,
Paolo
Re: [PATCH RFC 1/2] qemu: use "-accel" option to specify accelerator instead of "-machine"
On 09/01/21 19:58, huang...@chinatelecom.cn wrote:
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 6f970a3..9a64473 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6711,40 +6711,6 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
virCommandAddArg(cmd, "-machine");
virBufferAdd(, def->os.machine, -1);
-switch ((virDomainVirtType)def->virtType) {
-case VIR_DOMAIN_VIRT_QEMU:
-virBufferAddLit(, ",accel=tcg");
-break;
-
-case VIR_DOMAIN_VIRT_KVM:
-virBufferAddLit(, ",accel=kvm");
-break;
-
-case VIR_DOMAIN_VIRT_KQEMU:
-case VIR_DOMAIN_VIRT_XEN:
-case VIR_DOMAIN_VIRT_LXC:
-case VIR_DOMAIN_VIRT_UML:
-case VIR_DOMAIN_VIRT_OPENVZ:
-case VIR_DOMAIN_VIRT_TEST:
-case VIR_DOMAIN_VIRT_VMWARE:
-case VIR_DOMAIN_VIRT_HYPERV:
-case VIR_DOMAIN_VIRT_VBOX:
-case VIR_DOMAIN_VIRT_PHYP:
-case VIR_DOMAIN_VIRT_PARALLELS:
-case VIR_DOMAIN_VIRT_BHYVE:
-case VIR_DOMAIN_VIRT_VZ:
-case VIR_DOMAIN_VIRT_NONE:
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("the QEMU binary does not support %s"),
- virDomainVirtTypeToString(def->virtType));
-return -1;
-
-case VIR_DOMAIN_VIRT_LAST:
-default:
-virReportEnumRangeError(virDomainVirtType, def->virtType);
-return -1;
-}
-
/* To avoid the collision of creating USB controllers when calling
* machine->init in QEMU, it needs to set usb=off
*/
@@ -6945,6 +6911,52 @@ qemuBuildMachineCommandLine(virCommandPtr cmd,
return 0;
}
+static int
+qemuBuildAccelCommandLine(virCommandPtr cmd,
+ const virDomainDef *def)
+{
+/* the '-machine' options for accelerator are legacy,
+ * using the '-accel' options by default */
+g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+virCommandAddArg(cmd, "-accel");
+
+switch ((virDomainVirtType)def->virtType) {
+case VIR_DOMAIN_VIRT_QEMU:
+virBufferAddLit(, "tcg");
+break;
+
+case VIR_DOMAIN_VIRT_KVM:
+virBufferAddLit(, "kvm");
+break;
+
+case VIR_DOMAIN_VIRT_KQEMU:
+case VIR_DOMAIN_VIRT_XEN:
+case VIR_DOMAIN_VIRT_LXC:
+case VIR_DOMAIN_VIRT_UML:
+case VIR_DOMAIN_VIRT_OPENVZ:
+case VIR_DOMAIN_VIRT_TEST:
+case VIR_DOMAIN_VIRT_VMWARE:
+case VIR_DOMAIN_VIRT_HYPERV:
+case VIR_DOMAIN_VIRT_VBOX:
+case VIR_DOMAIN_VIRT_PHYP:
+case VIR_DOMAIN_VIRT_PARALLELS:
+case VIR_DOMAIN_VIRT_BHYVE:
+case VIR_DOMAIN_VIRT_VZ:
+case VIR_DOMAIN_VIRT_NONE:
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("the QEMU binary does not support %s"),
+ virDomainVirtTypeToString(def->virtType));
+return -1;
+
+case VIR_DOMAIN_VIRT_LAST:
+default:
+virReportEnumRangeError(virDomainVirtType, def->virtType);
+return -1;
+}
+
+virCommandAddArgBuffer(cmd, );
+return 0;
+}
static void
qemuBuildTSEGCommandLine(virCommandPtr cmd,
@@ -9840,6 +9852,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
if (qemuBuildMachineCommandLine(cmd, cfg, def, qemuCaps, priv) < 0)
return NULL;
+if (qemuBuildAccelCommandLine(cmd, def) < 0)
+return NULL;
+
qemuBuildTSEGCommandLine(cmd, def);
This unfortunately cannot be done unconditionally. You need to probe
for the availability of -accel, using something like
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 4d132defbd..23efaccbad 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -609,6 +609,7 @@ VIR_ENUM_IMPL(virQEMUCaps,
"ncr53c90",
"dc390",
"am53c974",
+ "accel",
);
@@ -3220,6 +3221,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ "fsdev", "multidevs", QEMU_CAPS_FSDEV_MULTIDEVS },
{ "fw_cfg", "file", QEMU_CAPS_FW_CFG },
{ "fsdev", "fmode", QEMU_CAPS_FSDEV_CREATEMODE }, /* Could have
also checked fsdev->dmode */
+{ "accel", "accel", QEMU_CAPS_ACCEL },
};
static int
Then you can choose between the two code paths using
virQEMUCapsGet(qemuCaps, QEMU_CAPS_ACCEL).
Thanks,
Paolo
Re: [PATCH 0/4] Remove deprecated CLI parameters
On 10/12/20 16:58, Thomas Huth wrote: Remove some simple CLI parameters that have been deprecated since at least two releass already. Philippe Mathieu-Daudé (1): accel/tcg: Remove deprecated '-tb-size' option Thomas Huth (3): docs/system: Move the list of removed features to a separate file Remove the deprecated -realtime option Remove the deprecated -show-cursor option accel/tcg/translate-all.c | 2 +- docs/system/deprecated.rst | 246 docs/system/removed-features.rst| 243 +++ qemu-options.hx | 29 +--- softmmu/vl.c| 45 + tests/migration/guestperf/engine.py | 2 +- 6 files changed, 247 insertions(+), 320 deletions(-) create mode 100644 docs/system/removed-features.rst Queued, thanks. paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 03/12/20 18:52, Eduardo Habkost wrote: On Thu, Dec 03, 2020 at 05:50:46PM +0100, Paolo Bonzini wrote: On 03/12/20 16:15, Kevin Wolf wrote: I don't think this is an intermediate state like Eduardo wants to have. Creating the object, then setting properties, then realize [1] will fail after your change. But keeping it working was the whole point of the exercise. With the sample code, you must remove object_class_property_set calls at the Do you mean object_property_set()? Yes. same time as you remove the setters. Usually that'd be when you convert to QAPI and oc->configure, but it doesn't have to be that way if there are good reasons not to do so. Having two (or more) similar but incompatible APIs to do exactly the same thing is a mistake we did before, and I wouldn't like us to repeat it. If we can keep qdev_new() + object_property_set() + realize working after the device is converted, we should. I believe we can. You can. If you want to do that, you have to give up on removing the setters; but that's not so beneficial for devices because they already use static properties anyway. They have much less boilerplate than -object objects. If we can make object_new_configure() work with all (or most) device types before we manually convert them to the new system, we should. I believe we can. Yup, object_new_configure() is the low-level visitor-based API and therefore it supports both properties and oc->configure. We may be able avoid these questions with -object because converting all backends at the same time is doable. With devices, API usability and maintainability during the transition period (which could be very long) needs to be taken into account. I think we're in violent agreement. :) Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 03/12/20 18:43, Kevin Wolf wrote: I think I'd want to do step 2 and 3 combined, because converting user-creatable objects to oc->configure means manually writing the configure function that will be generated from QAPI in step 3. Writing code just to throw it away isn't my favourite pastime. It would only be a couple lines of extra code, but yeah you don't have to do it. It still is clearer to show the steps one by one as they are logically separate and it shows what (modulo inlining) the generated code ends up doing. That said having no setter might simplify the introduction of field properties too (no allow_set to worry about); perhaps that's a good reason to do the oc->configure conversion earlier rather than later, especially if QAPI code generation ends up taking a bit longer. Another good reason is to make sure the API is stable before moving to generated code, especially with respect to inheritance. Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 03/12/20 16:15, Kevin Wolf wrote:
I don't think this is an intermediate state like Eduardo wants to have.
Creating the object, then setting properties, then realize [1] will fail
after your change. But keeping it working was the whole point of the
exercise.
With the sample code, you must remove object_class_property_set calls at
the same time as you remove the setters. Usually that'd be when you
convert to QAPI and oc->configure, but it doesn't have to be that way if
there are good reasons not to do so.
Also, it still allows you to do so one class at a time, and I *think*
the presence of subclasses or superclasses doesn't matter (only whether
properties are still writable). We can use chardevs (see ChardevCommon
in qapi/char.json) to validate that before tackling devices.
(In fact, this means that your series---plus -object and object_add
conversion---would be good, pretty much unchanged, as a first step. The
second would be adding oc->configure and object_configure, and
converting all user-creatable objects to oc->configure. The third would
involve QAPI code generation).
I'm also not really sure why you go from RngEgdOptions to QObject to a
visitor, only to reconstruct RngEgdOptions at the end.
The two visits are just because you cannot create an input visitor
directly on C data. I stole that from your patch 18/18 actually, just
with object_new+object_configure instead of user_creatable_add_type.
But I wouldn't read too much in the automatically-generated *_new
functions since they are already in QAPI code generator territory.
Instead the basic object_configure idea can be applied even without
having automatic code generation.
I think the class
implementations should have a normal C interface without visitors and we
should be able to just pass the existing RngEgdOptions object (or the
individual values for its fields for 'boxed': false).
Sure, however that requires changes to the QAPI code generator which was
only item (3) in your list list. Until then you can already work with a
visitor interface:
void rng_egd_configure(Object *obj, Visitor *v, Error **errp)
{
RngEgd *s = RNG_EGD(obj);
s->config = g_new0(MemoryBackendOptions, 1);
visit_type_MemoryBackendOptions(v, NULL, >config, errp);
s->config->share = (s->config->has_share
? s->config->share : false);
...
}
but if you had a QAPI description
{ 'object': 'RngEgd',
'qom-type': 'rng-egd',
'configuration': 'RngEgdOptions',
'boxed': true
}
the QAPI generator could produce the oc->configure implementation.
Similar to commands, that implementation would be an unmarshaling
wrapper that calls out to the natural C interface:
void qapi_RngEgd_configure(Object *obj, Visitor *v, Error **errp);
{
Error *local_err = NULL;
g_autoptr(MemoryBackendOptions) *config =
g_new0(MemoryBackendOptions, 1);
visit_type_MemoryBackendOptions(v, NULL, >config, _err);
if (local_err) {
error_propagate(errp, local_err);
return;
}
qom_rng_egd_configure(RNG_EGD(obj), config, errp);
}
void qom_rng_egd_configure(RngEng *s,
RngEgdOptions *config,
Error **errp)
{
config->share = (config->has_share
? config->share : false);
...
s->config = QAPI_CLONE(RngEgdOptions, config);
}
Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 02/12/20 18:35, Kevin Wolf wrote: Could we have an intermediate state that doesn't require any duplication and thus have no separate code paths that could diverge? The one requirement we have for an intermediate state is that it supports both interfaces: The well-know create/set properties/realize dance, and a new DeviceClass method, say .create(), that takes the configuration in parameters instead of relying on previously set properties. I assumed two separate implementations of transferring the configuration into the internal state. On second thought, this assumption is maybe wrong. You can implement the new method as wrapper around the old way: It could just set all the properties and call realize. Of course, you don't win much in terms of improving the class implementation this way, but just support the new interface, but I guess it can be a reasonable intermediate step to resolve complicated dependencies etc. I sketched something at https://wiki.qemu.org/Features/QOM-QAPI_integration. The main difference with what we discussed so far is that it doesn't subsume the complete/realize step, only the setting of properties. The idea is that user_creatable_add_type does the following: - calls an oc->configure method on every superclass of the object - takes what's left of the input visitor and uses it to set properties - then calls ucc->complete So in the end the only new step is the first. If the first two steps are bundled in a new function object_configure, they can be reused for devices, machines and accelerators. The QAPI code generator can also easily wrap them into a C API for QOM object creation. I glossed completely over the generation of properties within the QAPI code generator. Making properties read-only (or, in the field-properties world, having a default allow_set of "return false") is already a nice improvement over It would be much nicer to do the wrapper the other way round, i.e. setting properties before the device is realized would update a configuration struct and realize would then call .create() with that struct. To me, this sounds much harder, though also a more useful state. This sounds much harder. However, based on the RngEgd sample, going from a basic QAPI struct to a full conversion is not too hard and makes code shorter. So it's win-win even though it's human work. Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 02/12/20 13:51, Eduardo Habkost wrote:
I'm liking the direction this is taking. However, I would still
like to have a clearer and feasible plan that would work for
-device, -machine, and -cpu.
-cpu is not a problem since it's generally created with a static
configuration (now done with global properties, in the future it could be a
struct).
It is a problem if it requires manually converting existing code
defining CPU properties and we don't have a transition plan.
We do not have to convert everything _if_ for some objects there are
good reasons to do programmatically-generated properties. CPUs might be
one of those cases (or if we decide to convert them, they might endure
some more code duplication than other devices because they have so many
properties).
Would a -device conversion also involve non-user-creatable
devices, or would we keep existing internal usage of QOM
properties?
Even if it's just for user-creatable devices, getting rid of QOM
property usage in devices sounds like a very ambitious goal. I'd
like us to have a good transition plan, in addition to declaring
what's our ideal end goal.
For user-creatable objects Kevin is doing work in lockstep on all
classes; but once we have the infrastructure for QAPI object
configuration schemas we can proceed in the other direction and operate
on one device at a time.
With some handwaving, something like (see create_unimplemented_device)
DeviceState *dev = qdev_new(TYPE_UNIMPLEMENTED_DEVICE);
qdev_prop_set_string(dev, "name", name);
qdev_prop_set_uint64(dev, "size", size);
sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
might become something like
{ 'object': 'unimplemented-device',
'config': {
'name': 'str',
'size': 'size'
},
}
DeviceState *dev = qapi_Unimplemented_new(&(
(UnimplementedDeviceOptions) {
.name = name,
.size = size
}, _fatal);
object_unref(dev);
(i.e. all typesafe!) and the underlying code would be something like
void (*init_properties)(Object *obj, Visitor *v, Error **errp);
...
// QAPI generated constructor
UnimplementedDevice *qapi_Unimplemented_new(
UnimplementedDeviceOptions *opt, Error **errp)
{
Object *obj = object_new(TYPE_UNIMPLEMENTED_DEVICE);
if (!qapi_Unimplemented_init_properties(obj, opt, errp)) {
object_unref(obj);
return NULL;
}
return obj;
}
// QAPI generated Visitor->struct adapter
bool qapi_Unimplemented_init_properties(
Object *obj, Visitor *v, Error **errp)
{
UnimplementedDeviceOptions opt;
Error *local_err = NULL;
visit_type_UnimplementedDeviceOptions(v, NULL,
, _err);
if (local_err) {
error_propagate(errp, local_err);
return false;
}
unimplemented_init_properties(UNIMPLEMENTED_DEVICE(obj),
, _err);
if (local_err) {
error_propagate(errp, local_err);
return false;
}
return true;
}
// Hand-written (?) initializer:
void unimplemented_init_properties(Object *obj,
UnimplementedDeviceOptions *opt, Error **errp)
{
obj->name = name;
obj->size = size;
device_realize(obj, errp);
}
// class_init code:
oc->init_properties = qapi_Unimplemented_init_properties;
and all read-only-after-realize QOM properties could be made *really*
read-only.
Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 02/12/20 11:27, Kevin Wolf wrote: Declaring read-only QOM properties is trivial. Trivial sounds like it's something the computer should be doing. Possibly, but not necessarily. There's always a cost to automatic code generation. If things are _too_ trivial and easy to get right, the cost of automatic code generation exceeds the cost of doing things by hand. You pretty much proved that ucc->complete is hard enough to get right, that it benefits from code generation. But I am a bit more conservative about extending that to the rest of QOM. I'm just worried about having yet another incomplete transition. Would you really feel at home in a QEMU without incomplete transitions? One can always dream. :) But since you had already posted RFC patches a while ago to address this, I considered it solved. Yup, I posted the non-RFC today. 1. This series (mostly for documentation and introspection) 2. -object and HMP object_add (so that we get QAPI's validation, and to make sure that forgetting to update the schema means that the new options just doesn't work) 3. Create a separate 'object' entity in QAPI, generate ucc->complete implementations that call a create function in the C source code with type-specific parameters (like QMP command handlers) If we agree on all of these that's already a pretty good place to be in. The decreasing length of the replies to the thread suggests that we are. I wouldn't mind an example of (3) that is hand-coded and may only work for one object type (even a simple one such as iothread), to show what the generated QAPI code would look like. It's not a blocker as far as I am concerned, but it would be nice. More important, Markus and John should agree with the plan before (1) is committed. That may also require the aforementioned example, but it's up to them. What's still open: Should QAPI cover run-time properties, too? Should run-time properties even exist at all in the final state? What is the exact QAPI representation of everything? (The last one includes that I need to have a closer look at how QAPI can best be taught about inheritance.) Run-time properties will exist but they will probably be cut down substantially, and most of them will be read-only (they already are as far as external code is concerned, i.e. they have a setter but qom-set always fails because it's called too late). Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 02/12/20 11:38, Kevin Wolf wrote: Am 02.12.2020 um 10:30 hat Paolo Bonzini geschrieben: On 01/12/20 23:08, Eduardo Habkost wrote: Properties are only a useful concept if they have a use. If -object/object_add/object-add can do the same job without properties, properties are not needed anymore. Do you mean "not needed for -object anymore"? Properties are still used by internal C code (esp. board code), -device/device_add, -machine, -cpu, and debugging commands (like "info qtree" and qom-list/qom-get/qom-set). Yes. Are internal uses mostly just right after object creation, or do we make a lot of use of them during runtime? Not "a lot" but enough to be nasty. There are a couple uses of "well-known" QOM properties (e.g. to access the RTC time or the balloon stats), plus "info qtree". Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 01/12/20 23:08, Eduardo Habkost wrote:
Properties are only a useful concept if they have a use. If
-object/object_add/object-add can do the same job without properties,
properties are not needed anymore.
Do you mean "not needed for -object anymore"? Properties are
still used by internal C code (esp. board code),
-device/device_add, -machine, -cpu, and debugging commands (like
"info qtree" and qom-list/qom-get/qom-set).
Yes.
Right now QOM is all about exposing properties, and having multiple
interfaces to set them (by picking a different visitor). But in practice
most QOM objects have a lifetime that consists of 1) set properties 2) flip
a switch (realized/complete/open) 3) let the object live on its own. 1+2
are a single monitor command or CLI option; during 3 you access the object
through monitor commands, not properties.
I agree with this, except for the word "all" in "QOM is all
about". QOM is also an extensively used internal QEMU API,
including internal usage of the QOM property system.
Yeah, "all about exposing properties" includes internal usage. And
you're right that some "phase 3" monitor commands do work at the
property level (mostly "info qtree", but also "qom-get" because there
are some cases of public run-time properties).
I'm liking the direction this is taking. However, I would still
like to have a clearer and feasible plan that would work for
-device, -machine, and -cpu.
-cpu is not a problem since it's generally created with a static
configuration (now done with global properties, in the future it could
be a struct).
-machine and -device in principle could be done the same way as -object,
just through a different registry (_not_ a huge struct; that's an
acceptable stopgap for -object but that's it). The static aka field
properties would remain as read-only, with defaults moved to
instance_init or realize. But there would be again "triplication" with
a trivial conversion:
1) in the QAPI schema, e.g. 'num_queues': 'int16'
2) in the struct, "int16_t num_queues;"
3) in the realize function,
s->num_queues = cfg->has_num_queues ? cfg->num_queues : 8;
So having a mechanism for defaults in the QAPI schema would be good.
Maybe 'num_queues': { 'type': 'int16', 'default': '8' }?
I also need to review more the part of this code with respect to the
application of global properties. I wonder if there are visitor tricks
that we can do, so that global properties keep working but correspond to
QAPI fields instead of QOM properties.
Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 01/12/20 20:35, Kevin Wolf wrote: Am 01.12.2020 um 18:16 hat Paolo Bonzini geschrieben: I don't think this is actually a new things. We already have types and commands declared with things like 'if': 'defined(TARGET_S390X)'. As far as I understand, QAPI generated files are already built per target, so not compiling things into all binaries should be entirely possible. There is some complication due to having different discriminators per target. So yes it should be possible. But probably best left after objects because it's so much bigger a task and because objects have a bit more freedom for experimentation (less ties to other qdev-specific concepts, e.g. the magic "bus" property). So maybe only the abstract base class that actually defines the machine properties (like generic-pc-machine) should be described in QAPI, and then the concrete machine types would inherit from it without being described in QAPI themselves? Yes, maybe. 1) whether to generate _all_ boilerplate or only properties I would like to generate as much boilerplate as possible. That is, I don't want to constrain us to only properties, but at the same time, I'm not sure if it's possible to get rid of all boilerplate. Basically, the vision I have in mind is that QAPI would generate code that is the same for most instances, and then provide an option that prevents code generation for a specific part for more complicated cases, so that the respective function can (and must) be provided in the C source. Ok, so that's a bit different from what I am thinking of. I don't care very much about the internal boilerplate, only the external interface for configuration. So I don't care about type registration, dynamic cast macros etc., only essentially the part that leads to ucc->complete. 2) whether we want to introduce a separation between configuration schema and run-time state You mean the internal run-time state? How is this separation not already present with getter/setter functions for each property? In many cases they just directly access the run-time state, but there are other cases where they actually do things. I mean moving more towards the blockdev/chardev way of doing things, increasing the separation very much by having separate configuration structs that have (potentially) no link to the run-time state struct. 3) in the latter case, whether properties will survive at all---iothread and throttle-groups don't really need them even if they're writable after creation. How do you define properties, i.e. at which point would they stop existing and what would be a non-property alternative? Properties are only a useful concept if they have a use. If -object/object_add/object-add can do the same job without properties, properties are not needed anymore. Right now QOM is all about exposing properties, and having multiple interfaces to set them (by picking a different visitor). But in practice most QOM objects have a lifetime that consists of 1) set properties 2) flip a switch (realized/complete/open) 3) let the object live on its own. 1+2 are a single monitor command or CLI option; during 3 you access the object through monitor commands, not properties. So in summary, it seems to me that the QOM way is more flexible because you can get both models out of it. Whether we actually need this flexibility I can't say. I'm thinking there's no need for it, but maybe I'm overly optimistic. * Configuration options are described in the QAPI schema. This is mainly for object creation, but runtime modifiable properties are a subset of this. * Properties are generated for each option. By default, the getter just returns the value from the configuration at creation time, though generation of it can be disabled so that it can be overridden. Also, setters just return an error by default. * Property setters aren't called for object creation. Instead, the relevant ObjectOptions branch is made available to some init method. * Runtime modifiable properties (declared as such in the schema) don't get the default setter, so you have to provide an implementation for them. I wouldn't bother with properties at all in the QAPI schema. Just do the first and third bullet. Declaring read-only QOM properties is trivial. So while this series is doing only one part of the whole solution, that the second part is missing doesn't make the first part wrong. Yeah, I think it's clear that for the long term we're not really disagreeing (or perhaps I'm even more radical than you :)). I'm just worried about having yet another incomplete transition. One possibly nasty detail to consider there is that we sometimes declare the USER_CREATABLE interface in the base class, so ucc->complete is for the base class rather than the actually instantiated class. If we only instantiate leaf classes (I think this is true), we can move USER_CREATABLE there. You can also use a whi
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 01/12/20 17:20, Kevin Wolf wrote: Am 30.11.2020 um 20:35 hat Paolo Bonzini geschrieben: For devices it's just the practical issue that there are too many to have something like this series. For machine types the main issue is that the version-specific machine types would have to be defined in one more place. Sure, the number of devices means that we can't convert all of them at once. And we don't need to, we can make the change incrementally. There's also the question that most devices are not present in all binaries. So QAPI introspection would tell you what properties are supported but not what devices are. Also the marshaling/unmarshaling code for hundreds of devices would bloat the QEMU executables unnecessarily (it'd all be reachable from visit_DeviceOptions so it'd not be possible to compile it out, even with LTO). Plus the above issue with machine types. The single struct doesn't bother me _too much_ actually. What bothers me is that it won't be a single source of all QOM objects, only those that happen to be created by object-add. But isn't it only natural that a list of these objects will exist in some way, implicitly or explicitly? object-add must somehow decide which object types it allows the user to create. There's already one, it's objects that implement user creatable. I don't think having it as a QAPI enum (or discriminator) is worthwhile, and it's one more thing that can go out of sync between the QAPI schema and the C code. Well, we all know that this series duplicates things. But at the same time, we also know that this isn't going to be the final state. Once QAPI learns about QOM inheritance (which it has to if it should generate the boilerplate), it will know which objects are user creatable without a an explicitly defined separate enum. I think it will still need to have the enum internally, but as long as it's automatically generated, that shouldn't be a big deal. Right, I don't want to have the final state now but I'd like to have a rough idea of the plan: 1) whether to generate _all_ boilerplate or only properties 2) whether we want to introduce a separation between configuration schema and run-time state 3) in the latter case, whether properties will survive at all---iothread and throttle-groups don't really need them even if they're writable after creation. These questions have a substantial effect on how to handle this series. Without answers to this questions I cannot understand the long term and its interaction with other long term plans for QOM. A modified QOM might be the right solution, though. I would like to bring QAPI and QOM together because most of these weaknesses are strengths of QAPI. I agree wholeheartedly. But your series goes to the opposite excess. Eduardo is doing work in QOM to mitigate the issues you point out, and you have to meet in the middle with him. Starting with the user-visible parts focuses on the irrelevant parts. QAPI is first and foremost about user-visible parts, and in fact most of the problems I listed are about external interfaces. Yes, but QAPI is also about interfacing with existing code. Also, QAPI does not generate only structs, it also generate C function prototypes. I'm not sure whether a QOM object's more similar to the struct case (what you do here) or to the QMP command case: - there's a 1:1 correspondence between ObjectOptions cases and ucc->complete implementations - there's a registry of object types just like there's one for QMP commands. So another possibility could be that the QAPI generator essentially generated the user_creatable_add_type function that called out to user-provided functions qom_scsi_pr_helper_complete, qom_throttle_group_complete, etc. The arguments to those functions would be the configuration. That is a very interesting prospect (though one that would require changes to the QAPI code generator). BlockdevOptions is about external interfaces, not about implementation details. Same thing as QOM properties are external interfaces, not implementation details. There may be fields in the internal state that correspond 1:1 to the externally visible QAPI type/QOM property, but it's not necessarily the case. Right. It may well be that we decide that, as a result of this series, QOM's property interface is declared essentially a failed experiment. I wouldn't be surprised, and that's why I want to understand better where we want to go. For example, Eduardo is focusing specifically on external interfaces that correspond 1:1 to the internal implementation. If we decide that non-1:1-mappings and checks on mandatory properties are an important part of QOM, that may make large parts of his work moot. If we decide that most QOM objects need no properties at all, then we don't want to move more qdev-specific stuff from to QOM. QAPI is already here and it's going to stay. QOM doesn't have to duplicate input validation that exist
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 30/11/20 19:10, Kevin Wolf wrote: Am 30.11.2020 um 17:57 hat Paolo Bonzini geschrieben: The main problem is that it wouldn't extend well, if at all, to machines and devices. So those would still not be integrated into the QAPI schema. What do you think is the biggest difference there? Don't devices work the same as user creatable objects in that you first set a bunch of properties (which would now be done through QAPI instead) and then call the completion/realize method that actually makes use of them? For devices it's just the practical issue that there are too many to have something like this series. For machine types the main issue is that the version-specific machine types would have to be defined in one more place. The single struct doesn't bother me _too much_ actually. What bothers me is that it won't be a single source of all QOM objects, only those that happen to be created by object-add. But isn't it only natural that a list of these objects will exist in some way, implicitly or explicitly? object-add must somehow decide which object types it allows the user to create. There's already one, it's objects that implement user creatable. I don't think having it as a QAPI enum (or discriminator) is worthwhile, and it's one more thing that can go out of sync between the QAPI schema and the C code. I'm also pretty sure that QOM as it exists now is not the right solution for any of them because it has some major shortcomings. It's too easy to get things wrong (like the writable properties after creation), its introspection is rather weak and separated from the QAPI introspection, it doesn't encourage documentation, and it involves quite a bit of boilerplate and duplicated code between class implementations. A modified QOM might be the right solution, though. I would like to bring QAPI and QOM together because most of these weaknesses are strengths of QAPI. I agree wholeheartedly. But your series goes to the opposite excess. Eduardo is doing work in QOM to mitigate the issues you point out, and you have to meet in the middle with him. Starting with the user-visible parts focuses on the irrelevant parts. Mostly because it's more or less the same issue that you have with BlockdevOptions, with the extra complication that this series only deals with the easy one of the four above categories. I'm not sure which exact problem with BlockdevOptions you mean. The reason why the block layer doesn't use BlockdevOptions everywhere is -drive support. You don't have a single BlockdevOptions* field in the structs of block/. My interpretation of this is that BlockdevOptions* is a good schema for configuring things but not for run-time state. Maybe if we don't want to commit to keeping the ObjectOptions schema, the part that should wait is object-add and I should do the command line options first? Then the schema remains an implementation detail for now that is invisible in introspection. I don't see much benefit in converting _any_ of the three actually. The only good reason I see for QAPIfying this is the documentation, and the promise of deduplicating QOM boilerplate. The latter is only a promise, but documentation alone is a damn good reason and it's enough to get this work into a mergeable shape as soon as possible! I think having some input validation done by QAPI instead of in each QOM object individually is nice, too. You get it after CLI, QMP and HMP all go through QAPI. But the right way to do that is to use Eduardo's field properties: they make QOM do the right thing, adding another layer of parsing is just adding more complexity. Are there any validation bugs that you're fixing? Is that something that cannot be fixed elsewhere, or are you papering over bad QOM coding? (Again, I'm not debating that QOM properties are hard to write correctly). But maybe, we could start in the opposite direction: start with the use QAPI to eliminate QOM boilerplate. Basing your work on Eduardo's field properties series, you could add a new 'object' "kind" to QAPI that would create an array of field properties (e.g. a macro expanding to a compound literal?) There is a very simple reason why I don't want to start with the QAPI generator: John has multiple series pending that touch basically every part of the QAPI generator. This means not only that I need to be careful about merge conflict (or base my work on top of five other series, which feels adventurous), but also that I would be competing with John for Markus' reviewer capacity, further slowing things down. That's something that you have to discuss with John and Markus. It may be that John has to shelve part of his work or rebase on top of yours instead. By adding an extra parsing layer you're adding complexity that may not be needed in the end, and we're very bad of removing it later. So I'm worried that this series will become technical debt in just a few months. Well, two rea
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 30/11/20 16:46, Kevin Wolf wrote:
Am 30.11.2020 um 15:58 hat Paolo Bonzini geschrieben:
With this series it's basically pointless to have QOM properties at
all.
Not entirely, because there are still some writable properties that can
be changed later on.
Are there really any (that are not bugs like opened/loaded)? That's also
why Eduardo and I discussed a class-wide allow_set function for his
field properties series.
So with this in mind, I think I'm in favour of completely leaving the
initialisation of properties on object creation to QAPI, and only
providing individual setters if we actually intend to allow property
changes after creation.
The main problem is that it wouldn't extend well, if at all, to machines
and devices. So those would still not be integrated into the QAPI schema.
So the question is, are we okay with shoveling half of QEMU's backend data
model into a single struct? If so, there are important consequences.
Yeah, the single struct bothers me a bit, both in the QAPI schema and in
the C source.
The single struct doesn't bother me _too much_ actually. What bothers
me is that it won't be a single source of all QOM objects, only those
that happen to be created by object-add. So I start to wonder if QOM as
it exists now is the right solution for all kind of objects:
- backends and other object-add types (not per-target, relatively few
classes and even fewer hierarchies)
- machine (per-target, many classes, no hierarchy)
- device (can be treated as not per-target, many many classes, very few
hierarchies)
- accelerator (per-target, few classes, no hierarchy)
- chardev (ok those are the same as the first category)
If QOM is the right solution, this patch goes in the wrong direction.
If QOM is the wrong solution, this patch is okay but then we have
another problem to solve. :)
The problem with this series is that you are fine with deduplicating things
as a third step, but you cannot be sure that such deduplication is possible
at all. So while I don't have any problems in principle with the
ObjectOptions concept, I don't think it should be committed without a clear
idea of how to do the third step.
Do you have any specific concerns why the deduplication might not
possible, or just because it's uncharted territory?
Mostly because it's more or less the same issue that you have with
BlockdevOptions, with the extra complication that this series only deals
with the easy one of the four above categories.
Maybe if we don't want to commit to keeping the ObjectOptions schema,
the part that should wait is object-add and I should do the command line
options first? Then the schema remains an implementation detail for now
that is invisible in introspection.
I don't see much benefit in converting _any_ of the three actually. The
only good reason I see for QAPIfying this is the documentation, and the
promise of deduplicating QOM boilerplate. The latter is only a promise,
but documentation alone is a damn good reason and it's enough to get
this work into a mergeable shape as soon as possible!
But maybe, we could start in the opposite direction: start with the use
QAPI to eliminate QOM boilerplate. Basing your work on Eduardo's field
properties series, you could add a new 'object' "kind" to QAPI that
would create an array of field properties (e.g. a macro expanding to a
compound literal?)
. Something like
+{ 'object': 'InputBarrier',
+ 'data': { 'name': 'str',
+'x-origin': 'int16',
+'y-origin': 'int16',
+'width': 'int16',
+'height': 'int16' },
+ 'properties': { 'server': 'str',
+ 'port': 'str' } }
would create a macro QOM_InputBarrier_FIELDS defining properties for the
following fields of the InputBarrier struct:
gchar *name;
int16_t x_origin, y_origin;
int16_t width, height;
while server and port would only appear in the documentation (or
alternatively you could leave them out completely, as you wish).
The advantages would be noticeable:
1) the information would be moved in the QAPI schema JSON from the
beginning, decoupling the conflict-heavy part from the complex question
of how to expose the QOM schema in the introspection data
2) there would not be any more duplication than before (there would be
duplication between structs and QAPI schema, but not between structs and
C code that defines properties).
3) it would be opt-in, so it doesn't put on you the burden of keeping
the series in sync with new objects that are added (I have one for the
qtest server for example). At the same time it would be quite appealing
for owners of QOM code to convert their objects to field properties and
get documentation for free.
4) we could special-case 'object' definitions and generate them in the
system manual as well, since they are also useful to document -object.
Yes it's a huge change but you have the boring part already done. What
do
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 30/11/20 16:30, Daniel P. Berrangé wrote:
{ 'struct': 'QCryptoSecretCommon',
'base': 'Object',
'state': { 'rawdata': '*uint8_t',
'rawlen': 'size_t' },
'data': { '*format': 'QCryptoSecretFormat',
'*keyid': 'str',
'*iv': 'str' } }
{ 'struct': 'QCryptoSecret',
'base': 'QCryptoSecretCommon',
'data': { '*data': 'str',
'*file': 'str' } }
No, please don't do this. I want to keep writing C code, not a weird
JSON syntax for C.
I'd much rather have a FooOptions field in my struct so that I can just
do visit_FooOptions in the UserCreatable.complete() method, that's feasible.
Paolo
Re: [PATCH 01/18] qapi/qom: Add ObjectOptions for iothread
On 30/11/20 13:25, Kevin Wolf wrote:
+##
+# @IothreadProperties:
+#
+# Properties for iothread objects.
+#
+# @poll-max-ns: the maximum number of nanoseconds to busy wait for events.
+# 0 means polling is disabled (default: 32768 on POSIX hosts,
+# 0 otherwise)
+#
+# @poll-grow: the multiplier used to increase the polling time when the
+# algorithm detects it is missing events due to not polling long
+# enough. 0 selects a default behaviour (default: 0)
+#
+# @poll-shrink: the divisor used to decrease the polling time when the
+# algorithm detects it is spending too long polling without
+# encountering events. 0 selects a default behaviour (default: 0)
+#
+# Since: 6.0
+##
+{ 'struct': 'IothreadProperties',
+ 'data': { '*poll-max-ns': 'int',
+'*poll-grow': 'int',
+'*poll-shrink': 'int' } }
+
Documentation is the main advantage of the ObjectOptions concept.
However, please use the version where each object and property was
introduced for the "since" value. Otherwise the documentation will
appear to show that none of these objects was available before 6.0.
Yes, there is no documentation at all right now for QOM objects.
However, wrong documentation sometimes is worse than non-existing
documentation.
Paolo
Re: [PATCH 00/18] qapi/qom: QAPIfy object-add
On 30/11/20 13:25, Kevin Wolf wrote: This series adds a QAPI type for the properties of all user creatable QOM types and finally makes QMP object-add use the new ObjectOptions union so that QAPI introspection can be used for user creatable objects. After this series, there is least one obvious next step that needs to be done: Change HMP and all of the command line parser to use ObjectOptions, too, so that the QAPI schema is consistently enforced in all external interfaces. I am planning to send another series to address this. In a third step, we can try to start deduplicating and integrating things better between QAPI and the QOM implementation, e.g. by generating parts of the QOM boilerplate from the QAPI schema. With this series it's basically pointless to have QOM properties at all. Instead, you are basically having half of QEMU's backend data model into a single struct. So the question is, are we okay with shoveling half of QEMU's backend data model into a single struct? If so, there are important consequences. 1) QOM basically does not need properties anymore except for devices and machines (accelerators could be converted to QAPI as well). All user-creatable objects can be changed to something like chardev's "get a struct and use it fill in the fields", and only leave properties to devices and machines. 2) User-creatable objects can have a much more flexible schema. This means there's no reason to have block device creation as its own command and struct for example. The problem with this series is that you are fine with deduplicating things as a third step, but you cannot be sure that such deduplication is possible at all. So while I don't have any problems in principle with the ObjectOptions concept, I don't think it should be committed without a clear idea of how to do the third step. In the meanwhile, of course I have no problem with deprecating the opened and loaded properties. Paolo
[PATCH v2] gitlab-ci: publish test report as an artifact
Since version 0.55, "meson test" produces JUnit XML in the meson-logs directory. The XML can be parsed by GitLab and showed as part of the CI report. However, if the build and tests are performed by "meson dist", the tests are performed in "meson dist"'s own build directory and the logs are not accessible. So switch from "ninja dist" to "meson dist --no-tests" after a separate build step that is shared by the normal and the DIST=skip cases. Signed-off-by: Paolo Bonzini --- v1->v2: only do it for new-enough distros For an example see https://gitlab.com/bonzini/libvirt/-/pipelines/221545357/test_report. Test durations however are not yet available in upstream Meson. --- .gitlab-ci.yml | 38 ++ 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6792accf8f..c4b54201f8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -40,6 +40,36 @@ stages: <<: *container_job_definition allow_failure: true +# For new enough distros that have Meson 0.55, include the JUnit XML +# report in the artifacts. In order to preserve it, we cannot use +# "meson dist" and need to do the compilation test manually. +# Fortunately, "meson dist --no-tests" was also added in Meson 0.55. +.native_meson055_build_job_template: _meson055_build_job_definition + stage: builds + image: $CI_REGISTRY_IMAGE/ci-$NAME:latest + cache: +paths: + - ccache/ +key: "$CI_JOB_NAME" + before_script: +- *script_variables + script: +- meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1) +- ninja -C build +- ninja -C build test +- DESTDIR=$PWD/install/ ninja -C build install +- meson dist -C build --no-tests +- if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; + then +rpmbuild --nodeps -ta build/meson-dist/libvirt-*.tar.xz; + fi + artifacts: +when: always +paths: + - build/meson-logs/ +reports: + junit: build/meson-logs/testlog.junit.xml + .native_build_job_template: _build_job_definition stage: builds image: $CI_REGISTRY_IMAGE/ci-$NAME:latest @@ -292,7 +322,7 @@ x64-debian-10-clang: CC: clang x64-debian-sid: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-debian-sid-container variables: @@ -343,21 +373,21 @@ x64-fedora-31: RPM: skip x64-fedora-32: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-fedora-32-container variables: NAME: fedora-32 x64-fedora-rawhide: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-fedora-rawhide-container variables: NAME: fedora-rawhide x64-fedora-rawhide-clang: - <<: *native_build_job_definition + <<: *native_meson055_build_job_definition needs: - x64-fedora-rawhide-container variables: -- 2.28.0
[PATCH] gitlab-ci: publish test report as an artifact
"meson test" produces JUnit XML in the meson-logs directory. The XML can be parsed by GitLab and showed as part of the CI report. However, if the build and tests are performed by "meson dist", the tests are performed in "meson dist"'s own build directory and the logs are not accessible. So switch from "ninja dist" to "meson dist --no-tests" after a separate build step that is shared by the normal and the DIST=skip cases. Signed-off-by: Paolo Bonzini --- For an example see https://gitlab.com/bonzini/libvirt/-/pipelines/221545357/test_report. Test durations however are not yet available in upstream Meson. --- .gitlab-ci.yml | 14 ++ 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 6792accf8f..ce7b60dc6b 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -51,17 +51,23 @@ stages: - *script_variables script: - meson build --werror || (cat build/meson-logs/meson-log.txt && exit 1) +- ninja -C build +- ninja -C build test +- DESTDIR=$PWD/install/ ninja -C build install - if test "$DIST" != "skip"; then -ninja -C build dist; - else -ninja -C build; -ninja -C build test; +meson dist -C build --no-tests; fi - if test -x /usr/bin/rpmbuild && test "$RPM" != "skip"; then rpmbuild --nodeps -ta build/meson-dist/libvirt-*.tar.xz; fi + artifacts: +when: always +paths: + - build/meson-logs/ +reports: + junit: build/meson-logs/testlog.junit.xml # Jobs that we delegate to Cirrus CI because they require an operating # system other than Linux. These jobs will only run if the required -- 2.28.0
Re: [PATCH for-5.2] docs: Fix some typos (found by codespell)
On 17/11/20 20:34, Stefan Weil wrote: Fix also a similar typo in a code comment. Signed-off-by: Stefan Weil --- docs/can.txt | 8 docs/interop/vhost-user.rst | 2 +- docs/replay.txt | 2 +- docs/specs/ppc-spapr-numa.rst | 2 +- docs/system/deprecated.rst| 4 ++-- docs/tools/virtiofsd.rst | 2 +- hw/vfio/igd.c | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/can.txt b/docs/can.txt index 5838f6620c..0d310237df 100644 --- a/docs/can.txt +++ b/docs/can.txt @@ -19,7 +19,7 @@ interface to implement because such device can be easily connected to systems with different CPU architectures (x86, PowerPC, Arm, etc.). In 2020, CTU CAN FD controller model has been added as part -of the bachelor theses of Jan Charvat. This controller is complete +of the bachelor thesis of Jan Charvat. This controller is complete open-source/design/hardware solution. The core designer of the project is Ondrej Ille, the financial support has been provided by CTU, and more companies including Volkswagen subsidiaries. @@ -31,7 +31,7 @@ testing lead to goal change to provide environment which provides complete emulated environment for testing and RTEMS GSoC slot has been donated to work on CAN hardware emulation on QEMU. -Examples how to use CAN emulation for SJA1000 based borads +Examples how to use CAN emulation for SJA1000 based boards == When QEMU with CAN PCI support is compiled then one of the next @@ -106,8 +106,8 @@ This open-source core provides CAN FD support. CAN FD drames are delivered even to the host systems when SocketCAN interface is found CAN FD capable. -The PCIe borad emulation is provided for now (the device identifier is -ctucan_pci). The defauld build defines two CTU CAN FD cores +The PCIe board emulation is provided for now (the device identifier is +ctucan_pci). The default build defines two CTU CAN FD cores on the board. Example how to connect the canbus0-bus (virtual wire) to the host diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst index 988f154144..72b2e8c7ba 100644 --- a/docs/interop/vhost-user.rst +++ b/docs/interop/vhost-user.rst @@ -513,7 +513,7 @@ descriptor table (split virtqueue) or descriptor ring (packed virtqueue). However, it can't work when we process descriptors out-of-order because some entries which store the information of inflight descriptors in available ring (split virtqueue) or descriptor -ring (packed virtqueue) might be overrided by new entries. To solve +ring (packed virtqueue) might be overridden by new entries. To solve this problem, slave need to allocate an extra buffer to store this information of inflight descriptors and share it with master for persistent. ``VHOST_USER_GET_INFLIGHT_FD`` and diff --git a/docs/replay.txt b/docs/replay.txt index 87a64ae068..5b008ca491 100644 --- a/docs/replay.txt +++ b/docs/replay.txt @@ -328,7 +328,7 @@ between the snapshots. Each of the passes include the following steps: 1. loading the snapshot 2. replaying to examine the breakpoints 3. if breakpoint or watchpoint was met -- loading the snaphot again +- loading the snapshot again - replaying to the required breakpoint 4. else - proceeding to the p.1 with the earlier snapshot diff --git a/docs/specs/ppc-spapr-numa.rst b/docs/specs/ppc-spapr-numa.rst index 5fca2bdd8e..ffa687dc89 100644 --- a/docs/specs/ppc-spapr-numa.rst +++ b/docs/specs/ppc-spapr-numa.rst @@ -198,7 +198,7 @@ This is how it is being done: * user distance 121 and beyond will be interpreted as 160 * user distance 10 stays 10 -The reasoning behind this aproximation is to avoid any round up to the local +The reasoning behind this approximation is to avoid any round up to the local distance (10), keeping it exclusive to the 4th NUMA level (which is still exclusive to the node_id). All other ranges were chosen under the developer discretion of what would be (somewhat) sensible considering the user input. diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst index 32a0e620db..63e9db1463 100644 --- a/docs/system/deprecated.rst +++ b/docs/system/deprecated.rst @@ -465,7 +465,7 @@ default configuration. The CPU model runnability guarantee won't apply anymore to existing CPU models. Management software that needs runnability -guarantees must resolve the CPU model aliases using te +guarantees must resolve the CPU model aliases using the ``alias-of`` field returned by the ``query-cpu-definitions`` QMP command. @@ -637,7 +637,7 @@ Splitting RAM by default between NUMA nodes had the same issues as ``mem`` parameter with the difference that the role of the user plays QEMU using implicit generic or board specific splitting rule. Use ``memdev`` with *memory-backend-ram* backend or ``mem`` (if -it's supported by used machine
Re: [PATCH 0/2] char: Deprecate backend aliases
On 11/11/20 14:08, Kevin Wolf wrote: These aliases only work the command line, but not in QMP. Command line QAPIfication involves writing some compatibility glue for them, which I'm doing, but I think it's desirable to unify accepted values of both paths. So deprecate the aliases so that we can drop the compatibility glue later. In the deprecation documentation I assumed that this is for 6.0, but if we want to include it in 5.2 still, this can be changed, of course. Kevin Wolf (2): char: Skip CLI aliases in query-chardev-backends char: Deprecate backend aliases 'tty' and 'parport' docs/system/deprecated.rst | 6 ++ chardev/char.c | 32 2 files changed, 30 insertions(+), 8 deletions(-) Even though I disagree with QAPIfying -chardev, this one is obviously a good thing. Acked-by: Paolo Bonzini Paolo
Re: [PATCH] os: deprecate the -enable-fips option and QEMU's FIPS enforcement
On 21/10/20 12:17, Daniel P. Berrangé wrote: >> But would it be correct? In order to have the advertised behavior of >> "enable FIPS compliance just with procfs, no need to do anything in >> QEMU" you need to disable VNC password authentication; so while >> fips_set_state is an abomination, fips_get_state should remain. > There's no need for fips_get_state. Once you build QEMU with > libgcrypt, when VNC requests a DES cipher handle, gcrypt will > return an error as that algorithm is forbidden in FIPS mode. Oh, I thought we were still using our own code for the modified DES but it _is_ actually using gcrypt or nettle if available. Sorry for the noise. > This is the primary reason for outsourcing all crypto to a > separate library and ignoring the impls in QEMU. > > Claiming QEMU is FIPS compliant without using libgcrypt is a > bit of joke since we don't do any self-tests of ciphers, hence > this deprecation notice is warning people that libgcrypt is > going to be mandatory if you care about FIPS. Yes, agreed. Paolo
Re: [PATCH] os: deprecate the -enable-fips option and QEMU's FIPS enforcement
On 21/10/20 10:38, Daniel P. Berrangé wrote:
> On Tue, Oct 20, 2020 at 07:22:03PM +0200, Paolo Bonzini wrote:
>> On 20/10/20 18:22, Daniel P. Berrangé wrote:
>>> @@ -153,6 +153,9 @@ int os_parse_cmd_args(int index, const char *optarg)
>>> break;
>>> #if defined(CONFIG_LINUX)
>>> case QEMU_OPTION_enablefips:
>>> +warn_report("-enable-fips is deprecated, please build QEMU with "
>>> +"the `libgcrypt` library as the cryptography provider "
>>> +"to enable FIPS compliance");
>>> fips_set_state(true);
>>> break;
>>> #endif
>>
>> Should you also remove fips_set_state(true) and make fips_get_state()
>> return the contents of /proc/sys/crypto/fips_enabled, so that VNC
>> password authentication is disabled?
>
> I did think about doing that, but decided that since my intention is
> to delete all trace of fips_get_state / fips_set_state at the end of
> the deprecation period, that it'd be saner just to leave the semantics
> unchanged during the deprecation period.
But would it be correct? In order to have the advertised behavior of
"enable FIPS compliance just with procfs, no need to do anything in
QEMU" you need to disable VNC password authentication; so while
fips_set_state is an abomination, fips_get_state should remain.
> Deprecation notices shouldn't really be associated with changes in
> functionality at time they are introduced.
I think you can consider it a bugfix since no one sets fips_enabled
without knowing what they're doing.
Paolo
Re: [PATCH] os: deprecate the -enable-fips option and QEMU's FIPS enforcement
On 20/10/20 18:22, Daniel P. Berrangé wrote:
> @@ -153,6 +153,9 @@ int os_parse_cmd_args(int index, const char *optarg)
> break;
> #if defined(CONFIG_LINUX)
> case QEMU_OPTION_enablefips:
> +warn_report("-enable-fips is deprecated, please build QEMU with "
> +"the `libgcrypt` library as the cryptography provider "
> +"to enable FIPS compliance");
> fips_set_state(true);
> break;
> #endif
Should you also remove fips_set_state(true) and make fips_get_state()
return the contents of /proc/sys/crypto/fips_enabled, so that VNC
password authentication is disabled?
Paolo
