Patch introduces acpi_table_begin()/ acpi_table_end() API
that hides pointer/offset arithmetic from user as opposed
to build_header(), to prevent errors caused by it [1].
acpi_table_begin():
initializes table header and keeps track of
table data/offsets
acpi_table_end():
sets actual table length and tells bios loader
where table is for the later initialization on
guest side.
1) commits
bb9feea431 x86: acpi: use offset instead of pointer when using build_header()
4d027afeb3 Virt: ACPI: fix qemu assert due to re-assigned table data address
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Message-Id: <20210924122802.1455362-2-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Tested-by: Yanan Wang <wangyanan55@huawei.com>
virtio-vsock features, like VIRTIO_VSOCK_F_SEQPACKET, can be handled
by vhost-vsock-common parent class. In this way, we can reuse the
same code for all virtio-vsock backends (i.e. vhost-vsock,
vhost-user-vsock).
Let's move `seqpacket` property to vhost-vsock-common class, add
vhost_vsock_common_get_features() used by children, and disable
`seqpacket` for vhost-user-vsock device for machine types < 6.2.
The behavior of vhost-vsock device doesn't change; vhost-user-vsock
device now supports `seqpacket` property.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210921161642.206461-3-sgarzare@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Commit 1e08fd0a46 ("vhost-vsock: SOCK_SEQPACKET feature bit support")
enabled the SEQPACKET feature bit.
This commit is released with QEMU 6.1, so if we try to migrate a VM where
the host kernel supports SEQPACKET but machine type version is less than
6.1, we get the following errors:
Features 0x130000002 unsupported. Allowed features: 0x179000000
Failed to load virtio-vhost_vsock:virtio
error while loading state for instance 0x0 of device '0000:00:05.0/virtio-vhost_vsock'
load of migration failed: Operation not permitted
Let's disable the feature bit for machine types < 6.1.
We add a new OnOffAuto property for this, called `seqpacket`.
When it is `auto` (default), QEMU behaves as before, trying to enable the
feature, when it is `on` QEMU will fail if the backend (vhost-vsock
kernel module) doesn't support it.
Fixes: 1e08fd0a46 ("vhost-vsock: SOCK_SEQPACKET feature bit support")
Cc: qemu-stable@nongnu.org
Reported-by: Jiang Wang <jiang.wang@bytedance.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210921161642.206461-2-sgarzare@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
* fix search path when configuring with --cpu
* support for measured SEV boot with -kernel (Dov)
* fix missing BQL locks (Emanuele)
* retrieve applesmc key from the host (Pedro)
* KVM PV feature documentation (Vitaly)
-----BEGIN PGP SIGNATURE-----
iQFIBAABCAAyFiEE8TM4V0tmI4mGbHaCv/vSX3jHroMFAmFcMzwUHHBib256aW5p
QHJlZGhhdC5jb20ACgkQv/vSX3jHroOkigf/QbWFnBvd1haj3BWB9vrf5viEq2Fp
j5wIXZz/FsCFYS4u+MVzI3L0MBb8ROkgsHg1yO1Ol4Kx3m5nZ7kJSbCxI6GG+Ry3
1dNrN/0ncbjXJfFGZAADSeA+ajOnahvKII5LZE6XYk2SxFfjKmfj3iePDIb/Wil+
AXa1UfKu8ETC0hLDlq+HVspQOskmpzCVw0v5Xf5hyBp9vrSIyMl1sR63qe5TKWnd
L8cX8TRxi5Z5dHb+ZvB2Fd3vpMc8YrOkQOrl0RBnRk95vT1psDHArJhC5tIFEvIO
H9PNfCX4mAPMeOawzhGAMmF/tsxuFPKGrVvU67xI2jzpbnlYnee7IwvBGg==
=vlRk
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging
* Meson version update
* fix search path when configuring with --cpu
* support for measured SEV boot with -kernel (Dov)
* fix missing BQL locks (Emanuele)
* retrieve applesmc key from the host (Pedro)
* KVM PV feature documentation (Vitaly)
# gpg: Signature made Tue 05 Oct 2021 04:13:00 AM PDT
# gpg: using RSA key F13338574B662389866C7682BFFBD25F78C7AE83
# gpg: issuer "pbonzini@redhat.com"
# gpg: Good signature from "Paolo Bonzini <bonzini@gnu.org>" [full]
# gpg: aka "Paolo Bonzini <pbonzini@redhat.com>" [full]
* remotes/bonzini/tags/for-upstream:
meson: show library versions in the summary
target/xtensa: list cores in a text file
hexagon: use env keyword argument to pass PYTHONPATH
meson: switch minimum meson version to 0.58.2, minimum recommended to 0.59.2
meson: bump submodule to 0.59.2
migration: add missing qemu_mutex_lock_iothread in migration_completion
migration: block-dirty-bitmap: add missing qemu_mutex_lock_iothread
configure, meson: move CPU_CFLAGS out of QEMU_CFLAGS
hw/misc: applesmc: use host osk as default on macs
x86/sev: generate SEV kernel loader hashes in x86_load_linux
sev/i386: Introduce sev_add_kernel_loader_hashes for measured linux boot
i386: docs: Briefly describe KVM PV features
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
All code in block/aio_task.c expects `max_busy_tasks` to always
be greater than 0.
Assert this condition during the AioTaskPool creation where
`max_busy_tasks` is set.
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211005161157.282396-3-sgarzare@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
QAPI generates `struct BackupPerf` where `max-workers` value is stored
in an `int64_t` variable.
But block_copy_async(), and the underlying code, uses an `int` parameter.
At the end that variable is used to initialize `max_busy_tasks` in
block/aio_task.c causing the following assertion failure if a value
greater than INT_MAX(2147483647) is used:
../block/aio_task.c:63: aio_task_pool_wait_one: Assertion `pool->busy_tasks > 0' failed.
Let's check that `max-workers` doesn't exceed INT_MAX and print an
error in that case.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2009310
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20211005161157.282396-2-sgarzare@redhat.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Both virtqueue_packed_get_avail_bytes() and
virtqueue_split_get_avail_bytes() access the region cache, but
their caller also does. Simplify by having virtqueue_get_avail_bytes
calling both with RCU lock held, and passing the caches as argument.
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210906104318.1569967-4-philmd@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
vring_get_region_caches() must be called with the RCU read lock
acquired. virtqueue_packed_drop_all() does not, and uses the
'caches' pointer. Fix that by using the RCU_READ_LOCK_GUARD()
macro.
Reported-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20210906104318.1569967-3-philmd@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Meson 0.57 allows passing external programs and dependency objects
to summary(). Use this to show library versions and paths in the
summary.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Avoid that leftover files affect the build; instead, use the same
mechanism that was in place before the Meson transition of updating
a file from import_core.sh. Starting with Meson 0.57, the file
can be easily read from the filesystem module, so do that instead
of using run_command.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Meson 0.58.2 does not need b_staticpic=$pie anymore, and has
stabilized the keyval module. Remove the workaround and use a few
replacements for features deprecated in the 0.57.0 release cycle.
One feature that we would like to use is passing dependencies to
summary. However, that was broken in 0.59.0 and 0.59.1. Therefore,
use the embedded Meson if the host has anything older than 0.59.2,
but allow --meson= to use 0.58.2.
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The update to 0.57 has been delayed due to it causing warnings for
some actual issues, but it brings in important bugfixes and new
features. 0.58 also brings in a bugfix that is useful for modinfo.
Important bugfixes:
- 0.57: https://github.com/mesonbuild/meson/pull/7760, build: use PIE
objects for non-PIC static libraries if b_pie=true
- 0.57: https://github.com/mesonbuild/meson/pull/7900, thus avoiding
unnecessary rebuilds after running meson.
- 0.58.2: https://github.com/mesonbuild/meson/pull/8900, fixes for
passing extract_objects() to custom_target (useful for modinfo)
Features:
- 0.57: the keyval module has now been stabilized
- 0.57: env argument to custom_target (useful for hexagon)
- 0.57: Feature parity between "meson test" and QEMU's TAP driver
- 0.57: https://github.com/mesonbuild/meson/pull/8231, allows bringing
back version numbers in the configuration summary
- 0.59: Utility methods for feature objects
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
qemu_savevm_state_complete_postcopy assumes the iothread lock (BQL)
to be held, but instead it isn't.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20211005080751.3797161-3-eesposit@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
init_dirty_bitmap_migration assumes the iothread lock (BQL)
to be held, but instead it isn't.
Instead of adding the lock to qemu_savevm_state_setup(),
follow the same pattern as the other ->save_setup callbacks
and lock+unlock inside dirty_bitmap_save_setup().
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20211005080751.3797161-2-eesposit@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Flags that choose the target architecture, such as -m32 on x86, affect
all invocations of the compiler driver, for example including options
such as --print-search-dirs. To ensure that they are treated as such,
place them in the cross file in the [binaries] section instead of
including them in QEMU_CFLAGS.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
If SEV is enabled and a kernel is passed via -kernel, pass the hashes of
kernel/initrd/cmdline in an encrypted guest page to OVMF for SEV
measured boot.
Co-developed-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210930054915.13252-3-dovmurik@linux.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Add the sev_add_kernel_loader_hashes function to calculate the hashes of
the kernel/initrd/cmdline and fill a designated OVMF encrypted hash
table area. For this to work, OVMF must support an encrypted area to
place the data which is advertised via a special GUID in the OVMF reset
table.
The hashes of each of the files is calculated (or the string in the case
of the cmdline with trailing '\0' included). Each entry in the hashes
table is GUID identified and since they're passed through the
sev_encrypt_flash interface, the hashes will be accumulated by the AMD
PSP measurement (SEV_LAUNCH_MEASURE).
Co-developed-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: James Bottomley <jejb@linux.ibm.com>
Signed-off-by: Dov Murik <dovmurik@linux.ibm.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20210930054915.13252-2-dovmurik@linux.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
KVM PV features don't seem to be documented anywhere, in particular, the
fact that some of the features are enabled by default and some are not can
only be figured out from the code.
Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Message-Id: <20211004140445.624875-1-vkuznets@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The correct thing to do has been present but commented
out since the initial commit of the sh4 translator.
Fixes: fdf9b3e831
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210929130316.121330-1-richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
There is one numa config example in qemu-options.hx currently
using "-smp 2" and assuming that there will be 2 sockets and
2 cpus totally. However now the actual calculation logic of
missing sockets and cores is not immutable and is considered
liable to change. Although we will get maxcpus=2 finally based
on current parser, it's always stable to specify it explicitly.
So "-smp 2,sockets=2,maxcpus=2" will be optimal when we expect
multiple sockets and 2 cpus totally.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-Id: <20210928121134.21064-3-wangyanan55@huawei.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
In qemu-option.hx, there is "-smp [[cpus=]n][,maxcpus=cpus]..." in the
DEF part, and "-smp [[cpus=]n][,maxcpus=maxcpus]..." in the RST part.
Obviously the later is right, let's fix the previous one.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
Reviewed-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Message-Id: <20210928121134.21064-2-wangyanan55@huawei.com>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
virtio-mem logically plugs/unplugs memory within a sparse memory region
and notifies via the RamDiscardManager interface when parts become
plugged (populated) or unplugged (discarded).
Currently, we end up (via the two users)
1) zeroing all logically unplugged/discarded memory during TPM resets.
2) reading all logically unplugged/discarded memory when dumping, to
figure out the content is zero.
1) is always bad, because we assume unplugged memory stays discarded
(and is already implicitly zero).
2) isn't that bad with anonymous memory, we end up reading the zero
page (slow and unnecessary, though). However, once we use some
file-backed memory (future use case), even reading will populate memory.
Let's cut out all parts marked as not-populated (discarded) via the
RamDiscardManager. As virtio-mem is the single user, this now means that
logically unplugged memory ranges will no longer be included in the
dump, which results in smaller dump files and faster dumping.
virtio-mem has a minimum granularity of 1 MiB (and the default is usually
2 MiB). Theoretically, we can see quite some fragmentation, in practice
we won't have it completely fragmented in 1 MiB pieces. Still, we might
end up with many physical ranges.
Both, the ELF format and kdump seem to be ready to support many
individual ranges (e.g., for ELF it seems to be UINT32_MAX, kdump has a
linear bitmap).
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210727082545.17934-5-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Let's factor out adding a MemoryRegionSection to the list, to be reused in
RamDiscardManager context next.
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210727082545.17934-4-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Let's make sure to not merge when different memory regions are involved.
Unlikely, but theoretically possible.
Acked-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210727082545.17934-3-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We might not start at the beginning of the memory region. Let's
calculate the offset into the memory region via the difference in the
host addresses.
Acked-by: Stefan Berger <stefanb@linux.ibm.com>
Fixes: ffab1be706 ("tpm: clear RAM when "memory overwrite" requested")
Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Claudio Fontana <cfontana@suse.de>
Cc: Thomas Huth <thuth@redhat.com>
Cc: "Alex Bennée" <alex.bennee@linaro.org>
Cc: Peter Xu <peterx@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Message-Id: <20210727082545.17934-2-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We want to rate-limit MEMORY_DEVICE_SIZE_CHANGE events per device,
otherwise we can lose some events for devices. We can now use the
qom-path to reliably map an event to a device and make rate-limiting
device-aware.
This was noticed by starting a VM with two virtio-mem devices that each
have a requested size > 0. The Linux guest will initialize both devices
in parallel, resulting in losing MEMORY_DEVICE_SIZE_CHANGE events for
one of the devices.
Fixes: 722a3c783e ("virtio-pci: Send qapi events when the virtio-mem size changes")
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210929162445.64060-4-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
As we might not always have a device id, it is impossible to always
match MEMORY_DEVICE_SIZE_CHANGE events to an actual device. Let's
include the qom-path in the event, which allows for reliable mapping of
events to devices.
Fixes: 722a3c783e ("virtio-pci: Send qapi events when the virtio-mem size changes")
Suggested-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210929162445.64060-3-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Apparently, we don't have to duplicate the string.
Fixes: 722a3c783e ("virtio-pci: Send qapi events when the virtio-mem size changes")
Cc: qemu-stable@nongnu.org
Signed-off-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20210929162445.64060-2-david@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
As discussed in issue 614, we're shipping GCC 7.4.0 as the
system compiler in NetBSD 9, the most recent stable branch,
and are still actively interested in QEMU on this platform.
The differences between GCC 7.5.0 and 7.4.0 are trivial.
Signed-off-by: Nia Alarie <nia@NetBSD.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <YVcpe79I0rly1HJh@homeworld.netbsd.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-14-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Eh. Not worth the fuss today. There are bigger fish to fry.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-13-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-12-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The fix for this comment is forthcoming in a future commit, but this
will keep me honest. The linting configuration in ./python/setup.cfg
prohibits 'FIXME' comments. A goal of this long-running series is to
move ./scripts/qapi to ./python/qemu/qapi so that the QAPI generator is
regularly type-checked by GitLab CI.
This comment is a time-bomb to force me to address this issue prior to
that step.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-11-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Annotations do not change runtime behavior.
This commit consists of only annotations.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-10-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Adding static types causes a cycle in the QAPI generator:
[schema -> expr -> parser -> schema]. It exists because the QAPIDoc
class needs the names of types defined by the schema module, but the
schema module needs to import both expr.py/parser.py to do its actual
parsing.
Ultimately, the layering violation is that parser.py should not have any
knowledge of specifics of the Schema. QAPIDoc performs double-duty here
both as a parser *and* as a finalized object that is part of the schema.
In this patch, add the offending type hints alongside the workaround to
avoid the cycle becoming a problem at runtime. See
https://mypy.readthedocs.io/en/latest/runtime_troubles.html#import-cycles
for more information on this workaround technique.
I see three ultimate resolutions here:
(1) Just keep this patch and use the TYPE_CHECKING trick to eliminate
the cycle which is only present during static analysis.
(2) Don't bother to annotate connect_member() et al, give them 'object'
or 'Any'. I don't particularly like this, because it diminishes the
usefulness of type hints for documentation purposes. Still, it's an
extremely quick fix.
(3) Reimplement doc <--> definition correlation directly in schema.py,
integrating doc fields directly into QAPISchemaMember and relieving
the QAPIDoc class of the responsibility. Users of the information
would instead visit the members first and retrieve their
documentation instead of the inverse operation -- visiting the
documentation and retrieving their members.
My preference is (3), but in the short-term (1) is the easiest way to
have my cake (strong type hints) and eat it too (Not have import
cycles). Do (1) for now, but plan for (3).
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-9-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Here's the weird bit. QAPIDoc generally expects -- virtually everywhere
-- that it will always have a current section. The sole exception to
this is in the case that end_comment() is called, which leaves us with
*no* section. However, in this case, we also don't expect to actually
ever mutate the comment contents ever again.
NullSection is just a Null-object that allows us to maintain the
invariant that we *always* have a current section, enforced by static
typing -- allowing us to type that field as QAPIDoc.Section instead of
the more ambiguous Optional[QAPIDoc.Section].
end_section is renamed to switch_section and now accepts as an argument
the new section to activate, clarifying that no callers ever just
unilaterally end a section; they only do so when starting a new section.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-8-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
The "if self._section" clause in end_section is mysterious: In which
circumstances might we end a section when we don't have one?
QAPIDoc always expects there to be a "current section", only except
after a call to end_comment(). This actually *shouldn't* ever be 'None',
so let's remove that logic so I don't wonder why it's like this again in
three months.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-7-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
True, we do not check the validity of this symbol -- but we don't check
the validity of definition names during parse, either -- that happens
later, during the expr check. I don't want to introduce a dependency on
expr.py:check_name_str here and introduce a cycle.
Instead, rest assured that a documentation block is required for each
definition. This requirement uses the names of each section to ensure
that we fulfilled this requirement.
e.g., let's say that block-core.json has a comment block for
"Snapshot!Info" by accident. We'll see this error message:
In file included from ../../qapi/block.json:8:
../../qapi/block-core.json: In struct 'SnapshotInfo':
../../qapi/block-core.json:38: documentation comment is for 'Snapshot!Info'
That's a pretty decent error message.
Now, let's say that we actually mangle it twice, identically:
../../qapi/block-core.json: In struct 'Snapshot!Info':
../../qapi/block-core.json:38: struct has an invalid name
That's also pretty decent. If we forget to fix it in both places, we'll
just be back to the first error.
Therefore, let's just drop this FIXME and adjust the error message to
not imply a more thorough check than is actually performed.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-6-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Several QGA definitions omit a blank line after the symbol
declaration. This works OK currently, but it's the only place where we
do this. Adjust it for consistency.
Future commits may wind up enforcing this formatting.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-5-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Pylint informs us we're not using these arguments. Oops, it's
right. Correct the error message and remove the remaining unused
parameter.
Fix test output now that the error message is improved.
Fixes: e151941d1b
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-4-jsnow@redhat.com>
[Commit message formatting tweaked]
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
New pylint warning. I could silence it, but this is the only occurrence
in the entire tree, including everything in iotests/ and python/. Easier
to just change this one instance.
(The warning is emitted in cases where you are fetching the values
anyway, so you may as well just take advantage of the iterator to avoid
redundant lookups.)
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-3-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Pylint 2.11.x adds this warning. We're not yet ready to pursue that
conversion, so silence it for now.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-Id: <20210930205716.1148693-2-jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
