Re: [Qemu-block] [PATCH] block: all I/O should be completed before removing throttle timers.

2017-11-10 Thread l00284672
ok,thanks ! On 2017/11/10 23:33, Stefan Hajnoczi wrote: On Sat, Oct 21, 2017 at 01:34:00PM +0800, Zhengui Li wrote: From: Zhengui 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/

Re: [Qemu-block] [PATCH v2 for-2.11] block: Make bdrv_next() keep strong references

2017-11-10 Thread Paolo Bonzini
On 10/11/2017 18:25, Max Reitz wrote: > if (bs) { > +bdrv_ref(bs); > +bdrv_unref(old_bs); > return bs; > } Maybe instead goto... > it->phase = BDRV_NEXT_MONITOR_OWNED; > +} else { > +old_bs = it->bs; > } > > /

Re: [Qemu-block] [PATCH v2 0/1] Add 8-byte wide AMD flash support, partial interleaving

2017-11-10 Thread Paolo Bonzini
On 10/11/2017 21:25, Mike Nawrocki wrote: > This patch set does a few things. First, it switches the AMD CFI flash MMIO > operations from the old MMIO API to the new one. Second, it enables 8-byte > wide > flash arrays. Finally, it adds flash interleaving using the "device-width" and > "max-device

Re: [Qemu-block] [Qemu-devel] [PATCH v8 02/14] block/dirty-bitmap: add locked version of bdrv_release_dirty_bitmap

2017-11-10 Thread John Snow
On 10/30/2017 12:32 PM, Vladimir Sementsov-Ogievskiy wrote: > It is needed to realize bdrv_dirty_bitmap_release_successor in > the following patch. > OK, but... > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > block/dirty-bitmap.c | 25 - > 1 file changed, 20 ins

[Qemu-block] [PATCH for-2.12 1/4] iotests: Make BD-{remove, insert}-medium use @id

2017-11-10 Thread Max Reitz
In some cases, these commands still use the deprecated @device parameter. Fix that so we can later drop that parameter from their interface. Signed-off-by: Max Reitz --- tests/qemu-iotests/118 | 184 +++-- tests/qemu-iotests/155 | 60

[Qemu-block] [PATCH for-2.12 4/4] blockdev: Mark BD-{remove, insert}-medium stable

2017-11-10 Thread Max Reitz
Now that iotest 093 test proves that the throttling configuration survives a blockdev-remove-medium/blockdev-insert-medium pair, the original reason for declaring these commands experimental is gone (see commit 6e0abc251dd4f8eba1f53656dfede12e5840e83b). Signed-off-by: Max Reitz --- qapi/block-co

[Qemu-block] [PATCH for-2.12 3/4] blockdev: Drop BD-{remove, insert}-medium's @device

2017-11-10 Thread Max Reitz
This is an incompatible change, which is fine as the commands are experimental. Signed-off-by: Max Reitz --- qapi/block-core.json | 10 ++ blockdev.c | 30 +++--- 2 files changed, 25 insertions(+), 15 deletions(-) diff --git a/qapi/block-core.json b/qap

[Qemu-block] [PATCH for-2.12 2/4] tests/ahci: Switch tray and medium commands to @id

2017-11-10 Thread Max Reitz
Currently, the tray and medium commands in the AHCI test use the deprecated @device parameter. This patch switches all invocations over to use @id. Signed-off-by: Max Reitz --- tests/ahci-test.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ahci-test.c b

[Qemu-block] [PATCH for-2.12 0/4] blockdev: Mark BD-{remove, insert}-medium stable

2017-11-10 Thread Max Reitz
Berto's "Test I/O limits with removable media" patch proves that throttling survives a blockdev-remove-medium/blockdev-insert-medium pair now, so let's mark them stable (because that was the reason they were considered experimental, see commit 6e0abc251dd4f8eba1f53656dfede12e5840e83b for more). Bu

Re: [Qemu-block] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

2017-11-10 Thread Alberto Garcia
On Fri 10 Nov 2017 11:08:20 PM CET, Max Reitz wrote: >> I just noticed a typo in the commit message: >> >>> There'a a couple of problems with this: >> >> "There's a couple" >> >> If there's no v2 of this series you can correct this when committing. > > Well, the issue is that it's going to be i

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 2/3] block: Handle null backing link

2017-11-10 Thread Max Reitz
On 2017-11-10 23:22, Eric Blake wrote: > On 11/10/2017 04:13 PM, Max Reitz wrote: >> Instead of converting all "backing": null instances into "backing": "", >> handle a null value directly in bdrv_open_inherit(). >> >> This enables explicitly null backing links for json:{} filenames. >> >> Signed-o

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 2/3] block: Handle null backing link

2017-11-10 Thread Eric Blake
On 11/10/2017 04:13 PM, Max Reitz wrote: > Instead of converting all "backing": null instances into "backing": "", > handle a null value directly in bdrv_open_inherit(). > > This enables explicitly null backing links for json:{} filenames. > > Signed-off-by: Max Reitz > --- > block.c

Re: [Qemu-block] [PATCH 3/3] qemu-iotests: Test I/O limits with removable media

2017-11-10 Thread Max Reitz
On 2017-11-10 19:54, Alberto Garcia wrote: > 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 medi

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 3/3] block: Deprecate "backing": ""

2017-11-10 Thread Eric Blake
On 11/10/2017 04:13 PM, Max Reitz wrote: > We have a clear replacement, so let's deprecate it. > > Signed-off-by: Max Reitz > --- > qapi/block-core.json | 4 ++-- > block.c | 4 > qemu-doc.texi| 7 +++ > qemu-options.hx | 4 ++-- > 4 files changed, 15 insertion

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()

2017-11-10 Thread Eric Blake
On 11/10/2017 04:13 PM, Max Reitz wrote: > Signed-off-by: Max Reitz > --- > include/qapi/qmp/qdict.h | 1 + > qobject/qdict.c | 10 ++ > 2 files changed, 11 insertions(+) > Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301

[Qemu-block] [PATCH for-2.12 3/3] block: Deprecate "backing": ""

2017-11-10 Thread Max Reitz
We have a clear replacement, so let's deprecate it. Signed-off-by: Max Reitz --- qapi/block-core.json | 4 ++-- block.c | 4 qemu-doc.texi| 7 +++ qemu-options.hx | 4 ++-- 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/qapi/block-core.json b/q

[Qemu-block] [PATCH for-2.12 2/3] block: Handle null backing link

2017-11-10 Thread Max Reitz
Instead of converting all "backing": null instances into "backing": "", handle a null value directly in bdrv_open_inherit(). This enables explicitly null backing links for json:{} filenames. Signed-off-by: Max Reitz --- block.c| 2 +- blockdev.c | 14 ---

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

2017-11-10 Thread Max Reitz
On 2017-11-10 23:15, Eric Blake wrote: > On 11/10/2017 04:00 PM, Max Reitz wrote: >>> Trying to understand this: we have a double corruption, because we >>> encountered a refblock that points outside of the image, but fixing the >>> refblock in turn encounters a second refblock that points within t

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

2017-11-10 Thread Eric Blake
On 11/10/2017 04:00 PM, Max Reitz wrote: >> Trying to understand this: we have a double corruption, because we >> encountered a refblock that points outside of the image, but fixing the >> refblock in turn encounters a second refblock that points within the >> image but to an unaligned area. > > N

[Qemu-block] [PATCH for-2.12 0/3] block: Handle null backing link

2017-11-10 Thread Max Reitz
Currently, we try to rewrite every occurrence of "backing": null into "backing": "" in qmp_blockdev_add(). However, that breaks using the same "backing": null construction in json:{} file names (which do not go through qmp_blockdev_add()). Currently, these then just behave as if the option has no

[Qemu-block] [PATCH for-2.12 1/3] qapi: Add qdict_is_null()

2017-11-10 Thread Max Reitz
Signed-off-by: Max Reitz --- include/qapi/qmp/qdict.h | 1 + qobject/qdict.c | 10 ++ 2 files changed, 11 insertions(+) diff --git a/include/qapi/qmp/qdict.h b/include/qapi/qmp/qdict.h index fc218e7be6..c65ebfc748 100644 --- a/include/qapi/qmp/qdict.h +++ b/include/qapi/qmp/qdi

Re: [Qemu-block] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

2017-11-10 Thread Max Reitz
On 2017-11-10 23:06, Alberto Garcia wrote: > On Fri 10 Nov 2017 07:54:47 PM CET, Alberto Garcia wrote: > > I just noticed a typo in the commit message: > >> There'a a couple of problems with this: > > "There's a couple" > > If there's no v2 of this series you can correct this when committing.

Re: [Qemu-block] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

2017-11-10 Thread Alberto Garcia
On Fri 10 Nov 2017 07:54:47 PM CET, Alberto Garcia wrote: I just noticed a typo in the commit message: > There'a a couple of problems with this: "There's a couple" If there's no v2 of this series you can correct this when committing. Berto

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

2017-11-10 Thread Max Reitz
On 2017-11-10 22:54, Eric Blake wrote: > On 11/10/2017 02:31 PM, Max Reitz wrote: >> Instead of using an assertion, it is better to emit a corruption event >> here. Checking all offsets for correct alignment can be tedious and it >> is easily possible to forget to do so. qcow2_cache_do_get() is a

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

2017-11-10 Thread Eric Blake
On 11/10/2017 02:31 PM, Max Reitz wrote: > Instead of using an assertion, it is better to emit a corruption event > here. Checking all offsets for correct alignment can be tedious and it > is easily possible to forget to do so. qcow2_cache_do_get() is a > function every L2 and refblock access has

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 4/5] qcow2: Add bounds check to get_refblock_offset()

2017-11-10 Thread Eric Blake
On 11/10/2017 02:31 PM, Max Reitz wrote: > Reported-by: R. Nageswara Sastry > Buglink: https://bugs.launchpad.net/qemu/+bug/1728661 > Signed-off-by: Max Reitz > --- > block/qcow2.h | 6 -- > block/qcow2-refcount.c | 26 +- > tests/qemu-iotests/060

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 3/5] block: Guard against NULL bs->drv

2017-11-10 Thread Eric Blake
On 11/10/2017 02:31 PM, Max Reitz wrote: > We currently do not guard everywhere against a NULL bs->drv where we > should be doing so. Most of the places fixed here just do not care > about that case at all. > > Some care implicitly, e.g. through a prior function call to > bdrv_getlength() which w

Re: [Qemu-block] [PATCH] iotests: Add test for failing qemu-img commit

2017-11-10 Thread Max Reitz
On 2017-06-16 15:58, Max Reitz wrote: > Signed-off-by: Max Reitz > --- > In order to pass, this depends on "fix: avoid an infinite loop or a > dangling pointer problem in img_commit" > (http://lists.nongnu.org/archive/html/qemu-block/2017-06/msg00443.html) > and on the "block: Don't compare string

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 2/5] qcow2: Unaligned zero cluster in handle_alloc()

2017-11-10 Thread Eric Blake
On 11/10/2017 02:31 PM, Max Reitz wrote: > We should check whether the cluster offset we are about to use is > actually valid; that is, whether it is aligned to cluster boundaries. > > Reported-by: R. Nageswara Sastry > Buglink: https://bugs.launchpad.net/qemu/+bug/1728643 > Buglink: https://bugs

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11 1/5] qcow2: check_errors are fatal

2017-11-10 Thread Eric Blake
On 11/10/2017 02:31 PM, Max Reitz wrote: > When trying to repair a dirty image, qcow2_check() may apparently > succeed (no really fatal error occurred that would prevent the check > from continuing), but if check_errors in the result object is non-zero, > we cannot trust the image to be usable. >

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.12 1/1] qcow2: Repair unaligned preallocated zero clusters

2017-11-10 Thread Eric Blake
On 11/10/2017 02:37 PM, Max Reitz wrote: > We can easily repair unaligned preallocated zero clusters by discarding > them, so why not do it? > > Signed-off-by: Max Reitz > --- > block/qcow2-refcount.c | 70 > ++ > tests/qemu-iotests/060 | 3 +

Re: [Qemu-block] [PATCH for-2.11 0/5] qcow2: Fixes for corrupted images

2017-11-10 Thread Max Reitz
On 2017-11-10 21:31, Max Reitz wrote: > This series contains fixes for another batch of qcow2-related crashes > reported on Launchpad by Nageswara (the first batch was > http://lists.nongnu.org/archive/html/qemu-block/2017-11/msg00082.html by > Berto). > > Patch 4 fixes an out-of-bounds array acce

[Qemu-block] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache

2017-11-10 Thread Max Reitz
Instead of using an assertion, it is better to emit a corruption event here. Checking all offsets for correct alignment can be tedious and it is easily possible to forget to do so. qcow2_cache_do_get() is a function every L2 and refblock access has to go through, so this is a good central point t

[Qemu-block] [PATCH for-2.12 0/1] qcow2: Repair unaligned preallocated zero clusters

2017-11-10 Thread Max Reitz
This is a follow-up to patch 2 of my "qcow2: Unaligned zero cluster in handle_alloc()" series. That patch adds a way to correctly deal with such clusters, this patch here adds a way to repair them. Naturally, this patch is therefore based on that series: Based-on: <20171110203111.7666-1-mre...@r

[Qemu-block] [PATCH for-2.12 1/1] qcow2: Repair unaligned preallocated zero clusters

2017-11-10 Thread Max Reitz
We can easily repair unaligned preallocated zero clusters by discarding them, so why not do it? Signed-off-by: Max Reitz --- block/qcow2-refcount.c | 70 ++ tests/qemu-iotests/060 | 3 +- tests/qemu-iotests/060.out | 9 ++ 3 files changed

[Qemu-block] [PATCH for-2.11 4/5] qcow2: Add bounds check to get_refblock_offset()

2017-11-10 Thread Max Reitz
Reported-by: R. Nageswara Sastry Buglink: https://bugs.launchpad.net/qemu/+bug/1728661 Signed-off-by: Max Reitz --- block/qcow2.h | 6 -- block/qcow2-refcount.c | 26 +- tests/qemu-iotests/060 | 46 ++

Re: [Qemu-block] [PATCH 3/3] qemu-iotests: Test I/O limits with removable media

2017-11-10 Thread Max Reitz
On 2017-11-10 19:54, Alberto Garcia wrote: > 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 medi

[Qemu-block] [PATCH for-2.11 2/5] qcow2: Unaligned zero cluster in handle_alloc()

2017-11-10 Thread Max Reitz
We should check whether the cluster offset we are about to use is actually valid; that is, whether it is aligned to cluster boundaries. Reported-by: R. Nageswara Sastry Buglink: https://bugs.launchpad.net/qemu/+bug/1728643 Buglink: https://bugs.launchpad.net/qemu/+bug/1728657 Signed-off-by: Max R

[Qemu-block] [PATCH for-2.11 0/5] qcow2: Fixes for corrupted images

2017-11-10 Thread Max Reitz
This series contains fixes for another batch of qcow2-related crashes reported on Launchpad by Nageswara (the first batch was http://lists.nongnu.org/archive/html/qemu-block/2017-11/msg00082.html by Berto). Patch 4 fixes an out-of-bounds array access in memory which is not really a security issue

[Qemu-block] [PATCH for-2.11 1/5] qcow2: check_errors are fatal

2017-11-10 Thread Max Reitz
When trying to repair a dirty image, qcow2_check() may apparently succeed (no really fatal error occurred that would prevent the check from continuing), but if check_errors in the result object is non-zero, we cannot trust the image to be usable. Reported-by: R. Nageswara Sastry Buglink: https://

[Qemu-block] [PATCH for-2.11 3/5] block: Guard against NULL bs->drv

2017-11-10 Thread Max Reitz
We currently do not guard everywhere against a NULL bs->drv where we should be doing so. Most of the places fixed here just do not care about that case at all. Some care implicitly, e.g. through a prior function call to bdrv_getlength() which would always fail for an ejected BDS. Add an assert t

Re: [Qemu-block] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

2017-11-10 Thread Max Reitz
On 2017-11-10 19:54, Alberto Garcia wrote: > 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

[Qemu-block] [PATCH v2 1/1] Add 8-byte access, interleaving to AMD CFI devices

2017-11-10 Thread Mike Nawrocki
This adds 8-byte wide access support to AMD CFI flash devices. Additionally, it migrates the MMIO operations from old_mmio to the new API. Finally, it mirrors the interleaving support already in place in pflash_cfi01.c, using the max_device_width and device_width properties. Signed-off-by: Mike Na

[Qemu-block] [PATCH v2 0/1] Add 8-byte wide AMD flash support, partial interleaving

2017-11-10 Thread Mike Nawrocki
This patch set does a few things. First, it switches the AMD CFI flash MMIO operations from the old MMIO API to the new one. Second, it enables 8-byte wide flash arrays. Finally, it adds flash interleaving using the "device-width" and "max-device-width" properties, using the same interface as pflas

Re: [Qemu-block] [PATCH 1/3] block: Check for inserted BlockDriverState in blk_io_limits_disable()

2017-11-10 Thread Max Reitz
On 2017-11-10 19:54, Alberto Garcia wrote: > 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 b

Re: [Qemu-block] [PATCH] iotests: test clearing unknown autoclear_features by qcow2

2017-11-10 Thread Kevin Wolf
Am 10.11.2017 um 18:54 hat Vladimir Sementsov-Ogievskiy geschrieben: > Test clearing unknown autoclear_features by qcow2 on incoming > migration. > > Signed-off-by: Vladimir Sementsov-Ogievskiy > --- > > Hi all! > > This patch shows degradation, added in 2.10 in commit > > commit 9c5e6594f15b7

[Qemu-block] [PATCH 1/3] block: Check for inserted BlockDriverState in blk_io_limits_disable()

2017-11-10 Thread Alberto Garcia
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

[Qemu-block] [PATCH 3/3] qemu-iotests: Test I/O limits with removable media

2017-11-10 Thread Alberto Garcia
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 li

[Qemu-block] [PATCH 0/3] Fix throttling crashes in BlockBackend with no BlockDriverState

2017-11-10 Thread Alberto Garcia
Hi, this series fixes the problems reported by Sochin Jiang in BlockBackend when there's a valid throttling configuration but the BDS has been removed. The patches apply on top of Li Zhengui's "all I/O should be completed before removing throttle timers" and I tested this on top of Stefan's block

[Qemu-block] [PATCH 2/3] block: Leave valid throttle timers when removing a BDS from a backend

2017-11-10 Thread Alberto Garcia
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

Re: [Qemu-block] [Qemu-devel] [PATCH v2 4/5] iotests: Make 083 less flaky

2017-11-10 Thread Eric Blake
On 11/10/2017 11:29 AM, Max Reitz wrote: It seems that in this patch you're indenting with spaces but this file uses tabs. >>> >>> Yes, but tabs are wrong. :-) >> >> I actually agree with you, but don't mix them in the file :-) > > I can whistle and say here, too, that Eric liked it. O:

Re: [Qemu-block] [PATCH 0/1] qcow2: Check that corrupted images can be repaired in iotest 060

2017-11-10 Thread Max Reitz
On 2017-11-08 13:13, Alberto Garcia wrote: > Hi, > > I sent the 'Misc qcow2 corruption checks' series the other day, and > Kevin suggested that we check that the corrupted images can be > repaired using qemu-img. > > This patch extends the tests that I wrote in order to do just > that. Since the

Re: [Qemu-block] [Qemu-devel] [PATCH v6 0/6] block: Don't compare strings in bdrv_reopen_prepare()

2017-11-10 Thread Max Reitz
On 2017-11-10 18:47, Kevin Wolf wrote: > Am 10.11.2017 um 18:36 hat Max Reitz geschrieben: >> On 2017-11-10 10:16, Markus Armbruster wrote: >>> Max Reitz writes: >>> bdrv_reopen_prepare() assumes that all BDS options are strings, which is not necessarily correct. This series introduces a

[Qemu-block] [PATCH] iotests: test clearing unknown autoclear_features by qcow2

2017-11-10 Thread Vladimir Sementsov-Ogievskiy
Test clearing unknown autoclear_features by qcow2 on incoming migration. Signed-off-by: Vladimir Sementsov-Ogievskiy --- Hi all! This patch shows degradation, added in 2.10 in commit commit 9c5e6594f15b7364624a3ad40306c396c93a2145 Author: Kevin Wolf Date: Thu May 4 18:52:40 2017 +0200

Re: [Qemu-block] [Qemu-devel] Intermittent hang of iotest 194 (bdrv_drain_all after non-shared storage migration)

2017-11-10 Thread Max Reitz
On 2017-11-10 03:36, Fam Zheng wrote: > On Thu, 11/09 20:31, Max Reitz wrote: >> On 2017-11-09 16:30, Fam Zheng wrote: >>> On Thu, 11/09 16:14, Max Reitz wrote: [...] *sigh* OK, I'll look into it... >>> >>> OK, I'll let you.. Just one more thing: could it relate to the >>> use-aft

Re: [Qemu-block] [Qemu-devel] [PATCH v6 0/6] block: Don't compare strings in bdrv_reopen_prepare()

2017-11-10 Thread Kevin Wolf
Am 10.11.2017 um 18:36 hat Max Reitz geschrieben: > On 2017-11-10 10:16, Markus Armbruster wrote: > > Max Reitz writes: > > > >> bdrv_reopen_prepare() assumes that all BDS options are strings, which is > >> not necessarily correct. This series introduces a new qobject_is_equal() > >> function whi

Re: [Qemu-block] [Qemu-devel] [PATCH v6 0/6] block: Don't compare strings in bdrv_reopen_prepare()

2017-11-10 Thread Max Reitz
On 2017-11-10 10:16, Markus Armbruster wrote: > Max Reitz writes: > >> bdrv_reopen_prepare() assumes that all BDS options are strings, which is >> not necessarily correct. This series introduces a new qobject_is_equal() >> function which can be used to test whether any options have changed, >> in

Re: [Qemu-block] [PATCH v2 4/5] iotests: Make 083 less flaky

2017-11-10 Thread Max Reitz
On 2017-11-10 16:51, Alberto Garcia wrote: > On Fri 10 Nov 2017 04:18:15 PM CET, Max Reitz wrote: >> On 2017-11-10 11:02, Alberto Garcia wrote: >>> On Thu 09 Nov 2017 09:30:24 PM CET, Max Reitz wrote: +echo > "$TEST_DIR/nbd-fault-injector.out" $PYTHON nbd-fault-injector.py $ext

[Qemu-block] [PATCH v2 for-2.11] block: Make bdrv_next() keep strong references

2017-11-10 Thread Max Reitz
On one hand, it is a good idea for bdrv_next() to return a strong reference because ideally nearly every pointer should be refcounted. This fixes intermittent failure of iotest 194. On the other, it is absolutely necessary for bdrv_next() itself to keep a strong reference to both the BB (in its fi

Re: [Qemu-block] [Qemu-devel] [PATCH] qcow2: fix image corruption after committing qcow2 image into base

2017-11-10 Thread Daniel P. Berrange
On Fri, Nov 10, 2017 at 10:34:59AM -0600, Eric Blake wrote: > On 11/03/2017 09:41 AM, Daniel P. Berrange wrote: > > After committing the qcow2 image contents into the base image, qemu-img > > will call bdrv_make_empty to drop the payload in the layered image. > > > > When this is done for qcow2 im

Re: [Qemu-block] [Qemu-devel] [PATCH] qcow2: fix image corruption after committing qcow2 image into base

2017-11-10 Thread Vladimir Sementsov-Ogievskiy
10.11.2017 19:34, Eric Blake wrote: On 11/03/2017 09:41 AM, Daniel P. Berrange wrote: After committing the qcow2 image contents into the base image, qemu-img will call bdrv_make_empty to drop the payload in the layered image. When this is done for qcow2 images, it blows away the LUKS encryption

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Max Reitz
On 2017-11-10 17:22, Kevin Wolf wrote: > Am 10.11.2017 um 17:13 hat Max Reitz geschrieben: >> On 2017-11-10 17:05, Kevin Wolf wrote: >>> Am 10.11.2017 um 16:23 hat Max Reitz geschrieben: On 2017-11-10 14:32, Fam Zheng wrote: > On Fri, 11/10 14:17, Kevin Wolf wrote: >> Do you actually n

Re: [Qemu-block] [Qemu-devel] [PATCH] qcow2: fix image corruption after committing qcow2 image into base

2017-11-10 Thread Eric Blake
On 11/03/2017 09:41 AM, Daniel P. Berrange wrote: > After committing the qcow2 image contents into the base image, qemu-img > will call bdrv_make_empty to drop the payload in the layered image. > > When this is done for qcow2 images, it blows away the LUKS encryption > header, making the resulting

Re: [Qemu-block] [PATCH] qcow2: fix image corruption after committing qcow2 image into base

2017-11-10 Thread Kevin Wolf
Am 03.11.2017 um 15:41 hat Daniel P. Berrange geschrieben: > After committing the qcow2 image contents into the base image, qemu-img > will call bdrv_make_empty to drop the payload in the layered image. > > When this is done for qcow2 images, it blows away the LUKS encryption > header, making the

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Kevin Wolf
Am 10.11.2017 um 17:13 hat Max Reitz geschrieben: > On 2017-11-10 17:05, Kevin Wolf wrote: > > Am 10.11.2017 um 16:23 hat Max Reitz geschrieben: > >> On 2017-11-10 14:32, Fam Zheng wrote: > >>> On Fri, 11/10 14:17, Kevin Wolf wrote: > Do you actually need to keep references to all BDSes in the

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Max Reitz
On 2017-11-10 17:05, Kevin Wolf wrote: > Am 10.11.2017 um 16:23 hat Max Reitz geschrieben: >> On 2017-11-10 14:32, Fam Zheng wrote: >>> On Fri, 11/10 14:17, Kevin Wolf wrote: Do you actually need to keep references to all BDSes in the whole list while using the iterator or would it be eno

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Kevin Wolf
Am 10.11.2017 um 16:23 hat Max Reitz geschrieben: > On 2017-11-10 14:32, Fam Zheng wrote: > > On Fri, 11/10 14:17, Kevin Wolf wrote: > >> Do you actually need to keep references to all BDSes in the whole list > >> while using the iterator or would it be enough to just keep a reference > >> to the c

Re: [Qemu-block] [PATCH v2 4/5] iotests: Make 083 less flaky

2017-11-10 Thread Alberto Garcia
On Fri 10 Nov 2017 04:18:15 PM CET, Max Reitz wrote: > On 2017-11-10 11:02, Alberto Garcia wrote: >> On Thu 09 Nov 2017 09:30:24 PM CET, Max Reitz wrote: >>> +echo > "$TEST_DIR/nbd-fault-injector.out" >>> $PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" >>> "$TEST_DIR/nbd-fault-in

Re: [Qemu-block] [PATCH] block: all I/O should be completed before removing throttle timers.

2017-11-10 Thread Stefan Hajnoczi
On Sat, Oct 21, 2017 at 01:34:00PM +0800, Zhengui Li wrote: > From: Zhengui > > 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

Re: [Qemu-block] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Max Reitz
On 2017-11-10 10:19, Stefan Hajnoczi wrote: > On Thu, Nov 09, 2017 at 09:43:15PM +0100, Max Reitz wrote: >> Draining a BDS may lead to graph modifications, which in turn may result >> in it and other BDS being stripped of their current references. If >> bdrv_drain_all_begin() and bdrv_drain_all_en

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Fam Zheng
On Fri, 11/10 16:23, Max Reitz wrote: > But bdrv_unref() is safe only in the main loop. Without having checked, > I'm not sure whether all callers of bdrv_next() are running in the main > loop. They must be. The reasoning is simple: 1) one needs to acquire the ctx of all the BDSes for safe acces

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Max Reitz
On 2017-11-10 14:32, Fam Zheng wrote: > On Fri, 11/10 14:17, Kevin Wolf wrote: >> Am 10.11.2017 um 03:45 hat Fam Zheng geschrieben: >>> On Thu, 11/09 21:43, Max Reitz wrote: Draining a BDS may lead to graph modifications, which in turn may result in it and other BDS being stripped of thei

[Qemu-block] [PATCH v4] throttle-groups: drain before detaching ThrottleState

2017-11-10 Thread Stefan Hajnoczi
I/O requests hang after stop/cont commands at least since QEMU 2.10.0 with -drive iops=100: (guest)$ dd if=/dev/zero of=/dev/vdb oflag=direct count=1000 (qemu) stop (qemu) cont ...I/O is stuck... This happens because blk_set_aio_context() detaches the ThrottleState while requests may stil

Re: [Qemu-block] [PATCH v2 4/5] iotests: Make 083 less flaky

2017-11-10 Thread Max Reitz
On 2017-11-10 11:02, Alberto Garcia wrote: > On Thu 09 Nov 2017 09:30:24 PM CET, Max Reitz wrote: >> +echo > "$TEST_DIR/nbd-fault-injector.out" >> $PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" >> "$TEST_DIR/nbd-fault-injector.conf" >"$TEST_DIR/nbd-fault-injector.out" 2>&1 >>

Re: [Qemu-block] [PATCH v2] throttle: fix a qemu crash problem when calling blk_delete

2017-11-10 Thread Alberto Garcia
On Thu 09 Nov 2017 06:12:10 PM CET, Stefan Hajnoczi wrote: >> diff --git a/block/block-backend.c b/block/block-backend.c >> index 45d9101..39c7cca 100644 >> --- a/block/block-backend.c >> +++ b/block/block-backend.c >> @@ -341,7 +341,7 @@ static void blk_delete(BlockBackend *blk) >> assert(!bl

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Fam Zheng
On Fri, 11/10 14:17, Kevin Wolf wrote: > Am 10.11.2017 um 03:45 hat Fam Zheng geschrieben: > > On Thu, 11/09 21:43, Max Reitz wrote: > > > Draining a BDS may lead to graph modifications, which in turn may result > > > in it and other BDS being stripped of their current references. If > > > bdrv_dr

Re: [Qemu-block] [Qemu-devel] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Kevin Wolf
Am 10.11.2017 um 03:45 hat Fam Zheng geschrieben: > On Thu, 11/09 21:43, Max Reitz wrote: > > Draining a BDS may lead to graph modifications, which in turn may result > > in it and other BDS being stripped of their current references. If > > bdrv_drain_all_begin() and bdrv_drain_all_end() do not k

Re: [Qemu-block] [PATCH] block: all I/O should be completed before removing throttle timers.

2017-11-10 Thread Alberto Garcia
On Sat 21 Oct 2017 07:34:00 AM CEST, Zhengui Li wrote: > From: Zhengui > > 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 befor

Re: [Qemu-block] [PATCH v2 4/5] iotests: Make 083 less flaky

2017-11-10 Thread Alberto Garcia
On Thu 09 Nov 2017 09:30:24 PM CET, Max Reitz wrote: > +echo > "$TEST_DIR/nbd-fault-injector.out" > $PYTHON nbd-fault-injector.py $extra_args "$nbd_addr" > "$TEST_DIR/nbd-fault-injector.conf" >"$TEST_DIR/nbd-fault-injector.out" 2>&1 & It seems that in this patch you're indenting wit

Re: [Qemu-block] [PATCH] block: all I/O should be completed before removing throttle timers.

2017-11-10 Thread Alberto Garcia
On Sat 21 Oct 2017 07:34:00 AM CEST, Zhengui Li wrote: > From: Zhengui > > 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 befor

Re: [Qemu-block] [PATCH for-2.11] block: Keep strong reference when draining all BDS

2017-11-10 Thread Stefan Hajnoczi
On Thu, Nov 09, 2017 at 09:43:15PM +0100, Max Reitz wrote: > Draining a BDS may lead to graph modifications, which in turn may result > in it and other BDS being stripped of their current references. If > bdrv_drain_all_begin() and bdrv_drain_all_end() do not keep strong > references themselves, t

Re: [Qemu-block] [Qemu-devel] [PATCH v6 0/6] block: Don't compare strings in bdrv_reopen_prepare()

2017-11-10 Thread Markus Armbruster
Max Reitz writes: > bdrv_reopen_prepare() assumes that all BDS options are strings, which is > not necessarily correct. This series introduces a new qobject_is_equal() > function which can be used to test whether any options have changed, > independently of their type. Series looks ready to me.

Re: [Qemu-block] [Qemu-devel] [PATCH v6 6/6] tests: Add check-qobject for equality tests

2017-11-10 Thread Markus Armbruster
Max Reitz writes: > Add a new test file (check-qobject.c) for unit tests that concern > QObjects as a whole. > > Its only purpose for now is to test the qobject_is_equal() function. > > Signed-off-by: Max Reitz Reviewed-by: Markus Armbruster