[Qemu-devel] [PATCH v2 00/12] block: Fixes for concurrent block jobs

Max Reitz posted 12 patches 4 years, 10 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
include/block/block_int.h     |   3 +
block.c                       |  93 +++++++++++++------
block/commit.c                |   4 +
block/mirror.c                |   4 +
block/stream.c                |   4 +-
tests/qemu-iotests/030        | 150 +++++++++++++++++++++++++------
tests/qemu-iotests/030.out    |   4 +-
tests/qemu-iotests/258        | 163 ++++++++++++++++++++++++++++++++++
tests/qemu-iotests/258.out    |  33 +++++++
tests/qemu-iotests/group      |   1 +
tests/qemu-iotests/iotests.py |  18 ++--
11 files changed, 413 insertions(+), 64 deletions(-)
create mode 100755 tests/qemu-iotests/258
create mode 100644 tests/qemu-iotests/258.out
[Qemu-devel] [PATCH v2 00/12] block: Fixes for concurrent block jobs
Posted by Max Reitz 4 years, 10 months ago
This is a v2 to “block: Add BDS.never_freeze”.

It depends on my “block: Delay poll when ending drained sections”
series:

Depends-on: <20190619152603.5937-1-mreitz@redhat.com>


It turned out that if you run 030 (or just the new test_overlapping_5
case) sufficiently often, it breaks; which is why I’m hesitant to just
merge the “add never_freeze” series as it is.

There are several reasons for why this test case breaks, I hope patches
3 to 6 fix them.  Patch 12 adds a test that is much more reliable than
test_overlapping_5 at detecting the problems fixed by at least patches 4
to 6.  (I think that 3 doesn’t really need a test.)

I’m sure there are other ways to see these problems, but well, coming
from 030, concurrent commit/stream jobs are how I reproduced them.
Hence the same of this series.

Patch 2 is for something I encountered on the way.  Patch 11 tests it.


v2:
- Added a bunch of more patches.


git backport-diff against v1:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/12:[----] [--] 'block: Add BDS.never_freeze'
002/12:[down] 'block/stream: Fix error path'
003/12:[down] 'block/stream: Swap backing file change order'
004/12:[down] 'block: Keep subtree drained in drop_intermediate'
005/12:[down] 'block: Reduce (un)drains when replacing a child'
006/12:[down] 'block: Deep-clear inherits_from'
007/12:[----] [--] 'iotests: Fix throttling in 030'
008/12:[----] [--] 'iotests: Compare error messages in 030'
009/12:[----] [--] 'iotests: Add @use_log to VM.run_job()'
010/12:[----] [--] 'iotests: Add new case to 030'
011/12:[down] 'iotests: Add read-only test case to 030'
012/12:[down] 'iotests: Add test for concurrent stream/commit'



Max Reitz (12):
  block: Add BDS.never_freeze
  block/stream: Fix error path
  block/stream: Swap backing file change order
  block: Keep subtree drained in drop_intermediate
  block: Reduce (un)drains when replacing a child
  block: Deep-clear inherits_from
  iotests: Fix throttling in 030
  iotests: Compare error messages in 030
  iotests: Add @use_log to VM.run_job()
  iotests: Add new case to 030
  iotests: Add read-only test case to 030
  iotests: Add test for concurrent stream/commit

 include/block/block_int.h     |   3 +
 block.c                       |  93 +++++++++++++------
 block/commit.c                |   4 +
 block/mirror.c                |   4 +
 block/stream.c                |   4 +-
 tests/qemu-iotests/030        | 150 +++++++++++++++++++++++++------
 tests/qemu-iotests/030.out    |   4 +-
 tests/qemu-iotests/258        | 163 ++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/258.out    |  33 +++++++
 tests/qemu-iotests/group      |   1 +
 tests/qemu-iotests/iotests.py |  18 ++--
 11 files changed, 413 insertions(+), 64 deletions(-)
 create mode 100755 tests/qemu-iotests/258
 create mode 100644 tests/qemu-iotests/258.out

-- 
2.21.0


Re: [Qemu-devel] [PATCH v2 00/12] block: Fixes for concurrent block jobs
Posted by Max Reitz 4 years, 9 months ago
Ping

I suppose I’ll apply the patches I had in v1 if I don’t get reviews for
the new patches, because some fixes are better than none.

(And probably patch 2, because it’s obvious.)

Max

On 03.07.19 19:28, Max Reitz wrote:
> This is a v2 to “block: Add BDS.never_freeze”.
> 
> It depends on my “block: Delay poll when ending drained sections”
> series:
> 
> Depends-on: <20190619152603.5937-1-mreitz@redhat.com>
> 
> 
> It turned out that if you run 030 (or just the new test_overlapping_5
> case) sufficiently often, it breaks; which is why I’m hesitant to just
> merge the “add never_freeze” series as it is.
> 
> There are several reasons for why this test case breaks, I hope patches
> 3 to 6 fix them.  Patch 12 adds a test that is much more reliable than
> test_overlapping_5 at detecting the problems fixed by at least patches 4
> to 6.  (I think that 3 doesn’t really need a test.)
> 
> I’m sure there are other ways to see these problems, but well, coming
> from 030, concurrent commit/stream jobs are how I reproduced them.
> Hence the same of this series.
> 
> Patch 2 is for something I encountered on the way.  Patch 11 tests it.
> 
> 
> v2:
> - Added a bunch of more patches.
> 
> 
> git backport-diff against v1:
> 
> Key:
> [----] : patches are identical
> [####] : number of functional differences between upstream/downstream patch
> [down] : patch is downstream-only
> The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
> 001/12:[----] [--] 'block: Add BDS.never_freeze'
> 002/12:[down] 'block/stream: Fix error path'
> 003/12:[down] 'block/stream: Swap backing file change order'
> 004/12:[down] 'block: Keep subtree drained in drop_intermediate'
> 005/12:[down] 'block: Reduce (un)drains when replacing a child'
> 006/12:[down] 'block: Deep-clear inherits_from'
> 007/12:[----] [--] 'iotests: Fix throttling in 030'
> 008/12:[----] [--] 'iotests: Compare error messages in 030'
> 009/12:[----] [--] 'iotests: Add @use_log to VM.run_job()'
> 010/12:[----] [--] 'iotests: Add new case to 030'
> 011/12:[down] 'iotests: Add read-only test case to 030'
> 012/12:[down] 'iotests: Add test for concurrent stream/commit'
> 
> 
> 
> Max Reitz (12):
>   block: Add BDS.never_freeze
>   block/stream: Fix error path
>   block/stream: Swap backing file change order
>   block: Keep subtree drained in drop_intermediate
>   block: Reduce (un)drains when replacing a child
>   block: Deep-clear inherits_from
>   iotests: Fix throttling in 030
>   iotests: Compare error messages in 030
>   iotests: Add @use_log to VM.run_job()
>   iotests: Add new case to 030
>   iotests: Add read-only test case to 030
>   iotests: Add test for concurrent stream/commit
> 
>  include/block/block_int.h     |   3 +
>  block.c                       |  93 +++++++++++++------
>  block/commit.c                |   4 +
>  block/mirror.c                |   4 +
>  block/stream.c                |   4 +-
>  tests/qemu-iotests/030        | 150 +++++++++++++++++++++++++------
>  tests/qemu-iotests/030.out    |   4 +-
>  tests/qemu-iotests/258        | 163 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/258.out    |  33 +++++++
>  tests/qemu-iotests/group      |   1 +
>  tests/qemu-iotests/iotests.py |  18 ++--
>  11 files changed, 413 insertions(+), 64 deletions(-)
>  create mode 100755 tests/qemu-iotests/258
>  create mode 100644 tests/qemu-iotests/258.out
> 


Re: [Qemu-devel] [PATCH v2 00/12] block: Fixes for concurrent block jobs
Posted by Max Reitz 4 years, 9 months ago
On 03.07.19 19:28, Max Reitz wrote:
> This is a v2 to “block: Add BDS.never_freeze”.
> 
> It depends on my “block: Delay poll when ending drained sections”
> series:
> 
> Depends-on: <20190619152603.5937-1-mreitz@redhat.com>
> 
> 
> It turned out that if you run 030 (or just the new test_overlapping_5
> case) sufficiently often, it breaks; which is why I’m hesitant to just
> merge the “add never_freeze” series as it is.
> 
> There are several reasons for why this test case breaks, I hope patches
> 3 to 6 fix them.  Patch 12 adds a test that is much more reliable than
> test_overlapping_5 at detecting the problems fixed by at least patches 4
> to 6.  (I think that 3 doesn’t really need a test.)
> 
> I’m sure there are other ways to see these problems, but well, coming
> from 030, concurrent commit/stream jobs are how I reproduced them.
> Hence the same of this series.
> 
> Patch 2 is for something I encountered on the way.  Patch 11 tests it.

Applied patches 1, 2, 3, and 6 through 11 to my block branch.


So what remains are patches 4, 5, and 12.

For 4, I don’t know whether the approach is too heavy-handed (maybe a
simple bdrv_drained_begin()/-end() would suffice), and whether it’s even
right to wrap the whole function in a drained section.  (Unless there
are objections, I’ll still take it for rc2.)

5 is just pretty complex, and it also depends on my other series in its
current form.  So I wouldn’t like to take it without any reviews.

12 is a test that will fail without 4 and 5, so I can’t take it without
those two.

Max