mbox series

[v4,00/15] Apply COR-filter to the block-stream permanently

Message ID 1589302245-893269-1-git-send-email-andrey.shinkevich@virtuozzo.com (mailing list archive)
Headers show
Series Apply COR-filter to the block-stream permanently | expand

Message

Andrey Shinkevich May 12, 2020, 4:50 p.m. UTC 
With this series, all the block-stream COR operations pass through
the COR-filter. The patches 01-08/15 are taken from the series
"block: Deal with filters" by Max Reitz, the full version of that
can be found in the branches:

      https://git.xanclic.moe/XanClic/qemu child-access-functions-v6
      https://github.com/XanClic/qemu child-access-functions-v6

      When running iotests, apply "char-socket: Fix race condition"
      to avoid sporadic segmentation faults.
v4:
  01: Initialization of the COR-filter BDRVStateCOR member.

v3:
  01: The COR filter insert/remove functions moved to block/copy-on-read.c
      to be a part of API.
  02: block/stream.c code refactoring.
  03: The separate call to block_job_add_bdrv() is used to block operations
      on the active node after the filter inserted and the job created.
  04: The iotests case 030::test_overlapping_4 was modified to unbound
      the block-stream job from the base node.
  05: The COR driver functions preadv/pwritev replaced with their analogous
      preadv/pwritev_part.

v2:
  01: No more skipping filters while checking for operation blockers.
      However, we exclude filters between the bottom node and base
      because we do not set the operation blockers for filters anymore.
  02: As stated above, we do not set the operation blockers for filters
      anymore. So, skip filters when we block operations for the target
      node.
  03: The comment added for the patch 4/7.
  04: The QAPI target version changed to 5.1.
  05: The 'filter-node-name' now excluded from using in the test #030.
      If we need it no more in a final version of the series, the patch
      5/7 may be removed.
  06: The COR-filter included into the frozen chain of a block-stream job.
      The 'above_base' node pointer is left because it is essential for
      finding the base node in case of filters above.


Andrey Shinkevich (7):
  block: prepare block-stream for using COR-filter
  copy-on-read: Support change filename functions
  copy-on-read: Support preadv/pwritev_part functions
  copy-on-read: add filter append/drop functions
  qapi: add filter-node-name to block-stream
  iotests: prepare 245 for using filter in block-stream
  block: apply COR-filter to block-stream jobs

Max Reitz (8):
  block: Mark commit and mirror as filter drivers
  copy-on-read: Support compressed writes
  block: Add child access functions
  block: Add chain helper functions
  block: Include filters when freezing backing chain
  block: Use CAFs in block status functions
  commit: Deal with filters when blocking intermediate nodes
  block: Use CAFs when working with backing chains

 block.c                        | 275 ++++++++++++++++++++++++++++++++++-------
 block/commit.c                 |  85 ++++++++++---
 block/copy-on-read.c           | 159 ++++++++++++++++++++++--
 block/io.c                     |  19 +--
 block/mirror.c                 |   6 +-
 block/monitor/block-hmp-cmds.c |   4 +-
 block/stream.c                 |  89 +++++++++----
 blockdev.c                     |  19 ++-
 include/block/block.h          |   6 +-
 include/block/block_int.h      |  67 +++++++++-
 qapi/block-core.json           |   6 +
 tests/qemu-iotests/030         |  17 +--
 tests/qemu-iotests/141.out     |   2 +-
 tests/qemu-iotests/245         |  10 +-
 14 files changed, 625 insertions(+), 139 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy May 12, 2020, 5:56 p.m. UTC | #1 
12.05.2020 19:50, Andrey Shinkevich wrote:
> With this series, all the block-stream COR operations pass through
> the COR-filter. The patches 01-08/15 are taken from the series
> "block: Deal with filters" by Max Reitz, the full version of that
> can be found in the branches:
> 
>        https://git.xanclic.moe/XanClic/qemu child-access-functions-v6
>        https://github.com/XanClic/qemu child-access-functions-v6
> 
>        When running iotests, apply "char-socket: Fix race condition"
>        to avoid sporadic segmentation faults.
> v4:
>    01: Initialization of the COR-filter BDRVStateCOR member.

Hmm... but 01 doesn't touch COR-filter

> 
> v3:
>    01: The COR filter insert/remove functions moved to block/copy-on-read.c
>        to be a part of API.
>    02: block/stream.c code refactoring.
>    03: The separate call to block_job_add_bdrv() is used to block operations
>        on the active node after the filter inserted and the job created.
>    04: The iotests case 030::test_overlapping_4 was modified to unbound
>        the block-stream job from the base node.
>    05: The COR driver functions preadv/pwritev replaced with their analogous
>        preadv/pwritev_part.

I assume, these changes are about your patches, which are 09-15, and Max's patches
are unchanged, right?

> 
> v2:
>    01: No more skipping filters while checking for operation blockers.
>        However, we exclude filters between the bottom node and base
>        because we do not set the operation blockers for filters anymore.
>    02: As stated above, we do not set the operation blockers for filters
>        anymore. So, skip filters when we block operations for the target
>        node.
>    03: The comment added for the patch 4/7.
>    04: The QAPI target version changed to 5.1.
>    05: The 'filter-node-name' now excluded from using in the test #030.
>        If we need it no more in a final version of the series, the patch
>        5/7 may be removed.
>    06: The COR-filter included into the frozen chain of a block-stream job.
>        The 'above_base' node pointer is left because it is essential for
>        finding the base node in case of filters above.
> 
> 
> Andrey Shinkevich (7):
>    block: prepare block-stream for using COR-filter
>    copy-on-read: Support change filename functions
>    copy-on-read: Support preadv/pwritev_part functions
>    copy-on-read: add filter append/drop functions
>    qapi: add filter-node-name to block-stream
>    iotests: prepare 245 for using filter in block-stream
>    block: apply COR-filter to block-stream jobs
> 
> Max Reitz (8):
>    block: Mark commit and mirror as filter drivers

this is for commit

>    copy-on-read: Support compressed writes

for stream

>    block: Add child access functions

I do think, that for these series we need only filtered child and nothing more

>    block: Add chain helper functions
>    block: Include filters when freezing backing chain
>    block: Use CAFs in block status functions
>    commit: Deal with filters when blocking intermediate nodes
>    block: Use CAFs when working with backing chains

So, fix stream, commit and some thing used in it.


Hi Max! Could you take a brief look and say, could we proceed in this way, taking part of your old series? How much it conflicts with your plans?

Let me clarify. This all is needed, as we have old proposed feature (and patches): discarding blocks from intermediate images during block-stream. It helps to save disk space during stream process. And the correct way to get access to intermediate nodes (to be able to discard from them) is to append a filter. Firstly we proposed our own filter, but that was proposed on list to use existing COR filter for stream and it seemed a correct way. So we are trying to insert this COR filter.

And the problem with it that without your series it breaks iotest 30, which does different magic with parallel stream and commit on the same backing chain.

So, it was my proposal to extract something from your series, to make this test work. And the result is here. I thought that the necessary part of your series for stream/commit is smaller.. But still, 8 patches is not too much. The feature for stream is being postponed already for more than a year due to this trouble. We need to proceed somehow. And the feature is useful.
no-reply@patchew.org May 13, 2020, 12:09 a.m. UTC | #2 
Patchew URL: https://patchew.org/QEMU/1589302245-893269-1-git-send-email-andrey.shinkevich@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      crypto/ivgen-plain.o
  CC      crypto/ivgen-plain64.o
  CC      crypto/afsplit.o
/tmp/qemu-test/src/block/copy-on-read.c:29:32: fatal error: block/copy-on-read.h: No such file or directory
 #include "block/copy-on-read.h"
                                ^
compilation terminated.
---
  CC      io/dns-resolver.o
  CC      io/net-listener.o
  CC      io/task.o
/tmp/qemu-test/src/block/stream.c:22:32: fatal error: block/copy-on-read.h: No such file or directory
 #include "block/copy-on-read.h"
                                ^
compilation terminated.
---
  CC      job-qmp.o
  CC      os-posix.o
  CC      monitor/monitor.o
make: *** [block/copy-on-read.o] Error 1
make: *** Waiting for unfinished jobs....
make: *** [block/stream.o] Error 1
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=55e77e0cea294d6d8e4dec1af650005c', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-2tumy38l/src/docker-src.2020-05-12-20.07.13.6173:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=55e77e0cea294d6d8e4dec1af650005c
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-2tumy38l/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m13.117s
user    0m8.234s


The full log is available at
http://patchew.org/logs/1589302245-893269-1-git-send-email-andrey.shinkevich@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org May 13, 2020, 12:13 a.m. UTC | #3 
Patchew URL: https://patchew.org/QEMU/1589302245-893269-1-git-send-email-andrey.shinkevich@virtuozzo.com/



Hi,

This series failed the asan build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-debug@fedora TARGET_LIST=x86_64-softmmu J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      io/channel.o
  CC      io/channel-buffer.o
  CC      io/channel-command.o
/tmp/qemu-test/src/block/copy-on-read.c:29:10: fatal error: 'block/copy-on-read.h' file not found
#include "block/copy-on-read.h"
         ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/copy-on-read.o] Error 1
make: *** Waiting for unfinished jobs....
/tmp/qemu-test/src/block/stream.c:22:10: fatal error: 'block/copy-on-read.h' file not found
#include "block/copy-on-read.h"
         ^~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/stream.o] Error 1
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=6ea58254d5ba4043a1dc03c95e1dda1b', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=x86_64-softmmu', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-qxc99rf5/src/docker-src.2020-05-12-20.09.58.12869:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-debug']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=6ea58254d5ba4043a1dc03c95e1dda1b
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-qxc99rf5/src'
make: *** [docker-run-test-debug@fedora] Error 2

real    3m4.403s
user    0m8.425s


The full log is available at
http://patchew.org/logs/1589302245-893269-1-git-send-email-andrey.shinkevich@virtuozzo.com/testing.asan/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
no-reply@patchew.org May 13, 2020, 12:15 a.m. UTC | #4 
Patchew URL: https://patchew.org/QEMU/1589302245-893269-1-git-send-email-andrey.shinkevich@virtuozzo.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      os-win32.o
  CC      monitor/monitor.o
  CC      monitor/qmp.o
/tmp/qemu-test/src/block/copy-on-read.c:29:10: fatal error: block/copy-on-read.h: No such file or directory
 #include "block/copy-on-read.h"
          ^~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/copy-on-read.o] Error 1
make: *** Waiting for unfinished jobs....
/tmp/qemu-test/src/block/stream.c:22:10: fatal error: block/copy-on-read.h: No such file or directory
 #include "block/copy-on-read.h"
          ^~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
make: *** [/tmp/qemu-test/src/rules.mak:69: block/stream.o] Error 1
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 664, in <module>
    sys.exit(main())
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=b2a19e036a5649adbddbea04db8b1509', '-u', '1003', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew2/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-os8td8ck/src/docker-src.2020-05-12-20.14.12.18252:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=b2a19e036a5649adbddbea04db8b1509
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-os8td8ck/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    1m38.821s
user    0m8.138s


The full log is available at
http://patchew.org/logs/1589302245-893269-1-git-send-email-andrey.shinkevich@virtuozzo.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Vladimir Sementsov-Ogievskiy May 13, 2020, 2:06 p.m. UTC | #5 
12.05.2020 20:56, Vladimir Sementsov-Ogievskiy wrote:
> 12.05.2020 19:50, Andrey Shinkevich wrote:
>> With this series, all the block-stream COR operations pass through
>> the COR-filter. The patches 01-08/15 are taken from the series
>> "block: Deal with filters" by Max Reitz, the full version of that
>> can be found in the branches:
>>
>>        https://git.xanclic.moe/XanClic/qemu child-access-functions-v6
>>        https://github.com/XanClic/qemu child-access-functions-v6
>>
>>        When running iotests, apply "char-socket: Fix race condition"
>>        to avoid sporadic segmentation faults.
>> v4:
>>    01: Initialization of the COR-filter BDRVStateCOR member.
> 
> Hmm... but 01 doesn't touch COR-filter
> 
>>
>> v3:
>>    01: The COR filter insert/remove functions moved to block/copy-on-read.c
>>        to be a part of API.
>>    02: block/stream.c code refactoring.
>>    03: The separate call to block_job_add_bdrv() is used to block operations
>>        on the active node after the filter inserted and the job created.
>>    04: The iotests case 030::test_overlapping_4 was modified to unbound
>>        the block-stream job from the base node.
>>    05: The COR driver functions preadv/pwritev replaced with their analogous
>>        preadv/pwritev_part.
> 
> I assume, these changes are about your patches, which are 09-15, and Max's patches
> are unchanged, right?
> 
>>
>> v2:
>>    01: No more skipping filters while checking for operation blockers.
>>        However, we exclude filters between the bottom node and base
>>        because we do not set the operation blockers for filters anymore.
>>    02: As stated above, we do not set the operation blockers for filters
>>        anymore. So, skip filters when we block operations for the target
>>        node.
>>    03: The comment added for the patch 4/7.
>>    04: The QAPI target version changed to 5.1.
>>    05: The 'filter-node-name' now excluded from using in the test #030.
>>        If we need it no more in a final version of the series, the patch
>>        5/7 may be removed.
>>    06: The COR-filter included into the frozen chain of a block-stream job.
>>        The 'above_base' node pointer is left because it is essential for
>>        finding the base node in case of filters above.
>>
>>
>> Andrey Shinkevich (7):
>>    block: prepare block-stream for using COR-filter
>>    copy-on-read: Support change filename functions
>>    copy-on-read: Support preadv/pwritev_part functions
>>    copy-on-read: add filter append/drop functions
>>    qapi: add filter-node-name to block-stream
>>    iotests: prepare 245 for using filter in block-stream
>>    block: apply COR-filter to block-stream jobs
>>
>> Max Reitz (8):
>>    block: Mark commit and mirror as filter drivers
> 
> this is for commit
> 
>>    copy-on-read: Support compressed writes
> 
> for stream
> 
>>    block: Add child access functions
> 
> I do think, that for these series we need only filtered child and nothing more
> 
>>    block: Add chain helper functions
>>    block: Include filters when freezing backing chain
>>    block: Use CAFs in block status functions
>>    commit: Deal with filters when blocking intermediate nodes
>>    block: Use CAFs when working with backing chains
> 
> So, fix stream, commit and some thing used in it.
> 
> 
> Hi Max! Could you take a brief look and say, could we proceed in this way, taking part of your old series? How much it conflicts with your plans?
> 
> Let me clarify. This all is needed, as we have old proposed feature (and patches): discarding blocks from intermediate images during block-stream. It helps to save disk space during stream process. And the correct way to get access to intermediate nodes (to be able to discard from them) is to append a filter. Firstly we proposed our own filter, but that was proposed on list to use existing COR filter for stream and it seemed a correct way. So we are trying to insert this COR filter.
> 
> And the problem with it that without your series it breaks iotest 30, which does different magic with parallel stream and commit on the same backing chain.
> 
> So, it was my proposal to extract something from your series, to make this test work. And the result is here. I thought that the necessary part of your series for stream/commit is smaller.. But still, 8 patches is not too much. The feature for stream is being postponed already for more than a year due to this trouble. We need to proceed somehow. And the feature is useful.
> 
> 

Just ping, to make it above v5. Sorry for accidentally sent v5, Andrey didn't receive my reply on v4 for some email issue. We firstly want to approve general design of the series, let's do it here.