Dou Liyang [Tue, 14 Nov 2017 02:34:01 +0000 (10:34 +0800)]
NUMA: Enable adding NUMA node implicitly
Linux and Windows need ACPI SRAT table to make memory hotplug work properly,
however currently QEMU doesn't create SRAT table if numa options aren't present
on CLI.
Which breaks both linux and windows guests in certain conditions:
* Windows: won't enable memory hotplug without SRAT table at all
* Linux: if QEMU is started with initial memory all below 4Gb and no SRAT table
present, guest kernel will use nommu DMA ops, which breaks 32bit hw drivers
when memory is hotplugged and guest tries to use it with that drivers.
Fix above issues by automatically creating a numa node when QEMU is started with
memory hotplug enabled but without '-numa' options on CLI.
(PS: auto-create numa node only for new machine types so not to break migration).
Which would provide SRAT table to guests without explicit -numa options on CLI
and would allow:
* Windows: to enable memory hotplug
* Linux: switch to SWIOTLB DMA ops, to bounce DMA transfers to 32bit allocated
buffers that legacy drivers/hw can handle.
[Rewritten by Igor]
Reported-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com> Suggested-by: Igor Mammedov <imammedo@redhat.com> Signed-off-by: Dou Liyang <douly.fnst@cn.fujitsu.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Eduardo Habkost <ehabkost@redhat.com> Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Marcel Apfelbaum <marcel@redhat.com> Cc: Igor Mammedov <imammedo@redhat.com> Cc: David Hildenbrand <david@redhat.com> Cc: Thomas Huth <thuth@redhat.com> Cc: Alistair Francis <alistair23@gmail.com> Cc: Takao Indoh <indou.takao@jp.fujitsu.com> Cc: Izumi Taku <izumi.taku@jp.fujitsu.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
The PCIE-PCI bridge is specific to "pure" PCIe systems
(on QEMU we have X86 and ARM), it does not make sense to
have it in other archs.
Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Tested-by: Yongbok Kim <yongbok.kim@mips.com>
Marcel Apfelbaum [Sat, 11 Nov 2017 15:25:00 +0000 (17:25 +0200)]
hw/pci-host: Fix x86 Host Bridges 64bit PCI hole
Currently there is no MMIO range over 4G
reserved for PCI hotplug. Since the 32bit PCI hole
depends on the number of cold-plugged PCI devices
and other factors, it is very possible is too small
to hotplug PCI devices with large BARs.
Fix it by reserving 2G for I4400FX chipset
in order to comply with older Win32 Guest OSes
and 32G for Q35 chipset.
Even if the new defaults of pci-hole64-size will appear in
"info qtree" also for older machines, the property was
not implemented so no changes will be visible to guests.
Note this is a regression since prev QEMU versions had
some range reserved for 64bit PCI hotplug.
Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
This moves pci_dev->name initialization earlier so
pci_dev->bus_master_as could get a name instead of an empty string.
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Xu <peterx@redhat.com> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
linzhecheng [Tue, 31 Oct 2017 08:03:03 +0000 (16:03 +0800)]
fix: unrealize virtio device if we fail to hotplug it
If we fail to hotplug virtio-blk device and then suspend
or shutdown VM, qemu is likely to crash.
Re-production steps:
1. Run VM named vm001
2. Create a virtio-blk.xml which contains wrong configurations:
<disk device="lun" rawio="yes" type="block">
<driver cache="none" io="native" name="qemu" type="raw" />
<source dev="/dev/mapper/11-dm" />
<target bus="virtio" dev="vdx" />
</disk>
3. Run command : virsh attach-device vm001 virtio-blk.xml
error: Failed to attach device from blk-scsi.xml
error: internal error: unable to execute QEMU command 'device_add': Please set scsi=off for virtio-blk devices in order to use virtio 1.0
it means hotplug virtio-blk device failed.
4. Suspend or shutdown VM will leads to qemu crash
Problem happens in virtio_vmstate_change which is called by
vm_state_notify:
vdev’s parent_bus is NULL, so qdev_get_parent_bus(DEVICE(vdev)) will crash.
virtio_vmstate_change is added to the list vm_change_state_head at virtio_blk_device_realize(virtio_init),
but after hotplug virtio-blk failed, virtio_vmstate_change will not be removed from vm_change_state_head.
Adding unrealize function of virtio-blk device can solve this problem.
Signed-off-by: linzhecheng <linzhecheng@huawei.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>
Peter Maydell [Thu, 16 Nov 2017 12:45:14 +0000 (12:45 +0000)]
Merge remote-tracking branch 'remotes/rth/tags/pull-tcg-20171115' into staging
User-mode memory helper fixes
# gpg: Signature made Wed 15 Nov 2017 12:32:33 GMT
# gpg: using RSA key 0x64DF38E8AF7E215F
# gpg: Good signature from "Richard Henderson <richard.henderson@linaro.org>"
# Primary key fingerprint: 7A48 1E78 868B 4DB6 A85A 05C0 64DF 38E8 AF7E 215F
* remotes/rth/tags/pull-tcg-20171115:
target/arm: Fix GETPC usage in do_paired_cmpxchg64_l/be
target/arm: Use helper_retaddr in stxp helpers
tcg: Record code_gen_buffer address for user-only memory helpers
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Thu, 16 Nov 2017 11:34:24 +0000 (11:34 +0000)]
Merge remote-tracking branch 'remotes/stefanberger/tags/pull-tpm-2017-11-15-1' into staging
Merge tpm 2017/11/15 v1
# gpg: Signature made Wed 15 Nov 2017 11:51:47 GMT
# gpg: using RSA key 0x75AD65802A0B4211
# gpg: Good signature from "Stefan Berger <stefanb@linux.vnet.ibm.com>"
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg: There is no indication that the signature belongs to the owner.
# Primary key fingerprint: B818 B9CA DF90 89C2 D5CE C66B 75AD 6580 2A0B 4211
* remotes/stefanberger/tags/pull-tpm-2017-11-15-1:
tpm_tis: Return 0 for every register in case of failure mode
tpm_tis: Return TPM_VERSION_UNSPEC in case of BE failure
tpm-emulator: protect concurrent ctrl_chr access
specs: Extend TPM spec with TPM emulator description
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Stefan Berger [Sat, 11 Nov 2017 03:33:14 +0000 (22:33 -0500)]
tpm_tis: Return 0 for every register in case of failure mode
Rather than returning ~0, return 0 for every register in case of failure
mode. The '0' is better to indicate that there's no device there. It avoids
SeaBIOS detecting a device and getting stuck on it trying to read and write
its registers.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Stefan Berger [Sat, 11 Nov 2017 03:07:35 +0000 (22:07 -0500)]
tpm_tis: Return TPM_VERSION_UNSPEC in case of BE failure
In case the backend has a failure, such as the tpm_emulator's CMD_INIT
failing, the TIS goes into failure mode and does not respond to reads
or writes to MMIO registers. In this case we need to prevent the ACPI
table from being added and the straight-forward way is to indicate that
there's no known TPM version being used.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
The control chardev is being used from the data thread to set the
locality of the next request. Altough the chr has a write mutex, we
may potentially read the reply from another thread request.
Add a mutex to protect from concurrent control commands.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
target/arm: Fix GETPC usage in do_paired_cmpxchg64_l/be
Use of GETPC must be restricted to those functions that are
directly called from TCG generated code.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Fixes: 2399d4e7cec22ecf1c51062d2ebfd45220dbaace Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
We use raw memory primitives along the !parallel_cpus paths in order to
simplify the endianness handling. Because of that, we did not benefit
from the generic changes to cpu_ldst_user_only_template.h.
The simplest fix is to manipulate helper_retaddr here.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
tcg: Record code_gen_buffer address for user-only memory helpers
When we handle a signal from a fault within a user-only memory helper,
we cannot cpu_restore_state with the PC found within the signal frame.
Use a TLS variable, helper_retaddr, to record the unwind start point
to find the faulting guest insn.
Tested-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Peter Maydell [Tue, 14 Nov 2017 17:35:41 +0000 (17:35 +0000)]
Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2017-11-14' into staging
Block patches for 2.11.0-rc1
# gpg: Signature made Tue 14 Nov 2017 17:22:17 GMT
# gpg: using RSA key 0xF407DB0061D5CF40
# gpg: Good signature from "Max Reitz <mreitz@redhat.com>"
# Primary key fingerprint: 91BE B60A 30DB 3E88 57D1 1829 F407 DB00 61D5 CF40
* remotes/maxreitz/tags/pull-block-2017-11-14:
qemu-iotests: update unsupported image formats in 194
block/parallels: add migration blocker
block/parallels: Do not update header or truncate image when INMIGRATE
block/vhdx.c: Don't blindly update the header
iotests: 077: Filter out 'resume' lines
block/snapshot: dirty all dirty bitmaps on snapshot-switch
qcow2: Check that corrupted images can be repaired in iotest 060
iotests: Use new-style NBD connections
iotests: Make 136 less flaky
iotests: Make 083 less flaky
iotests: Make 055 less flaky
iotests: Add missing 'blkdebug::' in 040
iotests: Make 030 less flaky
qcow2: Assert that the crypto header does not overlap other metadata
qcow2: Add iotest for an empty refcount table
qcow2: Add iotest for an image with header.refcount_table_offset == 0
qcow2: Don't open images with header.refcount_table_clusters == 0
qcow2: Prevent allocating compressed clusters at offset 0
qcow2: Prevent allocating L2 tables at offset 0
qcow2: Prevent allocating refcount blocks at offset 0
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Jeff Cody [Tue, 7 Nov 2017 13:10:35 +0000 (08:10 -0500)]
block/parallels: add migration blocker
Migration does not work for parallels, and has been broken for a while
(see patch 'block/parallels: Do not update header or truncate image when
INMIGRATE'). The bdrv_invalidate_cache() method needs to be added for
migration to be supported. Until this is done, prohibit migration.
Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 5e04a7c8a3089913fa58d484af42dab7993984ad.1510059970.git.jcody@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Max Reitz <mreitz@redhat.com>
Jeff Cody [Tue, 7 Nov 2017 13:10:34 +0000 (08:10 -0500)]
block/parallels: Do not update header or truncate image when INMIGRATE
If we write or modify the image file while the QEMU run state is
INMIGRATE, then the BDRV_O_INACTIVE BDS flag is set. This will cause
an assert, since the image is marked inactive. Make sure we obey this
flag.
Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru> Signed-off-by: Jeff Cody <jcody@redhat.com>
Message-id: 3996c930fa8cde8570b7a63032720d76a28fd78b.1510059970.git.jcody@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Max Reitz <mreitz@redhat.com>
Jeff Cody [Tue, 7 Nov 2017 13:10:33 +0000 (08:10 -0500)]
block/vhdx.c: Don't blindly update the header
The VHDX specification requires that before user data modification of
the vhdx image, the VHDX header file and data GUIDs need to be updated.
In vhdx_open(), if the image is set to RDWR, we go ahead and update the
header.
However, just because the image is set to RDWR does not mean we can go
ahead and write at this point - specifically, if the QEMU run state is
INMIGRATE, the underlying file BS may be set to inactive via the BDS
open flag of BDRV_O_INACTIVE. Attempting to write under this condition
will cause an assert in bdrv_co_pwritev().
We can alternatively latch the first time the image is written. And lo
and behold, we do just that, via vhdx_user_visible_write() in
vhdx_co_writev(). This means the call to vhdx_update_headers() in
vhdx_open() is likely just vestigial, and can be removed.
Reported-by: Alexey Kardashevskiy <aik@ozlabs.ru> Tested-by: Alexey Kardashevskiy <aik@ozlabs.ru> Signed-off-by: Jeff Cody <jcody@redhat.com>
Message-id: 659e4cdba6ef4c651737852777c8c93d27b38040.1510059970.git.jcody@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Denis V. Lunev <den@openvz.org> Signed-off-by: Max Reitz <mreitz@redhat.com>
Fam Zheng [Mon, 13 Nov 2017 15:00:26 +0000 (23:00 +0800)]
iotests: 077: Filter out 'resume' lines
In the "Overlapping multiple requests" cases, the 3rd reqs (the break
point B) doesn't wait for the 2nd, and once resumed the I/O will just
continue. This is because the 2nd is already waiting for the 1st, and
in wait_serialising_requests() there is:
/* If the request is already (indirectly) waiting for us, or
* will wait for us as soon as it wakes up, then just go on
* (instead of producing a deadlock in the former case). */
if (!req->waiting_for) {
/* actually break */
...
}
Consequently, the following "sleep 100; resume A" command races with the
completion of that request, and sometimes results in an unexpected
order of output:
> @@ -56,9 +56,9 @@
> wrote XXX/XXX bytes at offset XXX
> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> blkdebug: Resuming request 'B'
> +blkdebug: Resuming request 'A'
> wrote XXX/XXX bytes at offset XXX
> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> -blkdebug: Resuming request 'A'
> wrote XXX/XXX bytes at offset XXX
> XXX bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> wrote XXX/XXX bytes at offset XXX
Filter out the "Resuming request" lines to make the output
deterministic.
Reported-by: Patchew <no-reply@patchew.org> Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20171113150026.4743-1-famz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
block/snapshot: dirty all dirty bitmaps on snapshot-switch
Snapshot-switch actually changes active state of disk so it should
reflect on dirty bitmaps. Otherwise next incremental backup using
these bitmaps will be invalid.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-id: 20171023092945.54532-1-vsementsov@virtuozzo.com Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Wed, 8 Nov 2017 12:13:06 +0000 (14:13 +0200)]
qcow2: Check that corrupted images can be repaired in iotest 060
We just fixed a few bugs that caused QEMU to crash when trying to
write to corrupted qcow2 images, and iotest 060 was expanded to test
all those scenarios.
In almost all cases the corrupted images can be repaired using
qemu-img, so this patch verifies that.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 0b1b95340ecdfbc6927e36adf2fd42ae6198747a.1510143008.git.berto@igalia.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Eric Blake [Thu, 9 Nov 2017 22:12:16 +0000 (16:12 -0600)]
iotests: Use new-style NBD connections
Old-style NBD is deprecated upstream (it is documented, but no
longer implemented in the reference implementation), and it is
severely limited (it cannot support structured replies, which
means it cannot support efficient handling of zeroes), when
compared to new-style NBD. We are better off having our iotests
favor new-style everywhere (although some explicit tests,
particularly 83, still cover old-style for back-compat reasons);
this is as simple as supplying the empty string as the default
export name, as it does not change the URI needed to connect a
client to the server. This also gives us more coverage of the
just-added structured reply code, when not overriding $QEMU_NBD
to intentionally point to an older server.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20171109221216.10248-1-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Max Reitz [Thu, 9 Nov 2017 20:30:25 +0000 (21:30 +0100)]
iotests: Make 136 less flaky
136 executes some AIO requests without a final aio_flush; then it
advances the virtual clock and thus expects the last access time of the
device to be less than the current time when queried (i.e. idle_time_ns
to be greater than 0). However, without the aio_flush, some requests
may be settled after the clock_step invocation. In that case,
idle_time_ns would be 0 and the test fails.
Fix this by adding an aio_flush if any AIO request other than some other
aio_flush has been executed.
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20171109203025.27493-6-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Max Reitz [Thu, 9 Nov 2017 20:30:24 +0000 (21:30 +0100)]
iotests: Make 083 less flaky
083 has (at least) two issues:
1. By launching the nbd-fault-injector in background, it may not be
scheduled until the first grep on its output file is executed.
However, until then, that file may not have been created yet -- so it
either does not exist yet (thus making the grep emit an error), or it
does exist but contains stale data (thus making the rest of the test
case work connect to a wrong address).
Fix this by explicitly overwriting the output file before executing
nbd-fault-injector.
2. The nbd-fault-injector prints things other than "Listening on...".
It also prints a "Closing connection" message from time to time. We
currently invoke sed on the whole file in the hope of it only
containing the "Listening on..." line yet. That hope is sometimes
shattered by the brutal reality of race conditions, so make the sed
script more robust.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20171109203025.27493-5-mreitz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Max Reitz [Thu, 9 Nov 2017 20:30:23 +0000 (21:30 +0100)]
iotests: Make 055 less flaky
First of all, test 055 does a valiant job of invoking pause_drive()
sometimes, but that is worth nothing without blkdebug. So the first
thing to do is to sprinkle a couple of "blkdebug::" in there -- with the
exception of the transaction tests, because the blkdebug break points
make the transaction QMP command hang (which is bad). In that case, we
can get away with throttling the block job that it effectively is
paused.
Then, 055 usually does not pause the drive before starting a block job
that should be cancelled. This means that the backup job might be
completed already before block-job-cancel is invoked; thus making the
test either fail (currently) or moot if cancel_and_wait() ignored this
condition. Fix this by pausing the drive before starting the job.
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20171109203025.27493-4-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Max Reitz [Thu, 9 Nov 2017 20:30:22 +0000 (21:30 +0100)]
iotests: Add missing 'blkdebug::' in 040
040 tries to invoke pause_drive() on a drive that does not use blkdebug.
Good idea, but let's use blkdebug to make it actually work.
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20171109203025.27493-3-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Max Reitz [Thu, 9 Nov 2017 20:30:21 +0000 (21:30 +0100)]
iotests: Make 030 less flaky
This patch fixes two race conditions in 030:
1. The first is in TestENOSPC.test_enospc(). After resuming the job,
querying it to confirm it is no longer paused may fail because in the
meantime it might have completed already. The same was fixed in
TestEIO.test_ignore() already (in commit 2c3b44da07d341557a8203cc509ea07fe3605e11).
2. The second is in TestSetSpeed.test_set_speed_invalid(): Here, a
stream job is started on a drive without any break points, with a
block-job-set-speed invoked subsequently. However, without any break
points, the job might have completed in the meantime (on tmpfs at
least); or it might complete before cancel_and_wait() which expects
the job to still exist. This can be fixed like everywhere else by
pausing the drive (installing break points) before starting the job
and letting cancel_and_wait() resume it.
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 20171109203025.27493-2-mreitz@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Fri, 3 Nov 2017 14:18:56 +0000 (16:18 +0200)]
qcow2: Assert that the crypto header does not overlap other metadata
The crypto header is initialized only when QEMU is creating a new
image, so there's no chance of this happening on a corrupted image.
If QEMU is really trying to allocate the header overlapping other
existing metadata sections then this is a serious bug in QEMU itself
so let's add an assertion.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: ae3d77f312fc0c5e0ac2bbd71676c0112eebe2e5.1509718618.git.berto@igalia.com Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Fri, 3 Nov 2017 14:18:55 +0000 (16:18 +0200)]
qcow2: Add iotest for an empty refcount table
This patch adds a simple iotest in which we try to write to an image
with an empty refcount table (i.e. with all entries set to 0).
This scenario was already handled by the existing consistency checks,
but we add an explicit test case for completeness.
Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 7e48b0e2ae1a0a18e0ee303b3045f130feec0474.1509718618.git.berto@igalia.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Fri, 3 Nov 2017 14:18:54 +0000 (16:18 +0200)]
qcow2: Add iotest for an image with header.refcount_table_offset == 0
This patch adds a simple iotest in which we try to write to an image
with the refcount table offset set to 0.
This scenario was already handled by the existing consistency checks,
but we add an explicit test case for completeness.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: feeceada92486bb8790b90f303fc9fe82a27391a.1509718618.git.berto@igalia.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Fri, 3 Nov 2017 14:18:53 +0000 (16:18 +0200)]
qcow2: Don't open images with header.refcount_table_clusters == 0
qcow2_do_open() is checking that header.refcount_table_clusters is not
too large, but it doesn't check that it's greater than zero. Apart
from the fact that an image like that is obviously corrupted, trying
to use it crashes QEMU since we end up with a null s->refcount_table
after qcow2_refcount_init().
These images can however be repaired, so allow opening them if the
BDRV_O_CHECK flag is set.
Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: f9750f50c80359babba11062e88f5075a47e8e16.1509718618.git.berto@igalia.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Fri, 3 Nov 2017 14:18:52 +0000 (16:18 +0200)]
qcow2: Prevent allocating compressed clusters at offset 0
If the refcount data is corrupted then we can end up trying to
allocate a new compressed cluster at offset 0 in the image, triggering
an assertion in qcow2_alloc_bytes() that would crash QEMU:
qcow2_alloc_bytes: Assertion `offset' failed.
This patch adds an explicit check for this scenario and a new test
case.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: fb53467cf48e95ff3330def1cf1003a5b862b7d9.1509718618.git.berto@igalia.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Fri, 3 Nov 2017 14:18:51 +0000 (16:18 +0200)]
qcow2: Prevent allocating L2 tables at offset 0
If the refcount data is corrupted then we can end up trying to
allocate a new L2 table at offset 0 in the image, triggering an
assertion in the qcow2 cache that would crash QEMU:
This patch adds an explicit check for this scenario and a new test
case.
Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 92dac37191ae7844a2da22c122204eb493cc3133.1509718618.git.berto@igalia.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Alberto Garcia [Fri, 3 Nov 2017 14:18:50 +0000 (16:18 +0200)]
qcow2: Prevent allocating refcount blocks at offset 0
Each entry in the qcow2 cache contains an offset field indicating the
location of the data in the qcow2 image. If the offset is 0 then it
means that the entry contains no data and is available to be used when
needed.
Because of that it is not possible to store in the cache the first
cluster of the qcow2 image (offset = 0). This is not a problem because
that cluster always contains the qcow2 header and we're not using this
cache for that.
However, if the qcow2 image is corrupted it can happen that we try to
allocate a new refcount block at offset 0, triggering this assertion
and crashing QEMU:
Peter Maydell [Tue, 14 Nov 2017 16:11:19 +0000 (16:11 +0000)]
Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging
Pull request
The following disk I/O throttling fixes solve recent bugs.
# gpg: Signature made Tue 14 Nov 2017 10:37:12 GMT
# gpg: using RSA key 0x9CA4ABB381AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>"
# gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>"
# Primary key fingerprint: 8695 A8BF D3F9 7CDA AC35 775A 9CA4 ABB3 81AB 73C8
* remotes/stefanha/tags/block-pull-request:
qemu-iotests: Test I/O limits with removable media
block: Leave valid throttle timers when removing a BDS from a backend
block: Check for inserted BlockDriverState in blk_io_limits_disable()
throttle-groups: drain before detaching ThrottleState
block: all I/O should be completed before removing throttle timers.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Tue, 14 Nov 2017 13:53:00 +0000 (13:53 +0000)]
Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
# gpg: Signature made Tue 14 Nov 2017 02:05:34 GMT
# gpg: using RSA key 0xEF04965B398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat) <jasowang@redhat.com>"
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg: It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 215D 46F4 8246 689E C77F 3562 EF04 965B 398D 6211
* remotes/jasowang/tags/net-pull-request:
net/socket: fix coverity issue
Add new PCI ID for i82559a
Fix eepro100 simple transmission mode
colo: Consolidate the duplicate code chunk into a routine
colo-compare: Fix comments
colo-compare: compare the packet in a specified Connection
colo-compare: Insert packet into the suitable position of packet queue directly
net: fix check for number of parameters to -netdev socket
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Tue, 14 Nov 2017 10:26:08 +0000 (10:26 +0000)]
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20171113' into staging
target-arm queue:
* translate-a64.c: silence gcc5 warning
* highbank: validate register offset before access
* MAINTAINERS: Add entries for Smartfusion2
* accel/tcg/translate-all: expand cpu_restore_state addr check
(so usermode insn aborts don't crash with an assertion failure)
* fix TCG initialization of some Arm boards by allowing them
to specify min/default number of CPUs to create
# gpg: Signature made Mon 13 Nov 2017 14:11:09 GMT
# gpg: using RSA key 0x3C2525ED14360CDE
# gpg: Good signature from "Peter Maydell <peter.maydell@linaro.org>"
# gpg: aka "Peter Maydell <pmaydell@gmail.com>"
# gpg: aka "Peter Maydell <pmaydell@chiark.greenend.org.uk>"
# Primary key fingerprint: E1A5 C593 CD41 9DE2 8E83 15CF 3C25 25ED 1436 0CDE
* remotes/pmaydell/tags/pull-target-arm-20171113:
accel/tcg/translate-all: expand cpu_restore_state addr check
hw: add .min_cpus and .default_cpus fields to machine_class
xlnx-zcu102: Specify the max number of CPUs for the EP108
xlnx-zcu102: Add an info message deprecating the EP108
xlnx-zynqmp: Properly support the smp command line option
qom: move CPUClass.tcg_initialize to a global
MAINTAINERS: Add entries for Smartfusion2
highbank: validate register offset before access
arm/translate-a64: mark path as unreachable to eliminate warning
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
ie, all irqs are masked and XIRR is null, while we should get the
same output as with the emulated XICS.
If the guest is then migrated, 'info pic' shows the expected values
on both source and destination.
The problem is that QEMU doesn't synchronize with KVM before printing
the XICS state. Migration happens to fix the output because it enforces
synchronization with KVM.
To fix the invalid output of 'info pic', this patch introduces a new
synchronize_state operation for both ICPStateClass and ICSStateClass.
The ICP operation relies on run_on_cpu() in order to kick the vCPU
and avoid sleeping on KVM_GET_ONE_REG.
Signed-off-by: Greg Kurz <groug@kaod.org> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Sam Bobroff [Mon, 6 Nov 2017 03:14:35 +0000 (14:14 +1100)]
target/ppc: correct htab shift for hash on radix
KVM HV will soon support running a guest in hash mode on a POWER9 host
running in radix mode (see [1]), however the guest currently fails to
boot.
This is because the "htab_shift" value (the size of the MMU's hash
table) is added to the device tree before KVM has had a chance to
change it. If the host is in hash mode, KVM does not need to change it
and so the problem is not seen, but when the host is in radix mode a
change is required and we see a problem.
To fix this, move the call spapr_setup_hpt_and_vrma() (where
htab_shift could be changed) up a little so that it's called before
spapr_h_cas_compose_response() (where htab_shift is added to the
device tree).
Signed-off-by: Sam Bobroff <sam.bobroff@au1.ibm.com>
[1] See http://www.spinics.net/lists/kvm-ppc/msg13057.html Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Alberto Garcia [Fri, 10 Nov 2017 18:54:48 +0000 (20:54 +0200)]
qemu-iotests: Test I/O limits with removable media
This test hotplugs a CD drive to a VM and checks that I/O limits can
be set only when the drive has media inserted and that they are kept
when the media is replaced.
This also tests the removal of a device with valid I/O limits set but
no media inserted. This involves deleting and disabling the limits
of a BlockBackend without BlockDriverState, a scenario that has been
crashing until the fixes from the last couple of patches.
[Python PEP8 fixup: "Don't use spaces are the = sign when used to
indicate a keyword argument or a default parameter value"
--Stefan]
Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 071eb397118ed207c5a7f01d58766e415ee18d6a.1510339534.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Alberto Garcia [Fri, 10 Nov 2017 18:54:47 +0000 (20:54 +0200)]
block: Leave valid throttle timers when removing a BDS from a backend
If a BlockBackend has I/O limits set then its ThrottleGroupMember
structure uses the AioContext from its attached BlockDriverState.
Those two contexts must be kept in sync manually. This is not
ideal and will be fixed in the future by removing the throttling
configuration from the BlockBackend and storing it in an implicit
filter node instead, but for now we have to live with this.
When you remove the BlockDriverState from the backend then the
throttle timers are destroyed. If a new BlockDriverState is later
inserted then they are created again using the new AioContext.
There are a couple of problems with this:
a) The code manipulates the timers directly, leaving the
ThrottleGroupMember.aio_context field in an inconsisent state.
b) If you remove the I/O limits (e.g by destroying the backend)
when the timers are gone then throttle_group_unregister_tgm()
will attempt to destroy them again, crashing QEMU.
While b) could be fixed easily by allowing the timers to be freed
twice, this would result in a situation in which we can no longer
guarantee that a valid ThrottleState has a valid AioContext and
timers.
This patch ensures that the timers and AioContext are always valid
when I/O limits are set, regardless of whether the BlockBackend has a
BlockDriverState inserted or not.
[Fixed "There'a" typo as suggested by Max Reitz <mreitz@redhat.com>
--Stefan]
Reported-by: sochin jiang <sochin.jiang@huawei.com> Signed-off-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: e089c66e7c20289b046d782cea4373b765c5bc1d.1510339534.git.berto@igalia.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Alberto Garcia [Fri, 10 Nov 2017 18:54:46 +0000 (20:54 +0200)]
block: Check for inserted BlockDriverState in blk_io_limits_disable()
When you set I/O limits using block_set_io_throttle or the command
line throttling.* options they are kept in the BlockBackend regardless
of whether a BlockDriverState is attached to the backend or not.
Therefore when removing the limits using blk_io_limits_disable() we
need to check if there's a BDS before attempting to drain it, else it
will crash QEMU. This can be reproduced very easily using HMP:
* remotes/kraxel/tags/vga-20171110-pull-request:
vmsvga: use ARRAY_SIZE macro
vga: fix region checks in wraparound case
virtio-gpu: fix bug in host memory calculation.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This happens because blk_set_aio_context() detaches the ThrottleState
while requests may still be in flight:
if (tgm->throttle_state) {
throttle_group_detach_aio_context(tgm);
throttle_group_attach_aio_context(tgm, new_context);
}
This patch encloses the detach/attach calls in a drained region so no
I/O request is left hanging. Also add assertions so we don't make the
same mistake again in the future.
Reported-by: Yongxue Hong <yhong@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-id: 20171110151934.16883-1-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Zhengui [Sat, 21 Oct 2017 05:34:00 +0000 (13:34 +0800)]
block: all I/O should be completed before removing throttle timers.
In blk_remove_bs, all I/O should be completed before removing throttle
timers. If there has inflight I/O, removing throttle timers here will
cause the inflight I/O never return.
This patch add bdrv_drained_begin before throttle_timers_detach_aio_context
to let all I/O completed before removing throttle timers.
[Moved declaration of bs as suggested by Alberto Garcia
<berto@igalia.com>.
--Stefan]
Signed-off-by: Zhengui <lizhengui@huawei.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com>
Message-id: 1508564040-120700-1-git-send-email-lizhengui@huawei.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
We are still seeing signals during translation time when we walk over
a page protection boundary. This expands the check to ensure the host
PC is inside the code generation buffer. The original suggestion was
to check versus tcg_ctx.code_gen_ptr but as we now segment the
translation buffer we have to settle for just a general check for
being inside.
I've also fixed up the declaration to make it clear it can deal with
invalid addresses. A later patch will fix up the call sites.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reported-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Laurent Vivier <laurent@vivier.eu> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20171108153245.20740-2-alex.bennee@linaro.org Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Cc: Richard Henderson <rth@twiddle.net> Tested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Emilio G. Cota [Mon, 13 Nov 2017 13:55:27 +0000 (13:55 +0000)]
hw: add .min_cpus and .default_cpus fields to machine_class
max_cpus needs to be an upper bound on the number of vCPUs
initialized; otherwise TCG region initialization breaks.
Some boards initialize a hard-coded number of vCPUs, which is not
captured by the global max_cpus and therefore breaks TCG initialization.
Fix it by adding the .min_cpus field to machine_class.
This commit also changes some user-facing behaviour: we now die if
-smp is below this hard-coded vCPU minimum instead of silently
ignoring the passed -smp value (sometimes announcing this by printing
a warning). However, the introduction of .default_cpus lessens the
likelihood that users will notice this: if -smp isn't set, we now
assign the value in .default_cpus to both smp_cpus and max_cpus. IOW,
if a user does not set -smp, they always get a correct number of vCPUs.
This change fixes 3468b59 ("tcg: enable multiple TCG contexts in
softmmu", 2017-10-24), which broke TCG initialization for some
ARM boards.
Fixes: 3468b59e18b179bc63c7ce934de912dfa9596122 Reported-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com> Signed-off-by: Emilio G. Cota <cota@braap.org>
Message-id: 1510343626-25861-6-git-send-email-cota@braap.org Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Emilio G. Cota <cota@braap.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Emilio G. Cota [Mon, 13 Nov 2017 13:55:26 +0000 (13:55 +0000)]
xlnx-zcu102: Specify the max number of CPUs for the EP108
Just like the zcu102, the ep108 can instantiate several CPUs.
Signed-off-by: Emilio G. Cota <cota@braap.org> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>
Message-id: 1510343626-25861-5-git-send-email-cota@braap.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Alistair Francis [Mon, 13 Nov 2017 13:55:26 +0000 (13:55 +0000)]
xlnx-zcu102: Add an info message deprecating the EP108
The EP108 was an early access development board that is no longer used.
Add an info message to convert any users to the ZCU102 instead. On QEMU
they are both identical.
This patch also updated the qemu-doc.texi file to indicate that the
EP108 has been deprecated.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Reviewed-by: Emilio G. Cota <cota@braap.org>
Message-id: 1510343626-25861-4-git-send-email-cota@braap.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Emilio G. Cota [Mon, 13 Nov 2017 13:55:25 +0000 (13:55 +0000)]
qom: move CPUClass.tcg_initialize to a global
55c3cee ("qom: Introduce CPUClass.tcg_initialize", 2017-10-24)
introduces a per-CPUClass bool that we check so that the target CPU
is initialized for TCG only once. This works well except when
we end up creating more than one CPUClass, in which case we end
up incorrectly initializing TCG more than once, i.e. once for
each CPUClass.
This can be replicated with:
$ aarch64-softmmu/qemu-system-aarch64 -machine xlnx-zcu102 -smp 6 \
-global driver=xlnx,,zynqmp,property=has_rpu,value=on
In this case the class name of the "RPUs" is prefixed by "cortex-r5-",
whereas the "regular" CPUs are prefixed by "cortex-a53-". This
results in two CPUClass instances being created.
Fix it by introducing a static variable, so that only the first
target CPU being initialized will initialize the target-dependent
part of TCG, regardless of CPUClass instances.
Fixes: 55c3ceef61fcf06fc98ddc752b7cce788ce7680b Signed-off-by: Emilio G. Cota <cota@braap.org> Reviewed-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Alistair Francis <alistair.francis@xilinx.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Alistair Francis <alistair.francis@xilinx.com>
Message-id: 1510343626-25861-2-git-send-email-cota@braap.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
An 'offset' parameter sent to highbank register r/w functions
could be greater than number(NUM_REGS=0x200) of hb registers,
leading to an OOB access issue. Add check to avoid it.
Reported-by: Moguofang (Dennis mo) <moguofang@huawei.com> Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-id: 20171113062658.9697-1-ppandit@redhat.com Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Emilio G. Cota [Mon, 13 Nov 2017 13:55:24 +0000 (13:55 +0000)]
arm/translate-a64: mark path as unreachable to eliminate warning
Fixes the following warning when compiling with gcc 5.4.0 with -O1
optimizations and --enable-debug:
target/arm/translate-a64.c: In function ‘aarch64_tr_translate_insn’:
target/arm/translate-a64.c:2361:8: error: ‘post_index’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (!post_index) {
^
target/arm/translate-a64.c:2307:10: note: ‘post_index’ was declared here
bool post_index;
^
target/arm/translate-a64.c:2386:8: error: ‘writeback’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
if (writeback) {
^
target/arm/translate-a64.c:2308:10: note: ‘writeback’ was declared here
bool writeback;
^
Note that idx comes from selecting 2 bits, and therefore its value
can be at most 3.
Signed-off-by: Emilio G. Cota <cota@braap.org> Acked-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 1510087611-1851-1-git-send-email-cota@braap.org Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Jens Freimann [Mon, 6 Nov 2017 14:05:46 +0000 (15:05 +0100)]
net/socket: fix coverity issue
This fixes coverity issue CID1005339.
Make sure that saddr is not used uninitialized if the
mcast parameter is NULL.
Cc: qemu-stable@nongnu.org Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Jens Freimann <jfreimann@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Mike Nawrocki [Tue, 7 Nov 2017 18:35:03 +0000 (13:35 -0500)]
Add new PCI ID for i82559a
Adds a new PCI ID for the i82559a (0x8086 0x1030) interface. The
"x-use-alt-device-id" property controls whether this new ID is to be
used, and is true by default, and set to false in a compat entry.
Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Mike Nawrocki [Tue, 7 Nov 2017 18:35:02 +0000 (13:35 -0500)]
Fix eepro100 simple transmission mode
The simple transmission mode was treating the area immediately after the
transmit command block (TCB) as if it were a transmit buffer descriptor,
when in reality it is simply the packet data. This change simply copies
the data following the TCB into the packet buffer.
Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Mao Zhongyi [Fri, 13 Oct 2017 06:32:09 +0000 (14:32 +0800)]
colo: Consolidate the duplicate code chunk into a routine
Consolidate the code that extract the ip address(src,dst) and
port number(src,dst) of the packet into a separate routine
extract_ip_and_port() since the same chunk of code is called
from two place.
Cc: Zhang Chen <zhangckid@gmail.com> Cc: Li Zhijian <lizhijian@cn.fujitsu.com> Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Mao Zhongyi [Fri, 13 Oct 2017 06:32:08 +0000 (14:32 +0800)]
colo-compare: Fix comments
Cc: Zhang Chen <zhangckid@gmail.com> Cc: Li Zhijian <lizhijian@cn.fujitsu.com> Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> Signed-off-by: Zhang Chen <zhangckid@gmail.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Mao Zhongyi [Fri, 13 Oct 2017 06:32:07 +0000 (14:32 +0800)]
colo-compare: compare the packet in a specified Connection
A package from pri_indev or sec_indev only belongs to a particular
Connection, so we only need to compare the package in the specified
Connection's primary_list and secondary_list, rather than for each
the whole Connection list to compare. This is time-consuming and
unnecessary.
Less checkpoint more efficiency.
Cc: Zhang Chen <zhangckid@gmail.com> Cc: Li Zhijian <lizhijian@cn.fujitsu.com> Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Mao Zhongyi [Fri, 13 Oct 2017 06:32:06 +0000 (14:32 +0800)]
colo-compare: Insert packet into the suitable position of packet queue directly
Currently, a packet from pri_dev or sec_dev is fristly pushed at the
tail of the primary or secondary packet queue then sorted by the tcp
sequence number.
Now, this patch use g_queue_insert_sorted to insert the packet directly
into the suitable position to avoid ordering all packets each time when
a new packet is comming, thereby increasing efficiency.
In addition, consolidate the code that add a packet to the list of
Connection (primary or secondary) into a separate routine colo_insert_packet()
since the same chunk of code is called from two place.
Cc: Zhang Chen <zhangckid@gmail.com> Cc: Li Zhijian <lizhijian@cn.fujitsu.com> Cc: Jason Wang <jasowang@redhat.com> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> Signed-off-by: Zhang Chen <zhangckid@gmail.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
net: fix check for number of parameters to -netdev socket
Since commit 0f8c289ad "net: fix -netdev socket,fd= for UDP sockets"
we allow more than one parameter for -netdev socket. But now
we run into an assert when no parameter at all is specified
Fix this by reverting the change of the if condition done in 0f8c289ad.
Cc: Jason Wang <jasowang@redhat.com> Cc: qemu-stable@nongnu.org Fixes: 0f8c289ad539feb5135c545bea947b310a893f4b Reported-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com> Signed-off-by: Jens Freimann <jfreimann@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Peter Maydell [Fri, 10 Nov 2017 16:01:35 +0000 (16:01 +0000)]
Merge remote-tracking branch 'remotes/berrange/tags/pull-qcrypto-2017-11-08-1' into staging
Merge qcrypto 2017/11/08 v1
# gpg: Signature made Wed 08 Nov 2017 11:06:38 GMT
# gpg: using RSA key 0xBE86EBB415104FDF
# gpg: Good signature from "Daniel P. Berrange <dan@berrange.com>"
# gpg: aka "Daniel P. Berrange <berrange@redhat.com>"
# Primary key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF
* remotes/berrange/tags/pull-qcrypto-2017-11-08-1:
crypto: afalg: fix a NULL pointer dereference
tests: Run the luks tests in test-crypto-block only if encryption is available
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 10 Nov 2017 15:05:56 +0000 (15:05 +0000)]
Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.11-20171108' into staging
ppc patch queue 2017-11-08
Here's the current set of accumulated ppc patches for qemu-2.11.
Since we're now in hard freeze these are all bugfixes (although some
fix a bug by way of a cleanup).
Gerd Hoffmann [Mon, 30 Oct 2017 10:28:30 +0000 (11:28 +0100)]
vga: fix region checks in wraparound case
Cc: "Dr. David Alan Gilbert" <dgilbert@redhat.com> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-id: 20171030102830.4469-1-kraxel@redhat.com
Tao Wu [Wed, 8 Nov 2017 22:53:40 +0000 (14:53 -0800)]
slirp: don't zero the whole ti_i when m == NULL
98c63057d2144fb81681580cd84c13c93794c96e ('slirp: Factorizing
tcpiphdr structure with an union') introduced a memset call to clear
possibly-undefined fields in ti. This however overwrites src/dst/pr which
are used below.
So let us clear only the unused fields.
This should fix some rare cases (some RST cases, keep alive probes)
where packets would be sent to 0.0.0.0.
Signed-off-by: Tao Wu <lepton@google.com> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
Eric Blake [Wed, 8 Nov 2017 21:57:03 +0000 (15:57 -0600)]
nbd/server: Fix structured read of length 0
The NBD spec was recently clarified to state that a read of length 0
should not be attempted by a compliant client; but that a server must
still handle it correctly in an unspecified manner (that is, either
a successful no-op or an error reply, but not a crash) [1]. However,
it also implies that NBD_REPLY_TYPE_OFFSET_DATA must have a non-zero
payload length, but our existing code was replying with a chunk
that a picky client could reject as invalid because it was missing
a payload (our own client implementation was recently patched to be
that picky, after first fixing it to not send 0-length requests).
We are already doing successful no-ops for 0-length writes and for
non-structured reads; so for consistency, we want structured reply
reads to also be a no-op. The easiest way to do this is to return
a NBD_REPLY_TYPE_NONE chunk; this is best done via a new helper
function (especially since future patches for other structured
replies may benefit from using the same helper).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171108215703.9295-8-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Eric Blake [Wed, 8 Nov 2017 21:57:02 +0000 (15:57 -0600)]
nbd-client: Stricter enforcing of structured reply spec
Ensure that the server is not sending unexpected chunk lengths
for either the NONE or the OFFSET_DATA chunk, nor unexpected
hole length for OFFSET_HOLE. This will flag any server as
broken that responds to a zero-length read with an OFFSET_DATA
(what our server currently does, but that's about to be fixed)
or with OFFSET_HOLE, even though we previously fixed our client
to never be able to send such a request over the wire.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171108215703.9295-7-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Eric Blake [Wed, 8 Nov 2017 21:57:01 +0000 (15:57 -0600)]
nbd-client: Short-circuit 0-length operations
The NBD spec was recently clarified to state that clients should
not send 0-length requests to the server, as the server behavior
is undefined [1]. We know that qemu-nbd's behavior is a successful
no-op (once it has filtered for read-only exports), but other NBD
implementations might return an error. To avoid any questionable
server implementations, it is better to just short-circuit such
requests on the client side (we are relying on the block layer to
already filter out requests such as invalid offset, write to a
read-only volume, and so forth); do the short-circuit as late as
possible to still benefit from protections from assertions that
the block layer is not violating our assumptions.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171108215703.9295-6-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Eric Blake [Wed, 8 Nov 2017 21:57:00 +0000 (15:57 -0600)]
nbd: Fix struct name for structured reads
A closer read of the NBD spec shows that a structured reply chunk
for a hole is not quite identical to the prefix of a data chunk,
because the hole has to also send a 32-bit size field. Although
we do not yet send holes, we should fix the misleading information
in our header and make it easier for a future patch to support
sparse reads. Messed up in commit bae245d1.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171108215703.9295-5-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Eric Blake [Wed, 8 Nov 2017 21:56:59 +0000 (15:56 -0600)]
nbd/client: Nicer trace of structured reply
It's useful to know which structured reply chunk is being processed.
Missed in commit d2febedb.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171108215703.9295-4-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Eric Blake [Wed, 8 Nov 2017 21:56:58 +0000 (15:56 -0600)]
nbd-client: Refuse read-only client with BDRV_O_RDWR
The NBD spec says that clients should not try to write/trim to
an export advertised as read-only by the server. But we failed
to check that, and would allow the block layer to use NBD with
BDRV_O_RDWR even when the server is read-only, which meant we
were depending on the server sending a proper EPERM failure for
various commands, and also exposes a leaky abstraction: using
qemu-io in read-write mode would succeed on 'w -z 0 0' because
of local short-circuiting logic, but 'w 0 0' would send a
request over the wire (where it then depends on the server, and
fails at least for qemu-nbd but might pass for other NBD
implementations).
With this patch, a client MUST request read-only mode to access
a server that is doing a read-only export, or else it will get
a message like:
can't open device nbd://localhost:10809/foo: request for write access conflicts with read-only export
It is no longer possible to even attempt writes over the wire
(including the corner case of 0-length writes), because the block
layer enforces the explicit read-only request; this matches the
behavior of qcow2 when backed by a read-only POSIX file.
Fix several iotests to comply with the new behavior (since
qemu-nbd of an internal snapshot, as well as nbd-server-add over QMP,
default to a read-only export, we must tell blockdev-add/qemu-io to
set up a read-only client).
CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171108215703.9295-3-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Eric Blake [Wed, 8 Nov 2017 21:56:57 +0000 (15:56 -0600)]
nbd-client: Fix error message typos
Provide missing spaces that are required when using string
concatenation to break error messages across source lines.
Introduced in commit f140e300.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20171108215703.9295-2-eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
We added the entry to insn-data.def, but failed to update op_risbg
to match. No need to special-case the imask inversion, since that
is already ~0 for RISBG (and now RISBGN).
Fixes: 375ee58bedcda359011fe7fa99e0647f66f9ffa0 Fixes: https://bugs.launchpad.net/qemu/+bug/1701798 (s390x part) Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20171107145546.767-1-richard.henderson@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Tested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Makefile: Capstone: Add support for cross compile ranlib
When cross compiling QEMU for Windows we need to specify the cross
version of ranlib to avoid build errors when building capstone. This
patch ensures we use the same cross prefix on ranlib as other toolchain
components.
- Fedora23 mingw
- RHEL-7.2 with mingw packages from epel:
LINK qemu-img.exe
build-win64/capstone/capstone.lib: error adding symbols: Archive has no
index; run ranlib to add one
collect2: error: ld returned 1 exit status
$ x86_64-w64-mingw32-ar --version
GNU ar (GNU Binutils) 2.25
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <e457d4e906dceea4de6c3431813a06b137c1ab9c.1510103351.git.alistair.francis@xilinx.com> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
namelen should be here, length is unrelated, and always 0 at this
point. Broken in introduction in commit f37708f6, but mostly
harmless (replying with '' as the name does not violate protocol,
and does not confuse qemu as the nbd client since our implementation
does not ask for the name; but might confuse some other client that
does ask for the name especially if the default export is different
than the export name being queried).
Adding an assert makes it obvious that we are not skipping any bytes
in the client's message, as well as making it obvious that we were
using the wrong variable.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> CC: qemu-stable@nongnu.org
Message-Id: <20171101154204.27146-1-vsementsov@virtuozzo.com>
[eblake: improve commit message, squash in assert addition] Signed-off-by: Eric Blake <eblake@redhat.com>
Sergio Lopez [Wed, 8 Nov 2017 06:34:47 +0000 (07:34 +0100)]
util/async: use atomic_mb_set in qemu_bh_cancel
Commit b7a745d added a qemu_bh_cancel call to the completion function
as an optimization to prevent it from unnecessarily rescheduling itself.
This completion function is scheduled from worker_thread, after setting
the state of a ThreadPoolElement to THREAD_DONE.
This was considered to be safe, as the completion function restarts the
loop just after the call to qemu_bh_cancel. But, as this loop lacks a HW
memory barrier, the read of req->state may actually happen _before_ the
call, seeing it still as THREAD_QUEUED, and ending the completion
function without having processed a pending TPE linked at pool->head:
The source of the misunderstanding was that qemu_bh_cancel is now being
used by the _consumer_ rather than the producer, and therefore now needs
to have acquire semantics just like e.g. aio_bh_poll.
In some situations, if there are no other independent requests in the
same aio context that could eventually trigger the scheduling of the
completion function, the omitted TPE and all operations pending on it
will get stuck forever.
[Added Sergio's updated wording about the HW memory barrier.
--Stefan]
Signed-off-by: Sergio Lopez <slp@redhat.com>
Message-id: 20171108063447.2842-1-slp@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Longpeng [Tue, 7 Nov 2017 11:32:06 +0000 (19:32 +0800)]
crypto: afalg: fix a NULL pointer dereference
Test-crypto-hash calls qcrypto_hash_bytesv/digest/base64 with
errp=NULL, this will cause a NULL pointer dereference if afalg_driver
doesn't support requested algos:
ret = qcrypto_hash_afalg_driver.hash_bytesv(alg, iov, niov,
result, resultlen,
errp);
if (ret == 0) {
return ret;
}
error_free(*errp); // <--- here
Because the error message is thrown away immediately, we should
just pass NULL to hash_bytesv(). There is also the same problem in
afalg-backend cipher & hmac, let's fix them together.
Reviewed-by: Eric Blake <eblake@redhat.com> Reported-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Longpeng <longpeng2@huawei.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Thomas Huth [Fri, 3 Nov 2017 11:54:52 +0000 (12:54 +0100)]
tests: Run the luks tests in test-crypto-block only if encryption is available
The test-crypto-block currently fails if encryption has not been
compiled into QEMU:
TEST: tests/test-crypto-block... (pid=22231)
/crypto/block/qcow: OK
/crypto/block/luks/default:
Unexpected error in qcrypto_pbkdf2() at qemu/crypto/pbkdf-stub.c:41:
FAIL
GTester: last random seed: R02Sbbb5b6f299c6727f41bb50ba4aa6ef5c
(pid=22237)
/crypto/block/luks/aes-256-cbc-plain64:
Unexpected error in qcrypto_pbkdf2() at qemu/crypto/pbkdf-stub.c:41:
FAIL
GTester: last random seed: R02S3e27992a5ab4cc95e141c4ed3c7f0d2e
(pid=22239)
/crypto/block/luks/aes-256-cbc-essiv:
Unexpected error in qcrypto_pbkdf2() at qemu/crypto/pbkdf-stub.c:41:
FAIL
GTester: last random seed: R02S51b52bb02a66c42d8b331fd305384f53
(pid=22241)
FAIL: tests/test-crypto-block
So run the luks test only if the required encryption support is available.
Signed-off-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
s390x/pci: let pci devices start in configured mode
Currently, to enable a pci device in the guest, the user has to issue
echo 1 > /sys/bus/pci/slots/00000000/power. This is not what people
expect. On an LPAR, the user can put a PCI device in configured or
deconfigured state via IOCDS. The "start in deconfigured state" can be
used for "sharing" a pci function across LPARs. This is not what we are
going to use in KVM, so always start configured.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com> Acked-by: Yi Min Zhao <zyimin@linux.vnet.ibm.com> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Message-Id: <20171107175455.73793-2-borntraeger@de.ibm.com> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
test_multi_co_schedule_entry() set to_schedule[id] in the final loop
iteration before terminating the coroutine. There is a race condition
where the main thread attempts to enter the terminating or terminated
coroutine when signalling coroutines to stop:
atomic_mb_set(&now_stopping, true);
for (i = 0; i < NUM_CONTEXTS; i++) {
ctx_run(i, finish_cb, NULL); <--- enters dead coroutine!
to_schedule[i] = NULL;
}
Make sure only to set to_schedule[id] if this coroutine really needs to
be scheduled!
Reported-by: "R.Nageswara Sastry" <nasastry@in.ibm.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20171106190233.1175-1-stefanha@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>