On 07/13/2018 12:49 PM, Max Reitz wrote:
> Ping – any thoughts on the design, Kevin?
>
>
> (Continuing to see how much of a mess our backing filename handling is
> (half of the time, bs->backing_file is seen as a value in the image
> header (so relative paths are interpreted relatively to the overlay),
> half of the time it is seen as a cache of bs->backing->bs->filename (so
> relative paths are given relatively to qemu)), I am nearly inclined to
> move off the first part of this series that deals with detecting changed
> backing files until later, when I try to tackle that bs->backing_file
> issue.)
>
I'm not Kevin, but it seems this series needs updating to accommodate
the recent addition of the blklogwrites block driver.
Best regards,
Ari Sundholm
ari@tuxera.com
>
> On 2018-06-28 02:07, Max Reitz wrote:
>> Once more, I’ll spare both me and you another iteration of the cover
>> letter, so see here:
>>
>> http://lists.nongnu.org/archive/html/qemu-block/2017-09/msg01030.html
>>
>> (Although this series no longer includes a @base-directory option.)
>>
>> In regards to the last version, the biggest change is that I dropped
>> backing_overridden and instead try to compare the filename from the
>> image header with the filename of the actual backing BDS to find out
>> whether the backing file has been overridden.
>>
>> In order that this doesn’t break whenever the header contains a slightly
>> unusual (“non-canonical”) backing filename (e.g. “file:foo.qcow2” or
>> “nbd:localhost:10809” instead of “nbd://localhost:10809”, i.e. something
>> different from what bdrv_refresh_filename() would generate), when the
>> reference filename in the BDS (auto_backing_file) is used to open the
>> backing file, it is updated from the backing BDS's resulting filename.
>>
>>
>> All (I hope) changes in v9:
>> - Rebase (warranting an NVME patch)
>>
>> - Dropped backing_overridden, switched to the comparison described above
>> (and dealt with the fallout, for example test 191 can now stay
>> unchanged)
>>
>> - Removed all existing bdrv_refresh_filename() calls and moved them to
>> the users of BDS.filename (first patch) -- otherwise, I got iotest
>> failure (because it's hard to reflect all graph changes properly with
>> a “pushing” bdrv_refresh_filename()), and I don't even want to
>> remember how 191 broke without this change.
>>
>> - Renamed “significant options” to “strong options”
>>
>> - Implicit nodes should be completely skipped in
>> bdrv_refresh_filename(), including setting of bs->full_open_options
>> (patch 3)
>>
>> - vmdk_gather_child_options() failed to set backing=null when the image
>> header asked for a backing file, but the user forbid its use
>>
>> - Added the penultimate patch which makes json:{} filenames for explicit
>> internal nodes kind of working?
>> (When you specify @filter-node e.g. for block-commit, the node should
>> be visible, so it may appear in a json:{} filename; but creating that
>> node did not take a real options QDict, so it was lacking the @driver
>> option, and that was a bit sad.)
>>
>> - Dropped a superfluous “suspend.” in blkdebug
>>
>> - Dropped the first patch of v8
>>
>> - Restyled some comments (“/*” on its own line where “*/” is on its own
>> line)
>>
>> - Fixed mention of "NBD" in the NFS patch (18)
>>
>>
>> git-backport-diff against v8:
>>
>> 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/31:[down] 'block: Use bdrv_refresh_filename() to pull'
>> 002/31:[----] [--] 'block: Use children list in bdrv_refresh_filename'
>> 003/31:[down] 'block: Skip implicit nodes for filename info'
>> 004/31:[down] 'block: Add BDS.auto_backing_file'
>> 005/31:[0052] [FC] 'block: Respect backing bs in bdrv_refresh_filename'
>> 006/31:[down] 'iotests.py: Add filter_imgfmt()'
>> 007/31:[down] 'iotests.py: Add node_info()'
>> 008/31:[down] 'iotests: Add test for backing file overrides'
>> 009/31:[----] [--] 'block: Make path_combine() return the path'
>> 010/31:[0016] [FC] 'block: bdrv_get_full_backing_filename_from_...'s ret. val.'
>> 011/31:[0001] [FC] 'block: bdrv_get_full_backing_filename's ret. val.'
>> 012/31:[0022] [FC] 'block: Add bdrv_make_absolute_filename()'
>> 013/31:[0003] [FC] 'block: Fix bdrv_find_backing_image()'
>> 014/31:[0001] [FC] 'block: Add bdrv_dirname()'
>> 015/31:[----] [--] 'blkverify: Make bdrv_dirname() return NULL'
>> 016/31:[----] [-C] 'quorum: Make bdrv_dirname() return NULL'
>> 017/31:[----] [-C] 'block/nbd: Make bdrv_dirname() return NULL'
>> 018/31:[0003] [FC] 'block/nfs: Implement bdrv_dirname()'
>> 019/31:[0014] [FC] 'block: Use bdrv_dirname() for relative filenames'
>> 020/31:[----] [--] 'iotests: Add quorum case to test 110'
>> 021/31:[down] 'block: Add strong_runtime_opts to BlockDriver'
>> 022/31:[0037] [FC] 'block: Add BlockDriver.bdrv_gather_child_options'
>> 023/31:[0084] [FC] 'block: Generically refresh runtime options'
>> 024/31:[0076] [FC] 'block: Purify .bdrv_refresh_filename()'
>> 025/31:[0005] [FC] 'block: Do not copy exact_filename from format file'
>> 026/31:[down] 'block/nvme: Fix bdrv_refresh_filename()'
>> 027/31:[----] [--] 'block/curl: Harmonize option defaults'
>> 028/31:[----] [-C] 'block/curl: Implement bdrv_refresh_filename()'
>> 029/31:[----] [--] 'block/null: Generate filename even with latency-ns'
>> 030/31:[down] 'block: BDS options may lack the "driver" option'
>> 031/31:[down] 'iotests: Test json:{} filenames of internal BDSs'
>>
>>
>> Max Reitz (31):
>> block: Use bdrv_refresh_filename() to pull
>> block: Use children list in bdrv_refresh_filename
>> block: Skip implicit nodes for filename info
>> block: Add BDS.auto_backing_file
>> block: Respect backing bs in bdrv_refresh_filename
>> iotests.py: Add filter_imgfmt()
>> iotests.py: Add node_info()
>> iotests: Add test for backing file overrides
>> block: Make path_combine() return the path
>> block: bdrv_get_full_backing_filename_from_...'s ret. val.
>> block: bdrv_get_full_backing_filename's ret. val.
>> block: Add bdrv_make_absolute_filename()
>> block: Fix bdrv_find_backing_image()
>> block: Add bdrv_dirname()
>> blkverify: Make bdrv_dirname() return NULL
>> quorum: Make bdrv_dirname() return NULL
>> block/nbd: Make bdrv_dirname() return NULL
>> block/nfs: Implement bdrv_dirname()
>> block: Use bdrv_dirname() for relative filenames
>> iotests: Add quorum case to test 110
>> block: Add strong_runtime_opts to BlockDriver
>> block: Add BlockDriver.bdrv_gather_child_options
>> block: Generically refresh runtime options
>> block: Purify .bdrv_refresh_filename()
>> block: Do not copy exact_filename from format file
>> block/nvme: Fix bdrv_refresh_filename()
>> block/curl: Harmonize option defaults
>> block/curl: Implement bdrv_refresh_filename()
>> block/null: Generate filename even with latency-ns
>> block: BDS options may lack the "driver" option
>> iotests: Test json:{} filenames of internal BDSs
>>
>> include/block/block.h | 16 +-
>> include/block/block_int.h | 48 +++-
>> block.c | 505 ++++++++++++++++++++++------------
>> block/blkdebug.c | 70 ++---
>> block/blkverify.c | 29 +-
>> block/commit.c | 3 +-
>> block/crypto.c | 8 +
>> block/curl.c | 55 +++-
>> block/gluster.c | 19 ++
>> block/iscsi.c | 18 ++
>> block/mirror.c | 3 +-
>> block/nbd.c | 46 ++--
>> block/nfs.c | 54 ++--
>> block/null.c | 32 ++-
>> block/nvme.c | 27 +-
>> block/qapi.c | 16 +-
>> block/qcow.c | 14 +-
>> block/qcow2.c | 17 +-
>> block/qed.c | 7 +-
>> block/quorum.c | 71 +++--
>> block/raw-format.c | 11 +-
>> block/rbd.c | 14 +
>> block/replication.c | 10 +-
>> block/sheepdog.c | 12 +
>> block/ssh.c | 12 +
>> block/throttle.c | 7 +
>> block/vhdx-log.c | 1 +
>> block/vmdk.c | 43 ++-
>> block/vpc.c | 11 +-
>> block/vvfat.c | 12 +
>> block/vxhs.c | 11 +
>> blockdev.c | 8 +
>> qemu-img.c | 12 +-
>> tests/qemu-iotests/051.out | 8 +-
>> tests/qemu-iotests/051.pc.out | 8 +-
>> tests/qemu-iotests/110 | 29 +-
>> tests/qemu-iotests/110.out | 9 +-
>> tests/qemu-iotests/223 | 235 ++++++++++++++++
>> tests/qemu-iotests/223.out | 84 ++++++
>> tests/qemu-iotests/224 | 139 ++++++++++
>> tests/qemu-iotests/224.out | 18 ++
>> tests/qemu-iotests/group | 2 +
>> tests/qemu-iotests/iotests.py | 10 +
>> 43 files changed, 1374 insertions(+), 390 deletions(-)
>> create mode 100755 tests/qemu-iotests/223
>> create mode 100644 tests/qemu-iotests/223.out
>> create mode 100755 tests/qemu-iotests/224
>> create mode 100644 tests/qemu-iotests/224.out
>>
>
>