Re: [PATCH v7 00/47] block: Deal with filters
Am 25.06.2020 um 17:21 hat Max Reitz geschrieben: > v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html > > Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 > Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 Okay, finally made it through the series. Sorry for taking so long. You can add my Reviewed-by to all patches that I didn't comment on. (Yes, I'm just too lazy to make the list myself. :-)) Kevin
Re: [PATCH v7 00/47] block: Deal with filters
On 09.07.2020 11:20, Max Reitz wrote: On 08.07.20 22:47, Eric Blake wrote: On 7/8/20 12:20 PM, Andrey Shinkevich wrote: On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" introduced with the commit a7399eb iotests: Make _filter_img_create more active You appear to be staging off an unreleased preliminary tree. a7399eb is not upstream; the upstream commit 'iotests: Make _filter_img_create more active' is commit 57ee95ed, and while it uses readarray, it does not use the problematic -d. In other words, it looks like the problem was caught and fixed in between the original patch creation and the pull request. Ah, sorry, my mail client’s threading layout hid this mail from me a bit. Yes. Well, no, it wasn’t fixed before the pull request, but it was fixed in the second pull request. But yes. Max I'm clear with it now. Thank you all for your explenations and time! Andrey
Re: [PATCH v7 00/47] block: Deal with filters
On 08.07.20 22:47, Eric Blake wrote: > On 7/8/20 12:20 PM, Andrey Shinkevich wrote: >> On 25.06.2020 18:21, Max Reitz wrote: >>> v6: >>> https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html >>> >>> Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 >>> Branch: https://git.xanclic.moe/XanClic/qemu.git >>> child-access-functions-v7 >>> >>> >> I cloned the branch from the github and built successfully. >> >> Running the iotests reports multiple errors of such a kind: >> >> 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') >> >> "./common.filter: line 128: readarray: -d: invalid option" >> >> introduced with the commit >> >> a7399eb iotests: Make _filter_img_create more active > > You appear to be staging off an unreleased preliminary tree. a7399eb is > not upstream; the upstream commit 'iotests: Make _filter_img_create more > active' is commit 57ee95ed, and while it uses readarray, it does not use > the problematic -d. In other words, it looks like the problem was > caught and fixed in between the original patch creation and the pull > request. Ah, sorry, my mail client’s threading layout hid this mail from me a bit. Yes. Well, no, it wasn’t fixed before the pull request, but it was fixed in the second pull request. But yes. Max signature.asc Description: OpenPGP digital signature
Re: [PATCH v7 00/47] block: Deal with filters
On 08.07.20 22:37, Eric Blake wrote: > On 7/8/20 2:46 PM, Andrey Shinkevich wrote: >> >> On 08.07.2020 20:32, Eric Blake wrote: >>> On 7/8/20 12:20 PM, Andrey Shinkevich wrote: On 25.06.2020 18:21, Max Reitz wrote: > v6: > https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html > > Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 > Branch: https://git.xanclic.moe/XanClic/qemu.git > child-access-functions-v7 > > I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" >>> >>> Arrgh. If I'm reading bash's changelog correctly, readarray -d was >>> introduced in bash 4.4, so I'm guessing you're still on 4.3 or >>> earlier? What bash version and platform are you using? >>> >> My bash version is 4.2.46. >> >> It is the latest in the virtuozzolinux-base repository. I should >> install the 4.4 package manually. > > Well, if bash 4.2 is the default installed version on any of our > platforms that meet our supported criteria, then we should instead fix > the patch in question to avoid non-portable use of readarray. > > Per https://repology.org/project/bash/versions (hinted from > docs/system/build-platforms.rst), at least CentOS 7 still ships bash > 4.2, and per 'make docker', centos7 is still a viable build target. So > we do indeed need to fix our regression. There is no regression. It’s just that I based this series on an earlier version of “Make _filter_img_create more active” – when I sent a pull request for that version, Peter already reported to me that it failed on some test environments, so I revised it. You’ll find there is no a7399eb in master; it’s 57ee95ed4ee there and doesn’t use -d. (My branch on github/gitea is still based on that older version, though, because that’s what I wrote it on.) Max signature.asc Description: OpenPGP digital signature
Re: [PATCH v7 00/47] block: Deal with filters
On 7/8/20 12:20 PM, Andrey Shinkevich wrote: On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" introduced with the commit a7399eb iotests: Make _filter_img_create more active You appear to be staging off an unreleased preliminary tree. a7399eb is not upstream; the upstream commit 'iotests: Make _filter_img_create more active' is commit 57ee95ed, and while it uses readarray, it does not use the problematic -d. In other words, it looks like the problem was caught and fixed in between the original patch creation and the pull request. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v7 00/47] block: Deal with filters
On 7/8/20 2:46 PM, Andrey Shinkevich wrote: On 08.07.2020 20:32, Eric Blake wrote: On 7/8/20 12:20 PM, Andrey Shinkevich wrote: On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" Arrgh. If I'm reading bash's changelog correctly, readarray -d was introduced in bash 4.4, so I'm guessing you're still on 4.3 or earlier? What bash version and platform are you using? My bash version is 4.2.46. It is the latest in the virtuozzolinux-base repository. I should install the 4.4 package manually. Well, if bash 4.2 is the default installed version on any of our platforms that meet our supported criteria, then we should instead fix the patch in question to avoid non-portable use of readarray. Per https://repology.org/project/bash/versions (hinted from docs/system/build-platforms.rst), at least CentOS 7 still ships bash 4.2, and per 'make docker', centos7 is still a viable build target. So we do indeed need to fix our regression. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v7 00/47] block: Deal with filters
On 7/8/20 12:20 PM, Andrey Shinkevich wrote: On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" Arrgh. If I'm reading bash's changelog correctly, readarray -d was introduced in bash 4.4, so I'm guessing you're still on 4.3 or earlier? What bash version and platform are you using? introduced with the commit a7399eb iotests: Make _filter_img_create more active Andrey -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Re: [PATCH v7 00/47] block: Deal with filters
On 08.07.2020 20:32, Eric Blake wrote: On 7/8/20 12:20 PM, Andrey Shinkevich wrote: On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" Arrgh. If I'm reading bash's changelog correctly, readarray -d was introduced in bash 4.4, so I'm guessing you're still on 4.3 or earlier? What bash version and platform are you using? My bash version is 4.2.46. It is the latest in the virtuozzolinux-base repository. I should install the 4.4 package manually. Thank you Eric for your hint! Andrey introduced with the commit a7399eb iotests: Make _filter_img_create more active Andrey
Re: [PATCH v7 00/47] block: Deal with filters
On 25.06.2020 18:21, Max Reitz wrote: v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html Branch: https://github.com/XanClic/qemu.git child-access-functions-v7 Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7 I cloned the branch from the github and built successfully. Running the iotests reports multiple errors of such a kind: 128: readarray -td '' formatting_line < <(sed -e 's/, fmt=/\x0/') "./common.filter: line 128: readarray: -d: invalid option" introduced with the commit a7399eb iotests: Make _filter_img_create more active Andrey
[PATCH v7 00/47] block: Deal with filters
v6: https://lists.nongnu.org/archive/html/qemu-devel/2019-08/msg01715.html
Branch: https://github.com/XanClic/qemu.git child-access-functions-v7
Branch: https://git.xanclic.moe/XanClic/qemu.git child-access-functions-v7
Hello!
This is v7. Conceptually, not much has changed, so please follow the
above link to v6’s cover letter if you’re looking for an introduction
to this series.
I did say that conceptually, not much has changed, but from a diff
standpoint, a lot has changed all over this series.
Changes from v6:
- Patch 1:
- More elaborate explanation of .is_filter
- Changed function names
- Dropped bdrv_storage_child() and bdrv_metadata_child()
- Some checking of the BdrvChildRole
- Patch 2:
- Mostly changes resulting from the different naming scheme
- Patch 3: New
- Patch 5:
- Don’t rename those functions
- Don’t drop a comment that shouldn’t be dropped
- Patch 7:
- Use block_driver_can_compress()
- Move setting @filtered down where it’s needed
- Patches 10 and 11: New (extension of 8 and 9)
- Patch 12:
- Function name changes
- More cases:
- bdrv_recurse_can_replace()
- init_dirty_bitmap_migration()
- bdrv_co_truncate() has changed
- Patch 13:
- Function name changes
- Patch 14:
- Variables renamed to be more consistent with the rest of this series
- Function name changes
- The freeze backing chain functions haven’t been renamed
- STREAM_BUFFER_SIZE is STREAM_CHUNK as of some point last year
- Fix overlay finding (e.g. handle when @base is not in the device’s
backing chain)
- Patch 15:
- Added note to the commit message that bdrv_find_overlay()’s behavior
changes a bit
- Function name changes
- Restructured bdrv_find_backing_image() loop a bit
- Patch 16: New (became necessary because of truncate having to look at
the backing file as of 955c7d6687fefcd903900)
- Patch 17:
- Function name changes
- The freeze backing chain functions haven’t been renamed
- Patch 18:
- Only flush children for which the parent has taken the WRITE
permission
- Mention that this is a bug fix for qcow2
- Patch 19: New
- Patch 20: New, replaces “block: Use CAFs in bdrv_refresh_limits()”
- Patch 23:
- We can only really fall back to bs->file or bs->backing, so stop
pretending otherwise
- Patch 24:
- Rebase conflicts
- Patches 25, 26, 27, and 28: New, they replace “block: Fix
bdrv_get_allocated_file_size's fallback”
- Patch 29:
- Function name changes
- Patch 30: New, split out from the next patch
- Patch 31:
- Function name changes
- bdrv_skip_implicit_filters() can deal with NULL arguments, so don’t
wrap it in an “if (bs) {}” block
- For bdrv_query_bds_stats(), the bs->file part has been split into
the preceding patch (patch 30)
- Addional actual-size line in the iotest output thanks to patch 28
- Patch 32: New
- Patch 33:
- Additional note in the QAPI documentation concerning @replaces (that
by default, the first non-implicit node on @device is replaced)
- Move that skippage of implicit nodes to blockdev_mirror_common()
(from qmp_drive_mirror() and qmp_blockdev_mirror())
- Function name changes
- Rename s/source/target_backing_bs/ in qmp_drive_mirror(), because
that’s better
- Don’t disallow mirroring through filters with sync=top
- Patch 34:
- Function name changes
- There is backup-top.c to care about, too, now
- Patch 35:
- Function name changes
- Call bdrv_commit() even for nodes that do not have backing files so
we get an error
- s/above_base/base_overlay/
- Patch 36:
- Function name changes
- Patch 37:
- Function name changes
- In img_convert(), when inquiring target_backing_sectors, use
bdrv_backing_chain_next() instead of bdrv_cow_bs() (because @out_bs
may be a filter)
- Forgot to use the backing file of @unfiltered_bs for
bdrv_is_allocated_above() in img_rebase() (instead of @unfiltered_bs
itself), fixed
- Patch 39:
- Function name changes
- Patch 40:
- Function name changes
- There are backup-top and filter-compress now
- Patch 41:
- Make bdrv_backing_overridden() globally available, so block/qapi.c
can use it to determine whether we can inquire bs->backing’s format
to get the backing_format
- Function name changes
- Patch 42: New
- Patch 43:
- Rebase conflicts
- Patch 44:
- Create a dedicated do_test_io() function
- Don’t unnecessarily clear and pass has_quit
- Drop assert_no_active_block_jobs() that does very little
- Additional graph constraint check
- Rebase conflict in the reference output
- Patch 45:
- Rebase conflict in the reference output
- Patch 46:
- Use _rm_test_img rather than rm -f
- Skip one of the test cases when IMGOPTS asks for a data_file
- Patch 47:
- Rebase conflict in the reference output
Patches removed:
- The whole check_to_replace_node() stuff for mirror (was its own
series)
- Making bdrv_get_cumulative_perm() public, because it already
