[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues
From: |
Max Reitz |
Subject: |
[Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues |
Date: |
Sun, 27 Nov 2016 02:55:58 +0100 |
Finally, a year after I (allegedly) started working on this, here is v2!
Rejoice!
There are some issues regarding filename generation right now:
- You always get a JSON filename if you set even a single qcow2-specific
runtime options (as long as it does not have a dot in it, which is a
bug, too, but here it is working in our favor...). That is not nice
and actually breaks the usage of backing files with relative
filenames with such qcow2 BDS.
- As hinted above, you cannot use relative backing filenames with BDS
that have a JSON filename only, even though qemu might be able to
obtain the directory name by walking through the BDS graph to the
protocol level.
- Overriding the backing file at runtime should invalidate the filename
because it actually changes the BDS's data. Therefore, we need to
force a JSON filename in that case, containing the backing file
override.
- Much of our code assumes paths never to exceed PATH_MAX in length.
This is wrong, at least because of JSON filenames. This should be
fixed wherever the opportunity arises.
- If a driver decides to implement bdrv_refresh_filename(), that
implementation has to not only refresh the filename (as one would
think) but it must also refresh the runtime options
(bs->full_open_options). That is stupid. (I'm allowed to say that
because I'm to blame for it.)
This series is enclosed by four patches (two at the front, two at the
back) that fix more or less general issues. They are included because:
- Patch 1 is required so that in patch 3 it's obvious why we don't need
to set backing_overriden there or call bdrv_refresh_filename()
- Patch 2 is already reviewed, so I might just as well keep it.
- Patches 23 and 24 are basically general bug fixes. Their connection to
this series is obvious, however, I think, and they depend on the rest
of the series, so I decided to just put them in.
Patches 3 and 4 address the third issue above.
Patches 5 to 9 address the fourth issue above, and are also necessary
preparation for the following patches.
Patches 10 to 16 address the second issue above, patch 17 adds a test
case. They implement a bdrv_dirname() function that returns the base
directory of a BDS by walking through the BDS graph to the protocol
layer and then trying to obtain a path based on that BDS's
exact_filename. This obviously fails if exact_filename on the protocol
layer is not set.
This behavior can be overriden either by any block driver along the way
implementing bdrv_dirname() itself or by the user through the new
'base-directory' node option. This may allow us to resolve relative
filenames even if the reference BDS only has a JSON filename.
Patches 18 to 22 address both the first and last issues above. They add
a field called "sgfnt_runtime_opts" to the BlockDriver structure. Block
drivers may point this to an array containing all of the runtime options
they accept that may change their BDS's data (i.e. that are
"significant"). bdrv_refresh_filename() will use this list to generate
bs->full_open_options itself (with only a little help by the block
driver, if necessary, through the .bdrv_gather_child_options()
function). This not only simplifies the process significantly, but also
results in the default implementation generating JSON filenames only
when really necessary.
v2: Much has been rewritten, so I cannot say much about differences to
v1.
Anyway, here is the backport-diff:
Key:
[----] : patches are identical
[####] : number of functional differences between v1/v2 patch
[down] : patch is v2-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/24:[down] 'block/mirror: Small absolute-paths simplification'
002/24:[----] [--] 'block: Use children list in bdrv_refresh_filename'
003/24:[0032] [FC] 'block: Add BDS.backing_overridden'
004/24:[0006] [FC] 'block: Respect backing bs in bdrv_refresh_filename'
005/24:[----] [-C] 'block: Make path_combine() return the path'
006/24:[----] [-C] 'block: bdrv_get_full_backing_filename_from_...'s ret. val.'
007/24:[0008] [FC] 'block: bdrv_get_full_backing_filename's ret. val.'
008/24:[----] [-C] 'block: Add bdrv_make_absolute_filename()'
009/24:[----] [--] 'block: Fix bdrv_find_backing_image()'
010/24:[0002] [FC] 'block: Add bdrv_dirname()'
011/24:[----] [--] 'blkverify: Make bdrv_dirname() return NULL'
012/24:[----] [--] 'quorum: Make bdrv_dirname() return NULL'
013/24:[0034] [FC] 'block/nbd: Implement bdrv_dirname()'
014/24:[down] 'block/nfs: Implement bdrv_dirname()'
015/24:[----] [--] 'block: Use bdrv_dirname() for relative filenames'
016/24:[0005] [FC] 'block: Add 'base-directory' BDS option'
017/24:[0009] [FC] 'iotests: Add quorum case to test 110'
018/24:[down] 'block: Add sgfnt_runtime_opts to BlockDriver'
019/24:[down] 'block: Add BlockDriver.bdrv_gather_child_options'
020/24:[down] 'block: Generically refresh runtime options'
021/24:[down] 'block: Purify .bdrv_refresh_filename()'
022/24:[down] 'block: Do not copy exact_filename from format file'
023/24:[down] 'block/curl: Implement bdrv_refresh_filename()'
024/24:[down] 'block/null: Generate filename even with latency-ns'
Now, at least comments for the functionally differing patches:
- Patch 3: Basically rewritten.
- Patch 4: Now uses qdict_put() instead of qdict_put_obj(); doesn't
really matter since that portion of code will be rewritten
later in the series anyway (patches 20+21).
- Patch 7: *Very* old rebase conflict.
- Patch 10: Not really a difference there.
- Patch 13: More or less rewritten (not least due to rebase conflicts).
- Patch 16: Now actually adhere to 80 characters/line, and s/2\.7/2.9/
- Patch 17: I think at some point in time John fixed the issue where you
couldn't use qemu-img info on images whose absolute backing
filename could not be determined. Anyway, it was broken in
v1 and it is fixed now, so the reference output changed.
Max Reitz (24):
block/mirror: Small absolute-paths simplification
block: Use children list in bdrv_refresh_filename
block: Add BDS.backing_overridden
block: Respect backing bs in bdrv_refresh_filename
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: Implement bdrv_dirname()
block/nfs: Implement bdrv_dirname()
block: Use bdrv_dirname() for relative filenames
block: Add 'base-directory' BDS option
iotests: Add quorum case to test 110
block: Add sgfnt_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/curl: Implement bdrv_refresh_filename()
block/null: Generate filename even with latency-ns
block.c | 421 ++++++++++++++++++++++++++----------------
block/blkdebug.c | 50 ++---
block/blkverify.c | 30 ++-
block/curl.c | 38 ++++
block/gluster.c | 19 ++
block/mirror.c | 10 +-
block/nbd.c | 54 ++++--
block/nfs.c | 50 ++---
block/null.c | 33 +++-
block/qapi.c | 12 +-
block/quorum.c | 62 ++++---
block/rbd.c | 2 +
block/vmdk.c | 24 ++-
block/vvfat.c | 4 +
blockdev.c | 16 ++
include/block/block.h | 15 +-
include/block/block_int.h | 14 +-
qapi/block-core.json | 9 +
tests/qemu-iotests/051.out | 8 +-
tests/qemu-iotests/051.pc.out | 8 +-
tests/qemu-iotests/110 | 51 ++++-
tests/qemu-iotests/110.out | 14 +-
22 files changed, 609 insertions(+), 335 deletions(-)
--
2.10.2
- [Qemu-devel] [PATCH v2 00/24] block: Fix some filename generation issues,
Max Reitz <=
- [Qemu-devel] [PATCH v2 01/24] block/mirror: Small absolute-paths simplification, Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 02/24] block: Use children list in bdrv_refresh_filename, Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 03/24] block: Add BDS.backing_overridden, Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 05/24] block: Make path_combine() return the path, Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 04/24] block: Respect backing bs in bdrv_refresh_filename, Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 06/24] block: bdrv_get_full_backing_filename_from_...'s ret. val., Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 07/24] block: bdrv_get_full_backing_filename's ret. val., Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 08/24] block: Add bdrv_make_absolute_filename(), Max Reitz, 2016/11/26
- [Qemu-devel] [PATCH v2 09/24] block: Fix bdrv_find_backing_image(), Max Reitz, 2016/11/26