Ladi Prosek [Tue, 30 May 2017 08:59:43 +0000 (10:59 +0200)]
virtio-serial-bus: Unset hotplug handler when unrealize
Virtio serial device controls the lifetime of virtio-serial-bus and
virtio-serial-bus links back to the device via its hotplug-handler
property. This extra ref-count prevents the device from getting
finalized, leaving the VirtIODevice memory listener registered and
leading to use-after-free later on.
This patch addresses the same issue as Fam Zheng's
"virtio-scsi: Unset hotplug handler when unrealize"
only for a different virtio device.
Cc: qemu-stable@nongnu.org Signed-off-by: Ladi Prosek <lprosek@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com>
(cherry picked from commit f811f97040a48358b456b46ecbc9167f0131034f) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Kevin Wolf [Mon, 29 May 2017 12:08:32 +0000 (14:08 +0200)]
mirror: Drop permissions on s->target on completion
This fixes an assertion failure that was triggered by qemu-iotests 129
on some CI host, while the same test case didn't seem to fail on other
hosts.
Essentially the problem is that the blk_unref(s->target) in
mirror_exit() doesn't necessarily mean that the BlockBackend goes away
immediately. It is possible that the job completion was triggered nested
in mirror_drain(), which looks like this:
In this case, the write permissions for s->target are retained until
after blk_drain(), which makes removing mirror_top_bs fail for the
active commit case (can't have a writable backing file in the chain
without the filter driver).
Explicitly dropping the permissions first means that the additional
reference doesn't hurt and the job can complete successfully even if
called from the nested blk_drain().
Cc: qemu-stable@nongnu.org Signed-off-by: Kevin Wolf <kwolf@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit 63c8ef289087a225d445319d047501d4fe593687) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Mon, 5 Jun 2017 20:38:43 +0000 (15:38 -0500)]
block: Guarantee that *file is set on bdrv_get_block_status()
We document that *file is valid if the return is not an error and
includes BDRV_BLOCK_OFFSET_VALID, but forgot to obey this contract
when a driver (such as blkdebug) lacks a callback. Messed up in
commit 67a0fd2 (v2.6), when we added the file parameter.
Enhance qemu-iotest 177 to cover this, using a sequence that would
print garbage or even SEGV, because it was dererefencing through
uninitialized memory. [The resulting test output shows that we
have less-than-ideal block status from the blkdebug driver, but
that's a separate fix coming up soon.]
Setting *file on all paths that return BDRV_BLOCK_OFFSET_VALID is
enough to fix the crash, but we can go one step further: always
setting *file, even on error, means that a broken caller that
blindly dereferences file without checking for error is now more
likely to get a reliable SEGV instead of randomly acting on garbage,
making it easier to diagnose such buggy callers. Adding an
assertion that file is set where expected doesn't hurt either.
CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 81c219ac6ce0d6182e35f3976f2caa4cefcaf9f0) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Thu, 4 May 2017 17:37:45 +0000 (12:37 -0500)]
block: Simplify BDRV_BLOCK_RAW recursion
Since we are already in coroutine context during the body of
bdrv_co_get_block_status(), we can shave off a few layers of
wrappers when recursing to query the protocol when a format driver
returned BDRV_BLOCK_RAW.
Note that we are already using the correct recursion later on in
the same function, when probing whether the protocol layer is sparse
in order to find out if we can add BDRV_BLOCK_ZERO to an existing
BDRV_BLOCK_DATA|BDRV_BLOCK_OFFSET_VALID.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 20170504173745.27414-1-eblake@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit ee29d6adefcca7e76abb124183814ed3acc74fac)
* prereq for 81c219a Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Sat, 29 Apr 2017 19:14:19 +0000 (14:14 -0500)]
tests: Add coverage for recent block geometry fixes
Use blkdebug's new geometry constraints to emulate setups that
have needed past regression fixes: write zeroes asserting
when running through a loopback block device with max-transfer
smaller than cluster size, and discard rounding away portions
of requests not aligned to preferred boundaries. Also, add
coverage that the block layer is honoring max transfer limits.
For now, a single iotest performs all actions, with the idea
that we can add future blkdebug constraint test cases in the
same file; but it can be split into multiple iotests if we find
reason to run one portion of the test in more setups than what
are possible in the other.
For reference, the final portion of the test (checking whether
discard passes as much as possible to the lowest layers of the
stack) works as follows:
qemu-io: discard 30M at 80000001, passed to blkdebug
blkdebug: discard 511 bytes at 80000001, -ENOTSUP (smaller than
blkdebug's 512 align)
blkdebug: discard 14371328 bytes at 80000512, passed to qcow2
qcow2: discard 739840 bytes at 80000512, -ENOTSUP (smaller than
qcow2's 1M align)
qcow2: discard 13M bytes at 77M, succeeds
blkdebug: discard 15M bytes at 90M, passed to qcow2
qcow2: discard 15M bytes at 90M, succeeds
blkdebug: discard 1356800 bytes at 105M, passed to qcow2
qcow2: discard 1M at 105M, succeeds
qcow2: discard 308224 bytes at 106M, -ENOTSUP (smaller than qcow2's
1M align)
blkdebug: discard 1 byte at 111457280, -ENOTSUP (smaller than
blkdebug's 512 align)
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170429191419.30051-10-eblake@redhat.com
[mreitz: For cooperation with image locking, add -r to the qemu-io
invocation which verifies the image content] Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit 40812d937392fddc11f72a668aef251039cc15ce)
Conflicts:
tests/qemu-iotests/group
* dropped context dependency on other test groups
* prereq for 81c219a Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Sat, 29 Apr 2017 19:14:18 +0000 (14:14 -0500)]
blkdebug: Add ability to override unmap geometries
Make it easier to simulate various unusual hardware setups (for
example, recent commits 3482b9b and b8d0a98 affect the Dell
Equallogic iSCSI with its 15M preferred and maximum unmap and
write zero sizing, or b2f95fe deals with the Linux loopback
block device having a max_transfer of 64k), by allowing blkdebug
to wrap any other device with further restrictions on various
alignments.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170429191419.30051-9-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit 430b26a82da61876c4eaf559ae02332582968043)
* prereq for 81c219a Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Sat, 29 Apr 2017 19:14:17 +0000 (14:14 -0500)]
blkdebug: Simplify override logic
Rather than store into a local variable, then copy to the struct
if the value is valid, then reporting errors otherwise, it is
simpler to just store into the struct and report errors if the
value is invalid. This however requires that the struct store
a 64-bit number, rather than a narrower type. Likewise, setting
a sane errno value in ret prior to the sequence of parsing and
jumping to out: on error makes it easier for the next patch to
add a chain of similar checks.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20170429191419.30051-8-eblake@redhat.com Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit 3dc834f8795a46f919d90b1e5369308628858601)
* prereq for 81c219a Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Sat, 29 Apr 2017 19:14:16 +0000 (14:14 -0500)]
blkdebug: Add pass-through write_zero and discard support
In order to test the effects of artificial geometry constraints
on operations like write zero or discard, we first need blkdebug
to manage these actions. It also allows us to inject errors on
those operations, just like we can for read/write/flush.
We can also test the contract promised by the block layer; namely,
if a device has specified limits on alignment or maximum size,
then those limits must be obeyed (for now, the blkdebug driver
merely inherits limits from whatever it is wrapping, but the next
patch will further enhance it to allow specific limit overrides).
This patch intentionally refuses to service requests smaller than
the requested alignments; this is because an upcoming patch adds
a qemu-iotest to prove that the block layer is correctly handling
fragmentation, but the test only works if there is a way to tell
the difference at artificial alignment boundaries when blkdebug is
using a larger-than-default alignment. If we let the blkdebug
layer always defer to the underlying layer, which potentially has
a smaller granularity, the iotest will be thwarted.
Tested by setting up an NBD server with export 'foo', then invoking:
$ ./qemu-io
qemu-io> open -o driver=blkdebug blkdebug::nbd://localhost:10809/foo
qemu-io> d 0 15M
qemu-io> w -z 0 15M
Pre-patch, the server never sees the discard (it was silently
eaten by the block layer); post-patch it is passed across the
wire. Likewise, pre-patch the write is always passed with
NBD_WRITE (with 15M of zeroes on the wire), while post-patch
it can utilize NBD_WRITE_ZEROES (for less traffic).
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170429191419.30051-7-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit 63188c245013dbe383e8b031e665f813e2452ea5)
* prereq for 81c219a Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Sat, 29 Apr 2017 19:14:15 +0000 (14:14 -0500)]
blkdebug: Refactor error injection
Rather than repeat the logic at each caller of checking if a Rule
exists that warrants an error injection, fold that logic into
inject_error(); and rename it to rule_check() for legibility.
This will help the next patch, which adds two more callers that
need to check rules for the potential of injecting errors.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170429191419.30051-6-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit d157ed5f7235f3d2d5596a514ad7507b18e24b88)
* prereq for 81c219a Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Sat, 29 Apr 2017 19:14:14 +0000 (14:14 -0500)]
blkdebug: Sanity check block layer guarantees
Commits 04ed95f4 and 1a62d0ac updated the block layer to auto-fragment
any I/O to fit within device boundaries. Additionally, when using a
minimum alignment of 4k, we want to ensure the block layer does proper
read-modify-write rather than requesting I/O on a slice of a sector.
Let's enforce that the contract is obeyed when using blkdebug. For
now, blkdebug only allows alignment overrides, and just inherits other
limits from whatever device it is wrapping, but a future patch will
further enhance things.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170429191419.30051-5-eblake@redhat.com Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit e0ef439588ce1ede747f82b77d893190c1cc9f4d)
* prereq for 81c219a Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Yunjian Wang [Wed, 26 Apr 2017 06:45:56 +0000 (14:45 +0800)]
virtio-net: fix wild pointer when remove virtio-net queues
The tx_bh or tx_timer will free in virtio_net_del_queue() function, when
removing virtio-net queues if the guest doesn't support multiqueue. But
it might be still referenced by virtio_net_set_status(), which needs to
be set NULL. And also the tx_waiting needs to be set zero to prevent
virtio_net_set_status() accessing tx_bh or tx_timer.
Cc: qemu-stable@nongnu.org Signed-off-by: Yunjian Wang <wangyunjian@huawei.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
(cherry picked from commit f989c30cf834ba8625e98b808eac30e4e7ec5008) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Halil Pasic [Thu, 18 May 2017 11:14:05 +0000 (13:14 +0200)]
s390x/css: catch section mismatch on load
Prior to the virtio-ccw-2.7 machine (and commit 2a79eb1a), our virtio
devices residing under the virtual-css bus do not have qdev_path based
migration stream identifiers (because their qdev_path is NULL). The ids
are instead generated when the device is registered as a composition of
the so called idstr, which takes the vmsd name as its value, and an
instance_id, which is which is calculated as a maximal instance_id
registered with the same idstr plus one, or zero (if none was registered
previously).
That means, under certain circumstances, one device might try, and even
succeed, to load the state of a different device. This can lead to
trouble.
Let us fail the migration if the above problem is detected during load.
How to reproduce the problem:
1) start qemu-system-s390x making sure you have the following devices
defined on your command line:
-device virtio-rng-ccw,id=rng1,devno=fe.0.0001
-device virtio-rng-ccw,id=rng2,devno=fe.0.0002
2) detach the devices and reattach in reverse order using the monitor:
(qemu) device_del rng1
(qemu) device_del rng2
(qemu) device_add virtio-rng-ccw,id=rng2,devno=fe.0.0002
(qemu) device_add virtio-rng-ccw,id=rng1,devno=fe.0.0001
3) save the state of the vm into a temporary file and quit QEMU:
(qemu) migrate "exec:gzip -c > /tmp/tmp_vmstate.gz"
(qemu) q
4) use your command line from step 1 with
-incoming "exec:gzip -c -d /tmp/tmp_vmstate.gz"
appended to reproduce the problem (while trying to to load the saved vm)
CC: qemu-stable@nongnu.org Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com> Reviewed-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Message-Id: <20170518111405.56947-1-pasic@linux.vnet.ibm.com> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
(cherry picked from commit 8ed179c937830143dc0e03daac30a55272ed89e3)
* removed context dep on d8d98db5 Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Sameeh Jubran [Mon, 22 May 2017 11:26:22 +0000 (14:26 +0300)]
e1000e: Fix ICR "Other" causes clear logic
This commit fixes a bug which causes the guest to hang. The bug was
observed upon a "receive overrun" (bit #6 of the ICR register)
interrupt which could be triggered post migration in a heavy traffic
environment. Even though the "receive overrun" bit (#6) is masked out
by the IMS register (refer to the log below) the driver still receives
an interrupt as the "receive overrun" bit (#6) causes the "Other" -
bit #24 of the ICR register - bit to be set as documented below. The
driver handles the interrupt and clears the "Other" bit (#24) but
doesn't clear the "receive overrun" bit (#6) which leads to an
infinite loop. Apparently the Windows driver expects that the "receive
overrun" bit and other ones - documented below - to be cleared when
the "Other" bit (#24) is cleared.
So to sum that up:
1. Bit #6 of the ICR register is set by heavy traffic
2. As a results of setting bit #6, bit #24 is set
3. The driver receives an interrupt for bit 24 (it doesn't receieve an
interrupt for bit #6 as it is masked out by IMS)
4. The driver handles and clears the interrupt of bit #24
5. Bit #6 is still set.
6. 2 happens all over again
The Interrupt Cause Read - ICR register:
The ICR has the "Other" bit - bit #24 - that is set when one or more
of the following ICR register's bits are set:
LSC - bit #2, RXO - bit #6, MDAC - bit #9, SRPD - bit #16, ACK - bit
#17, MNG - bit #18
This bug can occur with any of these bits depending on the driver's
behaviour and the way it configures the device. However, trying to
reproduce it with any bit other than RX0 is challenging and came to
failure as the drivers don't implement most of these bits, trying to
reproduce it with LSC (Link Status Change - bit #2) bit didn't succeed
too as it seems that Windows handles this bit differently.
* This bug behaviour wasn't observed with the Linux driver.
This commit solves:
https://bugzilla.redhat.com/show_bug.cgi?id=1447935
https://bugzilla.redhat.com/show_bug.cgi?id=1449490
Cc: qemu-stable@nongnu.org Signed-off-by: Sameeh Jubran <sjubran@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
(cherry picked from commit 82342e91b60a4a078811df4e1a545e57abffa11d) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Fam Zheng [Thu, 18 May 2017 10:28:08 +0000 (18:28 +0800)]
virtio-scsi: Unset hotplug handler when unrealize
This matches the qbus_set_hotplug_handler in realize, and it releases
the final reference to the embedded VirtIODevice so that it is
properly finalized.
A use-after-free is fixed with this patch, indirectly:
virtio_device_instance_finalize wasn't called at hot-unplug, and the
vdev->listener would be a dangling pointer in the global and the per
address space listener list. See also RHBZ 1449031.
Cc: qemu-stable@nongnu.org Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20170518102808.30046-1-famz@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
(cherry picked from commit 2cbe2de5454cf9af44b620b2b40d56361a12a45f) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Greg Kurz [Wed, 17 May 2017 08:17:51 +0000 (10:17 +0200)]
virtio: allow broken device to notify guest
According to section 2.1.2 of the virtio-1 specification:
"The device SHOULD set DEVICE_NEEDS_RESET when it enters an error state that
a reset is needed. If DRIVER_OK is set, after it sets DEVICE_NEEDS_RESET,
the device MUST send a device configuration change notification to the
driver."
Commit "f5ed36635d8f virtio: stop virtqueue processing if device is broken"
introduced a virtio_error() call that just does that:
- internally mark the device as broken
- set the DEVICE_NEEDS_RESET bit in the status
- send a configuration change notification
Unfortunately, virtio_notify_vector(), called by virtio_notify_config(),
returns right away when the device is marked as broken and the notification
isn't sent in this case.
The spec doesn't say whether a broken device can send notifications
in other situations or not. But since the driver isn't supposed to do
anything but to reset the device, it makes sense to keep the check in
virtio_notify_config().
Marking the device as broken AFTER the configuration change notification was
sent is enough to fix the issue.
Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
(cherry picked from commit 66453cff9e5e75344c601cd7674c8ef5fefee8a6) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Alberto Garcia [Mon, 15 May 2017 09:34:24 +0000 (12:34 +0300)]
stream: fix crash in stream_start() when block_job_create() fails
The code that tries to reopen a BlockDriverState in stream_start()
when the creation of a new block job fails crashes because it attempts
to dereference a pointer that is known to be NULL.
Paolo Bonzini [Mon, 15 May 2017 10:00:55 +0000 (12:00 +0200)]
curl: avoid recursive locking of BDRVCURLState mutex
The curl driver has a ugly hack where, if it cannot find an empty CURLState,
it just uses aio_poll to wait for one to be empty. This is probably
buggy when used together with dataplane, and the simplest way to fix it
is to use coroutines instead.
A more immediate effect of the bug however is that it can cause a
recursive call to curl_readv_bh_cb and recursively taking the
BDRVCURLState mutex. This causes a deadlock.
The fix is to unlock the mutex around aio_poll, but for cleanliness we
should also take the mutex around all calls to curl_init_state, even if
reaching the unlock/lock pair is impossible. The same is true for
curl_clean_state.
Reported-by: Kun Wei <kuwei@redhat.com> Tested-by: Richard W.M. Jones <rjones@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 20170515100059.15795-4-pbonzini@redhat.com Cc: qemu-stable@nongnu.org Cc: Jeff Cody <jcody@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Jeff Cody <jcody@redhat.com>
(cherry picked from commit 456af346297ebef86aa097b3609534d34f3d2f75) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Paolo Bonzini [Mon, 15 May 2017 10:00:54 +0000 (12:00 +0200)]
curl: never invoke callbacks with s->mutex held
All curl callbacks go through curl_multi_do, and hence are called with
s->mutex held. Note that with comments, and make curl_read_cb drop the
lock before invoking the callback.
Likewise for curl_find_buf, where the callback can be invoked by the
caller.
Cc: qemu-stable@nongnu.org Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170515100059.15795-3-pbonzini@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
(cherry picked from commit 34db05e7ffe8d61ca7288b9532ad6e8300853318) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Paolo Bonzini [Mon, 15 May 2017 10:00:53 +0000 (12:00 +0200)]
curl: strengthen assertion in curl_clean_state
curl_clean_state should only be called after all AIOCBs have been
completed. This is not so obvious for the call from curl_detach_aio_context,
so assert that.
Cc: qemu-stable@nongnu.org Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170515100059.15795-2-pbonzini@redhat.com Signed-off-by: Jeff Cody <jcody@redhat.com>
(cherry picked from commit 675a775633e68bf8b426a896fea5b93a4f4ff1cc) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Filippov [Fri, 12 May 2017 19:05:23 +0000 (12:05 -0700)]
target/xtensa: fix return value of read/write simcalls
Return value of read/write simcalls is not calculated correctly in case
of operations crossing page boundary and in case of short reads/writes.
Read and write simcalls should return the size of data actually
read/written or -1 in case of error.
Cc: qemu-stable@nongnu.org Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
(cherry picked from commit 347ec03093f9668a379ef6b7fa1feb332fff039c) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Filippov [Fri, 12 May 2017 18:17:01 +0000 (11:17 -0700)]
target/xtensa: fix mapping direction in read/write simcalls
Read and write simcalls map physical memory to access I/O buffers, but
'read' simcall need to map it for writing and 'write' simcall need to
map it for reading, i.e. the opposite of what they do now. Fix that.
Cc: qemu-stable@nongnu.org Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
(cherry picked from commit 30c2afd151cbc38c012f7b441088980807183da6) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
John Snow [Wed, 10 May 2017 17:39:45 +0000 (13:39 -0400)]
blockdev: use drained_begin/end for qmp_block_resize
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1447551
If one tries to issue a block_resize while a guest is busy
accessing the disk, it is possible that qemu may deadlock
when invoking aio_poll from both the main loop and the iothread.
Replace another instance of bdrv_drain_all that doesn't
quite belong.
Cc: qemu-stable@nongnu.org Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 698bdfa07d66b5ec218a60229e58eae1dcde00e5) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Reitz [Tue, 28 Mar 2017 20:51:27 +0000 (22:51 +0200)]
block: Add errp to b{lk,drv}_truncate()
For one thing, this allows us to drop the error message generation from
qemu-img.c and blockdev.c and instead have it unified in
bdrv_truncate().
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 20170328205129.15138-3-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit ed3d2ec98a33fbdeabc471b11ff807075f07e996)
* prereq for 698bdfa Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Reitz [Tue, 28 Mar 2017 20:51:26 +0000 (22:51 +0200)]
block/vhdx: Make vhdx_create() always set errp
This patch makes vhdx_create() always set errp in case of an error. It
also adds errp parameters to vhdx_create_bat() and
vhdx_create_new_region_table() so we can pass on the error object
generated by blk_truncate() as of a future commit.
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20170328205129.15138-2-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
(cherry picked from commit 55b9392b98e500399f2da1edc1d110bbfd40fb05)
* prereq for 698bdfa Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Anton Nefedov [Wed, 26 Apr 2017 08:33:15 +0000 (11:33 +0300)]
qemu-img: wait for convert coroutines to complete
On error path (like i/o error in one of the coroutines), it's required to
- wait for coroutines completion before cleaning the common structures
- reenter dependent coroutines so they ever finish
Cc: qemu-stable@nongnu.org Signed-off-by: Anton Nefedov <anton.nefedov@virtuozzo.com> Reviewed-by: Peter Lieven <pl@kamp.de> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit b91127edd0ff96f27f1e58e47f4e9f9d6a0fed02) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Stefan Hajnoczi [Mon, 8 May 2017 18:07:05 +0000 (14:07 -0400)]
aio: add missing aio_notify() to aio_enable_external()
The main loop uses aio_disable_external()/aio_enable_external() to
temporarily disable processing of external AioContext clients like
device emulation.
This allows monitor commands to quiesce I/O and prevent the guest from
submitting new requests while a monitor command is in progress.
The aio_enable_external() API is currently broken when an IOThread is in
aio_poll() waiting for fd activity when the main loop re-enables
external clients. Incrementing ctx->external_disable_cnt does not wake
the IOThread from ppoll(2) so fd processing remains suspended and leads
to unresponsive emulated devices.
This patch adds an aio_notify() call to aio_enable_external() so the
IOThread is kicked out of ppoll(2) and will re-arm the file descriptors.
Zhiyong Yang [Thu, 4 May 2017 16:25:36 +0000 (00:25 +0800)]
hw/virtio: fix vhost user fails to startup when MQ
Qemu2.7~2.9 and vhost user for dpdk 17.02 release work together
to cause failures of new connection when negotiating to set MQ.
(one queue pair works well).
Because there exist some bugs in qemu code when introducing
VHOST_USER_PROTOCOL_F_REPLY_ACK to qemu. When vhost_user_set_mem_table
is invoked to deal with the vhost message VHOST_USER_SET_MEM_TABLE
for the second time, qemu indeed doesn't send the messge (The message
needs to be sent only once)but still will be waiting for dpdk's reply
ack, then, qemu is always freezing, while DPDK is always waiting for
next vhost message from qemu.
The patch aims to fix the bug, MQ can work well.
The same bug is found in function vhost_user_net_set_mtu, it is fixed
at the same time.
DPDK related patch is as following:
http://www.dpdk.org/dev/patchwork/patch/23955/
Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com> Cc: qemu-stable@nongnu.org Fixes: ca525ce5618b ("vhost-user: Introduce a new protocol feature REPLY_ACK.") Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> Tested-by: Jens Freimann <jfreiman@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
(cherry picked from commit 60cd11024f41cc73175e651a2dfe09a3cade56bb) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Fam Zheng [Tue, 2 May 2017 16:35:53 +0000 (00:35 +0800)]
block: Reuse bs as backing hd for drive-backup sync=none
Opening the backing image for the second time is bad, especially here
when it is also in use as the active image as the source. The
drive-backup job itself doesn't read from target->backing for COW,
instead it gets data from the write notifier, so it's not a big problem.
However, exporting the target to NBD etc. won't work, because of the
likely stale metadata cache.
Use BDRV_O_NO_BACKING in this case and manually set up the backing
BdrvChild.
Cc: qemu-stable@nongnu.org Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit fc0932fdcfc3e5cafa3641e361b681c07f639812) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Thu, 27 Apr 2017 21:58:17 +0000 (16:58 -0500)]
qobject: Use simpler QDict/QList scalar insertion macros
We now have macros in place to make it less verbose to add a scalar
to QDict and QList, so use them.
Patch created mechanically via:
spatch --sp-file scripts/coccinelle/qobject.cocci \
--macro-file scripts/cocci-macro-file.h --dir . --in-place
then touched up manually to fix a couple of '?:' back to original
spacing, as well as avoiding a long line in monitor.c.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170427215821.19397-7-eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
(cherry picked from commit 46f5ac205a9dc5e2c24274c7df371509a286281f)
* prereq for fc0932f Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Wed, 5 Apr 2017 19:47:31 +0000 (14:47 -0500)]
s390x: Drop useless casts
An upcoming Coccinelle cleanup script wanted to reformat the casts
present in this file - but on closer look, we don't need the casts
at all because C automatically converts void* to any other pointer.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20170405194741.18956-4-eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
(cherry picked from commit cb55c19a26d6928b445f1bd9394843c9eb60f8ab)
* prereq for 46f5ac2 Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Thu, 27 Apr 2017 21:58:16 +0000 (16:58 -0500)]
qobject: Add helper macros for common scalar insertions
Rather than making lots of callers wrap a scalar in a QInt, QString,
or QBool, provide helper macros that do the wrapping automatically.
Update the Coccinelle script to make mass conversions easy, although
the conversion itself will be done as a separate patches to ease
review and backport efforts.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170427215821.19397-6-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
(cherry picked from commit a92c21591b5bb9543996538f14854ca6b528318b) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Thu, 27 Apr 2017 21:58:15 +0000 (16:58 -0500)]
qobject: Drop useless QObject casts
We have macros in place to make it less verbose to add a subtype
of QObject to both QDict and QList. While we have made cleanups
like this in the past (see commit fcfcd8ffc, for example), having
it be automated by Coccinelle makes it easier to maintain.
Patch created mechanically via:
spatch --sp-file scripts/coccinelle/qobject.cocci \
--macro-file scripts/cocci-macro-file.h --dir . --in-place
then I verified that no manual touchups were required.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Alberto Garcia <berto@igalia.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170427215821.19397-5-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
(cherry picked from commit de6e7951fe66053dfeaac1a237f7aceb9e079619) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Thu, 27 Apr 2017 21:58:14 +0000 (16:58 -0500)]
coccinelle: Add script to remove useless QObject casts
We have macros in place to make it less verbose to add a subtype
of QObject to both QDict and QList. While we have made cleanups
like this in the past (see commit fcfcd8ffc, for example), having
it be automated by Coccinelle makes it easier to maintain.
The script is separate from the cleanups, for ease of review and
backporting. A later patch will then add further possible cleanups.
Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20170427215821.19397-4-eblake@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
(cherry picked from commit a2f3453ebc64b1ebe094aeaf83f9e67896f90ac3) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Greg Kurz [Thu, 25 May 2017 08:30:13 +0000 (10:30 +0200)]
9pfs: local: fix unlink of alien files in mapped-file mode
When trying to remove a file from a directory, both created in non-mapped
mode, the file remains and EBADF is returned to the guest.
This is a regression introduced by commit "df4938a6651b 9pfs: local:
unlinkat: don't follow symlinks" when fixing CVE-2016-9602. It changed the
way we unlink the metadata file from
ret = remove("$dir/.virtfs_metadata/$name");
if (ret < 0 && errno != ENOENT) {
/* Error out */
}
/* Ignore absence of metadata */
to
fd = openat("$dir/.virtfs_metadata")
unlinkat(fd, "$name")
if (ret < 0 && errno != ENOENT) {
/* Error out */
}
/* Ignore absence of metadata */
If $dir was created in non-mapped mode, openat() fails with ENOENT and
we pass -1 to unlinkat(), which fails in turn with EBADF.
We just need to check the return of openat() and ignore ENOENT, in order
to restore the behaviour we had with remove().
Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Eric Blake <eblake@redhat.com>
[groug: rewrote the comments as suggested by Eric]
Bruce Rogers [Thu, 27 Apr 2017 19:59:08 +0000 (13:59 -0600)]
ACPI: don't call acpi_pcihp_device_plug_cb on xen
Commit f0c9d64a exposed the issue that with a xenfv machine using
pci passthrough, acpi pci hotplug code was being executed by mistake.
Guard calls to acpi_pcihp_device_plug_cb (and corresponding
acpi_pcihp_device_unplug_cb) with a check for xen_enabled(). Without
this check I am seeing an error that the bus doesn't have the
acpi-pcihp-bsel property set.
Signed-off-by: Bruce Rogers <brogers@suse.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>
(cherry picked from commit 153eba4726dfa1bdfc31d1fe973b2a61b9035492) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Reitz [Thu, 13 Apr 2017 15:43:34 +0000 (17:43 +0200)]
block: Do not unref bs->file on error in BD's open
The block layer takes care of removing the bs->file child if the block
driver's bdrv_open()/bdrv_file_open() implementation fails. The block
driver therefore does not need to do so, and indeed should not unless it
sets bs->file to NULL afterwards -- because if this is not done, the
bdrv_unref_child() in bdrv_open_inherit() will dereference the freed
memory block at bs->file afterwards, which is not good.
We can now decide whether to add a "bs->file = NULL;" after each of the
offending bdrv_unref_child() invocations, or just drop them altogether.
The latter is simpler, so let's do that.
Cc: qemu-stable <qemu-stable@nongnu.org> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit de234897b60e034ba94b307fc289e2dc692c9251) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
If a pci device is not reset by VM (by writing into config space)
and unplugged by VM, after that when VM reboots, qemu may assert:
pcibus_reset: Assertion `bus->irq_count[i] == 0' failed
Cc: qemu-stable@nongnu.org Signed-off-by: herongguang <herongguang.he@huawei.com> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
(cherry picked from commit 3936161f1fd72d1dfa577aaba910819c5e873260) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
migration: setup bi-directional I/O channel for exec: protocol
Historically the migration data channel has only needed to be
unidirectional. Thus the 'exec:' protocol was requesting an
I/O channel with O_RDONLY on incoming side, and O_WRONLY on
the outgoing side.
This is fine for classic migration, but if you then try to run
TLS over it, this fails because the TLS handshake requires a
bi-directional channel.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Juan Quintela <quintela@redhat.com>
(cherry picked from commit 062d81f0e968fe1597474735f3ea038065027372) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Reitz [Thu, 13 Apr 2017 16:06:25 +0000 (18:06 +0200)]
iotests/051: Add test for empty filename
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 42dc10f17a7f1754d419e715114c37f5c5fde12f) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
This patch fixes this to be:
$ ./qemu-img info ''
qemu-img: Could not open '': The 'file' block driver requires a file
name
Cc: qemu-stable <qemu-stable@nongnu.org> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 4a0082401a770261b85625a41eef4a4e89ad7a74) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Reitz [Thu, 13 Apr 2017 20:34:01 +0000 (22:34 +0200)]
qemu-img/convert: Move bs_n > 1 && -B check down
It does not make much sense to use a backing image for the target when
you concatenate multiple images (because then there is no correspondence
between the source images' backing files and the target's); but it was
still possible to give one by using -o backing_file=X instead of -B X.
Fix this by moving the check.
(Also, change the error message because -B is not the only way to
specify the backing file, evidently.)
Cc: qemu-stable <qemu-stable@nongnu.org> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
* applied patch from v1 of series as suggested by author Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Reitz [Thu, 13 Apr 2017 20:34:00 +0000 (22:34 +0200)]
qemu-img/convert: Use @opts for one thing only
After storing the creation options for the new image into @opts, we
fetch some things for our own information, like the backing file name,
or whether to use encryption or preallocation.
With the -n parameter, there will not be any creation options; this is
not too bad because this just means that querying a NULL @opts will
always return the default value.
However, we also use @opts for the --object options. Therefore, @opts is
not necessarily NULL if -n was specified; instead, it may contain those
options. In practice, this probably does not cause any problems because
there most likely is no object that supports any of the parameters we
query here, but this is neither something we should rely on nor does
this variable reuse make the code very nice to read.
Therefore, just use an own variable for the --object options.
Cc: qemu-stable <qemu-stable@nongnu.org> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com>
* applied patch from v1 of series as suggested by author Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Max Reitz [Thu, 13 Apr 2017 20:33:59 +0000 (22:33 +0200)]
qemu-img/convert: Always set ret < 0 on error
Otherwise the qemu-img process will exit with EXIT_SUCCESS instead of
EXIT_FAILURE.
Cc: qemu-stable <qemu-stable@nongnu.org> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
* applied directly to stable, upstream code has issue fixed via a
refactoring introduced by 9fd77f9, which isn't targetted for stable Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
Eric Blake [Fri, 21 Jul 2017 18:32:42 +0000 (13:32 -0500)]
dirty-bitmap: Report BlockDirtyInfo.count in bytes, as documented
We've been documenting the value in bytes since its introduction
in commit b9a9b3a4 (v1.3), where it was actually reported in bytes.
Commit e4654d2 (v2.0) then removed things from block/qapi.c, in
preparation for a rewrite to a list of dirty sectors in the next
commit 21b5683 in block.c, but the new code mistakenly started
reporting in sectors.
Fixes: https://bugzilla.redhat.com/1441460 CC: qemu-stable@nongnu.org Signed-off-by: Eric Blake <eblake@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 6c98c57af3f4fab85bdf5f01616c91322bd4312a) Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
qga-win: Enable 'can-offline' field in 'guest-get-vcpus' reply
The QGA schema states:
@can-offline: Whether offlining the VCPU is possible. This member
is always filled in by the guest agent when the structure
is returned, and always ignored on input (hence it can be
omitted then).
Currently 'can-offline' is missing entirely from the reply. This causes
errors in libvirt which is expecting the reply to be compliant with the
schema docs.
Peter Maydell [Tue, 18 Apr 2017 15:18:15 +0000 (16:18 +0100)]
Merge remote-tracking branch 'remotes/famz/tags/block-pull-request' into staging
# gpg: Signature made Tue 18 Apr 2017 15:58:32 BST
# gpg: using RSA key 0xCA35624C6A9171C6
# gpg: Good signature from "Fam Zheng <famz@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: 5003 7CB7 9706 0F76 F021 AD56 CA35 624C 6A91 71C6
* remotes/famz/tags/block-pull-request:
block: Drain BH in bdrv_drained_begin
block: Walk bs->children carefully in bdrv_drain_recurse
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
During block job completion, nothing is preventing
block_job_defer_to_main_loop_bh from being called in a nested
aio_poll(), which is a trouble, such as in this code path:
block_job_defer_to_main_loop_bh is the last step of the stream job,
which should have been "paused" by the bdrv_drained_begin/end in
bdrv_reopen_multiple, but it is not done because it's in the form of a
main loop BH.
Similar to why block jobs should be paused between drained_begin and
drained_end, BHs they schedule must be excluded as well. To achieve
this, this patch forces draining the BH in BDRV_POLL_WHILE.
As a side effect this fixes a hang in block_job_detach_aio_context
during system_reset when a block job is ready:
#0 0x0000555555aa79f3 in bdrv_drain_recurse
#1 0x0000555555aa825d in bdrv_drained_begin
#2 0x0000555555aa8449 in bdrv_drain
#3 0x0000555555a9c356 in blk_drain
#4 0x0000555555aa3cfd in mirror_drain
#5 0x0000555555a66e11 in block_job_detach_aio_context
#6 0x0000555555a62f4d in bdrv_detach_aio_context
#7 0x0000555555a63116 in bdrv_set_aio_context
#8 0x0000555555a9d326 in blk_set_aio_context
#9 0x00005555557e38da in virtio_blk_data_plane_stop
#10 0x00005555559f9d5f in virtio_bus_stop_ioeventfd
#11 0x00005555559fa49b in virtio_bus_stop_ioeventfd
#12 0x00005555559f6a18 in virtio_pci_stop_ioeventfd
#13 0x00005555559f6a18 in virtio_pci_reset
#14 0x00005555559139a9 in qdev_reset_one
#15 0x0000555555916738 in qbus_walk_children
#16 0x0000555555913318 in qdev_walk_children
#17 0x0000555555916738 in qbus_walk_children
#18 0x00005555559168ca in qemu_devices_reset
#19 0x000055555581fcbb in pc_machine_reset
#20 0x00005555558a4d96 in qemu_system_reset
#21 0x000055555577157a in main_loop_should_exit
#22 0x000055555577157a in main_loop
#23 0x000055555577157a in main
The rationale is that the loop in block_job_detach_aio_context cannot
make any progress in pausing/completing the job, because bs->in_flight
is 0, so bdrv_drain doesn't process the block_job_defer_to_main_loop
BH. With this patch, it does.
Reported-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20170418143044.12187-3-famz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Tested-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
block: Walk bs->children carefully in bdrv_drain_recurse
The recursive bdrv_drain_recurse may run a block job completion BH that
drops nodes. The coming changes will make that more likely and use-after-free
would happen without this patch
Stash the bs pointer and use bdrv_ref/bdrv_unref in addition to
QLIST_FOREACH_SAFE to prevent such a case from happening.
Since bdrv_unref accesses global state that is not protected by the AioContext
lock, we cannot use bdrv_ref/bdrv_unref unconditionally. Fortunately the
protection is not needed in IOThread because only main loop can modify a graph
with the AioContext lock held.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20170418143044.12187-2-famz@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Tested-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com>
Greg Kurz [Mon, 17 Apr 2017 08:53:23 +0000 (10:53 +0200)]
9pfs: local: set the path of the export root to "."
The local backend was recently converted to using "at*()" syscalls in order
to ensure all accesses happen below the shared directory. This requires that
we only pass relative paths, otherwise the dirfd argument to the "at*()"
syscalls is ignored and the path is treated as an absolute path in the host.
This is actually the case for paths in all fids, with the notable exception
of the root fid, whose path is "/". This causes the following backend ops to
act on the "/" directory of the host instead of the virtfs shared directory
when the export root is involved:
- lstat
- chmod
- chown
- utimensat
ie, chmod /9p_mount_point in the guest will be converted to chmod / in the
host for example. This could cause security issues with a privileged QEMU.
All "*at()" syscalls are being passed an open file descriptor. In the case
of the export root, this file descriptor points to the path in the host that
was passed to -fsdev.
The fix is thus as simple as changing the path of the export root fid to be
"." instead of "/".
This is CVE-2017-7471.
Cc: qemu-stable@nongnu.org Reported-by: Léo Gaspard <leo@gaspard.io> Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Max Reitz [Tue, 11 Apr 2017 14:50:50 +0000 (16:50 +0200)]
block/io: Comment out permission assertions
In case of block migration, there may be writes to BlockBackends that do
not have the write permission taken. Before this issue is fixed (which
is not going to happen in 2.9), we therefore cannot assert that this is
the case.
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 20170411145050.31290-1-mreitz@redhat.com Tested-by: Laurent Vivier <lvivier@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Kevin Wolf [Tue, 11 Apr 2017 14:08:53 +0000 (16:08 +0200)]
sheepdog: Fix crash in co_read_response()
This fixes a regression introduced in commit 9d456654.
aio_co_wake() can only be used to reenter a coroutine that was already
previously entered, otherwise co->ctx is uninitialised and we access
garbage. Using it immediately after qemu_coroutine_create() like in
co_read_response() is wrong and causes segfaults.
Replace the call with aio_co_enter(), which gets an explicit AioContext
parameter and works even for new coroutines.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Kashyap Chamarthy <kchamart@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com>
Message-id: 1491919733-21065-1-git-send-email-kwolf@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Tue, 11 Apr 2017 13:53:32 +0000 (14:53 +0100)]
Merge remote-tracking branch 'remotes/maxreitz/tags/pull-block-2017-04-11' into staging
Block patches for 2.9.0-rc4
# gpg: Signature made Tue 11 Apr 2017 14:40:07 BST
# 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-04-11:
iscsi: Fix iscsi_create
throttle: Remove block from group on hot-unplug
block: pass the right options for BlockDriver.bdrv_open()
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Since d5895fcb (iscsi: Split URL into individual options), creating
qcow2 image on an iscsi LUN fails:
qemu-img create -f qcow2 iscsi://$SERVER/$IQN/0 1G
qemu-img: iscsi://$SERVER/$IQN/0: Could not create image: Invalid
argument
The problem is iscsi_open now expects that transport_name, portal and
target are already parsed into structured options by
iscsi_parse_filename, but it is not called in iscsi_create.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-id: 20170410075451.21329-1-famz@redhat.com Reviewed-by: Eric Blake <eblake@redhat.com>
[mreitz: Dropped now superfluous
qdict_put(bs_options, "filename", ...)] Signed-off-by: Max Reitz <mreitz@redhat.com>
Eric Blake [Thu, 6 Apr 2017 19:08:47 +0000 (14:08 -0500)]
throttle: Remove block from group on hot-unplug
When a block device that is part of a throttle group is hot-unplugged,
we forgot to remove it from the throttle group. This leaves stale
memory around, and causes an easily reproducible crash:
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1428810 Suggested-by: Alberto Garcia <berto@igalia.com> Signed-off-by: Eric Blake <eblake@redhat.com>
Message-id: 20170406190847.29347-1-eblake@redhat.com Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
block: pass the right options for BlockDriver.bdrv_open()
raw_open() expects the caller always passing in the right actual
@options parameter. But when trying to applying snapshot on a RBD
image, bdrv_snapshot_goto() calls raw_open() (by calling the
bdrv_open callback on the BlockDriver) with a NULL @options, and
that will result in a Segmentation fault.
For the other non-raw format drivers, it also makes sense to passing
in the actual options, althought they don't trigger the problem so
far.
Let's prepare a @options by adding the "file" key-value pair to a
copy of the actual options that were given for the node (i.e.
bs->options), and pass it to the callback.
BlockDriver.bdrv_open() expects bs->file to be NULL and just
overwrites it with the result from bdrv_open_child(). That means we
should actually make sure it's NULL because otherwise the child BDS
will have a reference count that is 1 too high. So we unconditionally
invoke bdrv_unref_child() before calling BlockDriver.bdrv_open(), and
we wrap everything in bdrv_ref()/bdrv_unref() so the BDS isn't
deleted in the meantime.
Suggested-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Dong Jia Shi <bjsdjshi@linux.vnet.ibm.com>
Message-id: 20170405091909.36357-2-bjsdjshi@linux.vnet.ibm.com Signed-off-by: Max Reitz <mreitz@redhat.com>
Peter Maydell [Tue, 11 Apr 2017 12:27:05 +0000 (13:27 +0100)]
Merge remote-tracking branch 'remotes/famz/tags/block-pull-request' into staging
# gpg: Signature made Tue 11 Apr 2017 13:10:55 BST
# gpg: using RSA key 0xCA35624C6A9171C6
# gpg: Good signature from "Fam Zheng <famz@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: 5003 7CB7 9706 0F76 F021 AD56 CA35 624C 6A91 71C6
* remotes/famz/tags/block-pull-request:
sheepdog: Use bdrv_coroutine_enter before BDRV_POLL_WHILE
block: Fix bdrv_co_flush early return
block: Use bdrv_coroutine_enter to start I/O coroutines
qemu-io-cmds: Use bdrv_coroutine_enter
blockjob: Use bdrv_coroutine_enter to start coroutine
block: Introduce bdrv_coroutine_enter
async: Introduce aio_co_enter
coroutine: Extract qemu_aio_coroutine_enter
tests/block-job-txn: Don't start block job before adding to txn
block: Quiesce old aio context during bdrv_set_aio_context
block: Make bdrv_parent_drained_begin/end public
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
bdrv_inc_in_flight and bdrv_dec_in_flight are mandatory for
BDRV_POLL_WHILE to work, even for the shortcut case where flush is
unnecessary. Move the if block to below bdrv_dec_in_flight, and BTW fix
the variable declaration position.
Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
block: Use bdrv_coroutine_enter to start I/O coroutines
BDRV_POLL_WHILE waits for the started I/O by releasing bs's ctx then polling
the main context, which relies on the yielded coroutine continuing on bs->ctx
before notifying qemu_aio_context with bdrv_wakeup().
Thus, using qemu_coroutine_enter to start I/O is wrong because if the coroutine
is entered from main loop, co->ctx will be qemu_aio_context, as a result of the
"release, poll, acquire" loop of BDRV_POLL_WHILE, race conditions happen when
both main thread and the iothread access the same BDS:
Note that in above case, bdrv_drained_begin() doesn't do the "release,
poll, acquire" in BDRV_POLL_WHILE, because bs->in_flight == 0.
Fix this by using bdrv_coroutine_enter and enter coroutine in the right
context.
iotests 109 output is updated because the coroutine reenter flow during
mirror job complete is different (now through co_queue_wakeup, instead
of the unconditional qemu_coroutine_switch before), making the end job
len different.
Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
blockjob: Use bdrv_coroutine_enter to start coroutine
Resuming and especially starting of the block job coroutine, could be issued in
the main thread. However the coroutine's "home" ctx should be set to the same
context as job->blk. Use bdrv_coroutine_enter to ensure that.
Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
tests/block-job-txn: Don't start block job before adding to txn
Previously, before test_block_job_start returns, the job can already
complete, as a result, the transactional state of other jobs added to
the same txn later cannot be handled correctly.
Move the block_job_start() calls to callers after
block_job_txn_add_job() calls.
Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
block: Quiesce old aio context during bdrv_set_aio_context
The fact that the bs->aio_context is changing can confuse the dataplane
iothread, because of the now fine granularity aio context lock.
bdrv_drain should rather be a bdrv_drained_begin/end pair, but since
bs->aio_context is changing, we can just use aio_disable_external and
bdrv_parent_drained_begin.
Reported-by: Ed Swierk <eswierk@skyportsystems.com> Signed-off-by: Fam Zheng <famz@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Acked-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Peter Maydell [Mon, 10 Apr 2017 15:08:37 +0000 (16:08 +0100)]
Merge remote-tracking branch 'remotes/gkurz/tags/for-upstream' into staging
Fixes a memory leak.
# gpg: Signature made Mon 10 Apr 2017 13:20:39 BST
# gpg: using DSA key 0x02FC3AEB0101DBC2
# gpg: Good signature from "Greg Kurz <groug@kaod.org>"
# gpg: aka "Greg Kurz <groug@free.fr>"
# gpg: aka "Greg Kurz <gkurz@linux.vnet.ibm.com>"
# gpg: aka "Gregory Kurz (Groug) <groug@free.fr>"
# gpg: aka "[jpeg image of size 3330]"
# 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: 2BD4 3B44 535E C0A7 9894 DBA2 02FC 3AEB 0101 DBC2
* remotes/gkurz/tags/for-upstream:
9pfs: xattr: fix memory leak in v9fs_list_xattr
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Mon, 10 Apr 2017 14:01:15 +0000 (15:01 +0100)]
Merge remote-tracking branch 'remotes/stsquad/tags/pull-mttcg-fixups-for-rc2-100417-1' into staging
Final icount and misc MTTCG fixes for 2.9
Minor differences from:
Message-Id: <20170405132503.32125-1-alex.bennee@linaro.org>
- dropped new feature patches
- last minute typo fix from Nikunj A Dadhania <nikunj@linux.vnet.ibm.com>
# gpg: Signature made Mon 10 Apr 2017 11:38:10 BST
# gpg: using RSA key 0xFBD0DB095A9E2A44
# gpg: Good signature from "Alex Bennée (Master Work Key) <alex.bennee@linaro.org>"
# Primary key fingerprint: 6685 AE99 E751 67BC AFC8 DF35 FBD0 DB09 5A9E 2A44
* remotes/stsquad/tags/pull-mttcg-fixups-for-rc2-100417-1:
replay: assert time only goes forward
cpus: call cpu_update_icount on read
cpu-exec: update icount after each TB_EXIT
cpus: introduce cpu_update_icount helper
cpus: don't credit executed instructions before they have run
cpus: move icount preparation out of tcg_exec_cpu
cpus: check cpu->running in cpu_get_icount_raw()
cpus: remove icount handling from qemu_tcg_cpu_thread_fn
target/i386/misc_helper: wrap BQL around another IRQ generator
cpus: fix wrong define name
scripts/qemugdb/mtree.py: fix up mtree dump
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Mon, 3 Apr 2017 13:04:15 +0000 (14:04 +0100)]
configure: on Windows minimum glib version must be 2.30
In the 2.7 release we stated in the ChangeLog that the
minimum glib version for Windows hosts was 2.30, but we
didn't update configure to enforce this because we were
very close to the release at the point where we noticed
the issue, and it only affected building the test suite.
We then forgot that we needed to do it. Fix the omission.
(The reason for the 2.30 requirement is use of
g_dir_make_tmp() -- our fallback implementation uses
mkdtemp(), which isn't available on Windows.)
Reported-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Stefan Weil <sw@weilnetz.de>
Message-id: 1491224655-5776-1-git-send-email-peter.maydell@linaro.org
Alex Bennée [Wed, 5 Apr 2017 10:05:28 +0000 (11:05 +0100)]
replay: assert time only goes forward
If we find ourselves trying to add an event to the log where time has
gone backwards it is because a vCPU event has occurred and the
main-loop is not yet aware of time moving forward. This should not
happen and if it does its better to fail early than generate a log
that will have weird behaviour.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Alex Bennée [Wed, 5 Apr 2017 09:53:47 +0000 (10:53 +0100)]
cpus: call cpu_update_icount on read
This ensures each time the vCPU thread reads the icount we update the
master timer_state.qemu_icount field. This way as long as updates are
in BQL protected sections (which they should be) the main-loop can
never come to update the log and find time has gone backwards.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Alex Bennée [Wed, 5 Apr 2017 11:35:48 +0000 (12:35 +0100)]
cpu-exec: update icount after each TB_EXIT
There is no particular reason we shouldn't update the global system
icount time as we exit each TranslationBlock run. This ensures the
main-loop doesn't have to wait until we exit to the outer loop for
executed instructions to be credited to timer_state.
The prepare_icount_for_run function is slightly tweaked to match the
logic we run in cpu_loop_exec_tb.
Based on Paolo's original suggestion.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Alex Bennée [Wed, 5 Apr 2017 11:32:37 +0000 (12:32 +0100)]
cpus: introduce cpu_update_icount helper
By holding off updates to timer_state.qemu_icount we can run into
trouble when the non-vCPU thread needs to know the time. This helper
ensures we atomically update timers_state.qemu_icount based on what
has been currently executed.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Alex Bennée [Fri, 31 Mar 2017 15:09:42 +0000 (16:09 +0100)]
cpus: don't credit executed instructions before they have run
Outside of the vCPU thread icount time will only be tracked against
timers_state.qemu_icount. We no longer credit cycles until they have
completed the run. Inside the vCPU thread we adjust for passage of
time by looking at how many have run so far. This is only valid inside
the vCPU thread while it is running.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Alex Bennée [Thu, 30 Mar 2017 17:49:22 +0000 (18:49 +0100)]
cpus: check cpu->running in cpu_get_icount_raw()
The lifetime of current_cpu is now the lifetime of the vCPU thread.
However get_icount_raw() can apply a fudge factor if called while code
is running to take into account the current executed instruction
count.
To ensure this is always the case we also check cpu->running.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <rth@twiddle.net>
Alex Bennée [Thu, 30 Mar 2017 17:32:29 +0000 (18:32 +0100)]
cpus: remove icount handling from qemu_tcg_cpu_thread_fn
We should never be running in multi-threaded mode with icount enabled.
There is no point calling handle_icount_deadline here so remove it and
assert !use_icount.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <rth@twiddle.net>
Alex Bennée [Tue, 21 Mar 2017 16:26:27 +0000 (16:26 +0000)]
scripts/qemugdb/mtree.py: fix up mtree dump
Since QEMU has been able to build with native Int128 support this was
broken as it attempts to fish values out of the non-existent
structure. Also the alias print was trying to make a %x out of
gdb.ValueType directly which didn't seem to work.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Peter Maydell [Fri, 7 Apr 2017 14:23:48 +0000 (15:23 +0100)]
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer fixes for 2.9.0-rc4
# gpg: Signature made Fri 07 Apr 2017 13:44:17 BST
# gpg: using RSA key 0x7F09B272C88F2FD6
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>"
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream:
mirror: Fix aio context of mirror_top_bs
block: Assert attached child node has right aio context
block: Fix unpaired aio_disable_external in external snapshot
block: Don't check permissions for copy on read
qemu-img: img_create does not support image-opts, fix docs
iotests: Add mirror tests for orphaned source
block/mirror: Fix use-after-free
commit: Set commit_top_bs->total_sectors
commit: Set commit_top_bs->aio_context
block: Ignore guest dev permissions during incoming migration
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
It should be moved to the same context as source, before inserting to the
graph.
Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block: Fix unpaired aio_disable_external in external snapshot
bdrv_replace_child_noperm tries to hand over the quiesce_counter state
from old bs to the new one, but if they are not on the same aio context
this causes unbalance.
Fix this by setting the correct aio context before calling
bdrv_append().
Reported-by: Ed Swierk <eswierk@skyportsystems.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Fam Zheng <famz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Kevin Wolf [Fri, 7 Apr 2017 10:29:05 +0000 (12:29 +0200)]
block: Don't check permissions for copy on read
The assertion is currently failing. We can't require callers to have
write permissions when all they are doing is a read, so comment it out.
Add a FIXME comment in the code so that the check is re-enabled when
copy on read is refactored into its own filter driver.
Reported-by: Richard W.M. Jones <rjones@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Richard W.M. Jones <rjones@redhat.com>
Jeff Cody [Thu, 6 Apr 2017 17:45:42 +0000 (13:45 -0400)]
qemu-img: img_create does not support image-opts, fix docs
The documentation and help for qemu-img claims that 'qemu-img create'
will take the '--image-opts' argument. This is not true, so this
patch removes those claims.
Signed-off-by: Jeff Cody <jcody@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Max Reitz [Mon, 3 Apr 2017 17:51:50 +0000 (19:51 +0200)]
iotests: Add mirror tests for orphaned source
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Max Reitz [Mon, 3 Apr 2017 17:51:49 +0000 (19:51 +0200)]
block/mirror: Fix use-after-free
If @bs does not have any parents, the only reference to @mirror_top_bs
will be held by the BlockJob object after the bdrv_unref() following
block_job_create(). However, if block_job_create() fails, this reference
will not exist and @mirror_top_bs will have been deleted when we
goto fail.
The issue comes back at all later entries to the fail label: We delete
the BlockJob object before rolling back our changes to the node graph.
This means that we will delete @mirror_top_bs in the process.
All in all, whenever @bs does not have any parents and we go down the
fail path we will dereference @mirror_top_bs after it has been deleted.
Fix this by invoking bdrv_unref() only when block_job_create() was
successful and by bdrv_ref()'ing @mirror_top_bs in the fail path before
deleting the BlockJob object. Finally, bdrv_unref() it at the end of the
fail path after we actually no longer need it.
Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: John Snow <jsnow@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Kevin Wolf [Thu, 6 Apr 2017 17:07:14 +0000 (19:07 +0200)]
commit: Set commit_top_bs->total_sectors
Like in the mirror filter driver, we also need to set the image size for
the commit filter driver. This is less likely to be a problem in
practice than for the mirror because we're not at the active layer here,
but attaching new parents to a node in the middle of the chain is
possible, so the size needs to be correct anyway.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Fam Zheng <famz@redhat.com>
Kevin Wolf [Tue, 4 Apr 2017 15:29:03 +0000 (17:29 +0200)]
block: Ignore guest dev permissions during incoming migration
Usually guest devices don't like other writers to the same image, so
they use blk_set_perm() to prevent this from happening. In the migration
phase before the VM is actually running, though, they don't have a
problem with writes to the image. On the other hand, storage migration
needs to be able to write to the image in this phase, so the restrictive
blk_set_perm() call of qdev devices breaks it.
This patch flags all BlockBackends with a qdev device as
blk->disable_perm during incoming migration, which means that the
requested permissions are stored in the BlockBackend, but not actually
applied to its root node yet.
Once migration has finished and the VM should be resumed, the
permissions are applied. If they cannot be applied (e.g. because the NBD
server used for block migration hasn't been shut down), resuming the VM
fails.
Signed-off-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Kashyap Chamarthy <kchamart@redhat.com>
projects / mirror_qemu.git / log