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.) 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 >