[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-block] [PATCH v2 0/5] block: Drop BDS.filename
From: |
Max Reitz |
Subject: |
[Qemu-block] [PATCH v2 0/5] block: Drop BDS.filename |
Date: |
Wed, 5 Aug 2015 22:52:44 +0200 |
The BDS filename field is generally only used when opening disk images
or emitting error or warning messages, the only exception to this rule
is the map command of qemu-img. However, using exact_filename there
instead should not be a problem. Therefore, we can drop the filename
field from the BlockDriverState and use a function instead which builds
the filename from scratch when called.
This is slower than reading a static char array but the problem of that
static array is that it may become obsolete due to changes in any
BlockDriverState or in the BDS graph. Using a function which rebuilds
the filename every time it is called resolves this problem.
The disadvantage of worse performance is negligible, on the other hand.
After patch 2 of this series, which replaces some queries of
BDS.filename by reads from somewhere else (mostly BDS.exact_filename),
the filename field is only used when a disk image is opened or some
message should be emitted, both of which cases do not suffer from the
performance hit.
http://lists.nongnu.org/archive/html/qemu-devel/2015-06/msg07025.html
gives an example of how to achieve an outdated filename, so we do need
this series. We can technically fix it differently (by appropriately
invoking bdrv_refresh_filename()), but that is pretty difficultâ„¢. I find
this series simpler and it it's something we wanted to do anyway.
Regarding the fear that this might change current behavior, especially
for libvirt which used filenames to identify nodes in the BDS graph:
Since bdrv_open() already calls bdrv_refresh_filename() today, and
apparently everything works fine, this series will most likely not break
anything. The only thing that will change is if the BDS graph is
dynamically reconfigured at runtime, and in that case it's most likely a
bug fix. Also, I don't think libvirt supports such cases already.
tl;dr: This series is a bug fix and I don't think it'll break legacy
management applications relying on the filename to identify a BDS; as
long as they are not trying to continue that practice with more advanced
configurations (e.g. with Quorum).
v2:
- Rebased on master
- Added patch 5: Removes a TODO from iotest 041 and thus demonstrates a
case where this series fixes something
git-backport-diff against v1:
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/5:[0015] [FC] 'block: Change bdrv_get_encrypted_filename()'
002/5:[0002] [FC] 'block: Avoid BlockDriverState.filename'
003/5:[----] [-C] 'block: Add bdrv_filename()'
004/5:[0038] [FC] 'block: Drop BlockDriverState.filename'
005/5:[down] 'iotests: Test changed Quorum filename'
Max Reitz (5):
block: Change bdrv_get_encrypted_filename()
block: Avoid BlockDriverState.filename
block: Add bdrv_filename()
block: Drop BlockDriverState.filename
iotests: Test changed Quorum filename
block.c | 91 ++++++++++++++++++++++++++++++++---------------
block/commit.c | 4 ++-
block/gluster.c | 2 +-
block/mirror.c | 14 +++++---
block/qapi.c | 5 +--
block/raw-posix.c | 16 +++++----
block/raw-win32.c | 4 +--
block/raw_bsd.c | 4 ++-
block/vhdx-log.c | 5 ++-
block/vmdk.c | 21 +++++++----
block/vpc.c | 6 ++--
blockdev.c | 21 ++++++++---
include/block/block.h | 3 +-
include/block/block_int.h | 1 -
monitor.c | 4 ++-
qemu-img.c | 2 +-
tests/qemu-iotests/041 | 17 ++++++---
17 files changed, 153 insertions(+), 67 deletions(-)
--
2.4.6
- [Qemu-block] [PATCH v2 0/5] block: Drop BDS.filename,
Max Reitz <=
- [Qemu-block] [PATCH v2 1/5] block: Change bdrv_get_encrypted_filename(), Max Reitz, 2015/08/05
- [Qemu-block] [PATCH v2 2/5] block: Avoid BlockDriverState.filename, Max Reitz, 2015/08/05
- [Qemu-block] [PATCH v2 3/5] block: Add bdrv_filename(), Max Reitz, 2015/08/05
- [Qemu-block] [PATCH v2 4/5] block: Drop BlockDriverState.filename, Max Reitz, 2015/08/05
- [Qemu-block] [PATCH v2 5/5] iotests: Test changed Quorum filename, Max Reitz, 2015/08/05
- Re: [Qemu-block] [PATCH v2 0/5] block: Drop BDS.filename, Stefan Hajnoczi, 2015/08/06