From 9a6fc88799cc9a68d2821e49665ca50de75f399c Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Fri, 31 May 2019 15:23:11 +0200 Subject: block: Add child access functions There are BDS children that the general block layer code can access, namely bs->file and bs->backing. Since the introduction of filters and external data files, their meaning is not quite clear. bs->backing can be a COW source, or it can be a filtered child; bs->file can be a filtered child, it can be data and metadata storage, or it can be just metadata storage. This overloading really is not helpful. This patch adds functions that retrieve the correct child for each exact purpose. Later patches in this series will make use of them. Doing so will allow us to handle filter nodes in a meaningful way. Signed-off-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy --- include/block/block_int.h | 44 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 41 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/block/block_int.h b/include/block/block_int.h index 9da7a42927..f280a95b26 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -92,9 +92,17 @@ struct BlockDriver { int instance_size; /* set to true if the BlockDriver is a block filter. Block filters pass - * certain callbacks that refer to data (see block.c) to their bs->file if - * the driver doesn't implement them. Drivers that do not wish to forward - * must implement them and return -ENOTSUP. + * certain callbacks that refer to data (see block.c) to their bs->file + * or bs->backing (whichever one exists) if the driver doesn't implement + * them. Drivers that do not wish to forward must implement them and return + * -ENOTSUP. + * Note that filters are not allowed to modify data. + * + * Filters generally cannot have more than a single filtered child, + * because the data they present must at all times be the same as + * that on their filtered child. That would be impossible to + * achieve for multiple filtered children. + * (And this filtered child must then be bs->file or bs->backing.) */ bool is_filter; /* @@ -1382,4 +1390,34 @@ BdrvDirtyBitmap *block_dirty_bitmap_remove(const char *node, const char *name, BlockDriverState **bitmap_bs, Error **errp); +BdrvChild *bdrv_cow_child(BlockDriverState *bs); +BdrvChild *bdrv_filter_child(BlockDriverState *bs); +BdrvChild *bdrv_filter_or_cow_child(BlockDriverState *bs); +BdrvChild *bdrv_primary_child(BlockDriverState *bs); + +static inline BlockDriverState *child_bs(BdrvChild *child) +{ + return child ? child->bs : NULL; +} + +static inline BlockDriverState *bdrv_cow_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_cow_child(bs)); +} + +static inline BlockDriverState *bdrv_filter_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_filter_child(bs)); +} + +static inline BlockDriverState *bdrv_filter_or_cow_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_filter_or_cow_child(bs)); +} + +static inline BlockDriverState *bdrv_primary_bs(BlockDriverState *bs) +{ + return child_bs(bdrv_primary_child(bs)); +} + #endif /* BLOCK_INT_H */ -- cgit v1.2.3 From d38d7eb8a5e78ecc1906e763f59356a0b26a8b53 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 12 Jun 2019 15:06:37 +0200 Subject: block: Add chain helper functions Add some helper functions for skipping filters in a chain of block nodes. Signed-off-by: Max Reitz --- include/block/block_int.h | 3 +++ 1 file changed, 3 insertions(+) (limited to 'include') diff --git a/include/block/block_int.h b/include/block/block_int.h index f280a95b26..8205ccaa62 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1394,6 +1394,9 @@ BdrvChild *bdrv_cow_child(BlockDriverState *bs); BdrvChild *bdrv_filter_child(BlockDriverState *bs); BdrvChild *bdrv_filter_or_cow_child(BlockDriverState *bs); BdrvChild *bdrv_primary_child(BlockDriverState *bs); +BlockDriverState *bdrv_skip_implicit_filters(BlockDriverState *bs); +BlockDriverState *bdrv_skip_filters(BlockDriverState *bs); +BlockDriverState *bdrv_backing_chain_next(BlockDriverState *bs); static inline BlockDriverState *child_bs(BdrvChild *child) { -- cgit v1.2.3 From 8b8277cdb05eef79bfe6b14c21dfaa027cebbd08 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 12 Jun 2019 17:08:20 +0200 Subject: block: Drop bdrv_is_encrypted() The original purpose of bdrv_is_encrypted() was to inquire whether a BDS can be used without the user entering a password or not. It has not been used for that purpose for quite some time. Actually, it is not even fit for that purpose, because to answer that question, it would have recursively query all of the given node's children. So now we have to decide in which direction we want to fix bdrv_is_encrypted(): Recursively query all children, or drop it and just use bs->encrypted to get the current node's status? Nowadays, its only purpose is to report through bdrv_query_image_info() whether the given image is encrypted or not. For this purpose, it is probably more interesting to see whether a given node itself is encrypted or not (otherwise, a management application cannot discern for certain which nodes are really encrypted and which just have encrypted children). Suggested-by: Vladimir Sementsov-Ogievskiy Signed-off-by: Max Reitz Reviewed-by: Andrey Shinkevich Reviewed-by: Kevin Wolf --- include/block/block.h | 1 - 1 file changed, 1 deletion(-) (limited to 'include') diff --git a/include/block/block.h b/include/block/block.h index 6e36154061..2c09b93d07 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -532,7 +532,6 @@ BlockDriverState *bdrv_next(BdrvNextIterator *it); void bdrv_next_cleanup(BdrvNextIterator *it); BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs); -bool bdrv_is_encrypted(BlockDriverState *bs); void bdrv_iterate_format(void (*it)(void *opaque, const char *name), void *opaque, bool read_only); const char *bdrv_get_node_name(const BlockDriverState *bs); -- cgit v1.2.3 From ae23f78646e9635ec9cbf15dbf82a717ea1b0e69 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 12 Jun 2019 22:57:15 +0200 Subject: block: Add bdrv_supports_compressed_writes() Filters cannot compress data themselves but they have to implement .bdrv_co_pwritev_compressed() still (or they cannot forward compressed writes). Therefore, checking whether bs->drv->bdrv_co_pwritev_compressed is non-NULL is not sufficient to know whether the node can actually handle compressed writes. This function looks down the filter chain to see whether there is a non-filter that can actually convert the compressed writes into compressed data (and thus normal writes). Signed-off-by: Max Reitz Reviewed-by: Andrey Shinkevich Reviewed-by: Kevin Wolf --- include/block/block.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/block/block.h b/include/block/block.h index 2c09b93d07..981ab5b314 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -532,6 +532,7 @@ BlockDriverState *bdrv_next(BdrvNextIterator *it); void bdrv_next_cleanup(BdrvNextIterator *it); BlockDriverState *bdrv_next_monitor_owned(BlockDriverState *bs); +bool bdrv_supports_compressed_writes(BlockDriverState *bs); void bdrv_iterate_format(void (*it)(void *opaque, const char *name), void *opaque, bool read_only); const char *bdrv_get_node_name(const BlockDriverState *bs); -- cgit v1.2.3 From f1a7f18f07ef41b4d078621bd6e515165106d91f Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Mon, 1 Apr 2019 22:16:14 +0200 Subject: block: Drop backing_bs() We want to make it explicit where bs->backing is used, and we have done so. The old role of backing_bs() is now effectively taken by bdrv_cow_bs(). Signed-off-by: Max Reitz Reviewed-by: Vladimir Sementsov-Ogievskiy Reviewed-by: Andrey Shinkevich Reviewed-by: Kevin Wolf --- include/block/block_int.h | 5 ----- 1 file changed, 5 deletions(-) (limited to 'include') diff --git a/include/block/block_int.h b/include/block/block_int.h index 8205ccaa62..bb3d1c8a8c 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1003,11 +1003,6 @@ typedef enum BlockMirrorBackingMode { MIRROR_LEAVE_BACKING_CHAIN, } BlockMirrorBackingMode; -static inline BlockDriverState *backing_bs(BlockDriverState *bs) -{ - return bs->backing ? bs->backing->bs : NULL; -} - /* Essential block drivers which must always be statically linked into qemu, and * which therefore can be accessed without using bdrv_find_format() */ -- cgit v1.2.3 From 549ec0d9783e1a94e502e677fe0ce4a90c0bd7df Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 13 Feb 2019 18:05:23 +0100 Subject: block: Inline bdrv_co_block_status_from_*() With bdrv_filter_bs(), we can easily handle this default filter behavior in bdrv_co_block_status(). blkdebug wants to have an additional assertion, so it keeps its own implementation, except bdrv_co_block_status_from_file() needs to be inlined there. Suggested-by: Eric Blake Signed-off-by: Max Reitz Reviewed-by: Andrey Shinkevich Reviewed-by: Kevin Wolf --- include/block/block_int.h | 22 ---------------------- 1 file changed, 22 deletions(-) (limited to 'include') diff --git a/include/block/block_int.h b/include/block/block_int.h index bb3d1c8a8c..8b9d769e14 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -1303,28 +1303,6 @@ void bdrv_default_perms(BlockDriverState *bs, BdrvChild *c, uint64_t perm, uint64_t shared, uint64_t *nperm, uint64_t *nshared); -/* - * Default implementation for drivers to pass bdrv_co_block_status() to - * their file. - */ -int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs, - bool want_zero, - int64_t offset, - int64_t bytes, - int64_t *pnum, - int64_t *map, - BlockDriverState **file); -/* - * Default implementation for drivers to pass bdrv_co_block_status() to - * their backing file. - */ -int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs, - bool want_zero, - int64_t offset, - int64_t bytes, - int64_t *pnum, - int64_t *map, - BlockDriverState **file); const char *bdrv_get_parent_name(const BlockDriverState *bs); void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp); bool blk_dev_has_removable_media(BlockBackend *blk); -- cgit v1.2.3 From 0b877d09df4557900149384d60be0e9c3d309105 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 1 Aug 2018 20:34:11 +0200 Subject: block: Leave BDS.backing_{file,format} constant Parts of the block layer treat BDS.backing_file as if it were whatever the image header says (i.e., if it is a relative path, it is relative to the overlay), other parts treat it like a cache for bs->backing->bs->filename (relative paths are relative to the CWD). Considering bs->backing->bs->filename exists, let us make it mean the former. Among other things, this now allows the user to specify a base when using qemu-img to commit an image file in a directory that is not the CWD (assuming, everything uses relative filenames). Before this patch: $ ./qemu-img create -f qcow2 foo/bot.qcow2 1M $ ./qemu-img create -f qcow2 -b bot.qcow2 foo/mid.qcow2 $ ./qemu-img create -f qcow2 -b mid.qcow2 foo/top.qcow2 $ ./qemu-img commit -b mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find '[...]/foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' After this patch: $ ./qemu-img commit -b mid.qcow2 foo/top.qcow2 Image committed. $ ./qemu-img commit -b foo/mid.qcow2 foo/top.qcow2 qemu-img: Did not find 'foo/mid.qcow2' in the backing chain of 'foo/top.qcow2' $ ./qemu-img commit -b $PWD/foo/mid.qcow2 foo/top.qcow2 Image committed. With this change, bdrv_find_backing_image() must look at whether the user has overridden a BDS's backing file. If so, it can no longer use bs->backing_file, but must instead compare the given filename against the backing node's filename directly. Note that this changes the QAPI output for a node's backing_file. We had very inconsistent output there (sometimes what the image header said, sometimes the actual filename of the backing image). This inconsistent output was effectively useless, so we have to decide one way or the other. Considering that bs->backing_file usually at runtime contained the path to the image relative to qemu's CWD (or absolute), this patch changes QAPI's backing_file to always report the bs->backing->bs->filename from now on. If you want to receive the image header information, you have to refer to full-backing-filename. This necessitates a change to iotest 228. The interesting information it really wanted is the image header, and it can get that now, but it has to use full-backing-filename instead of backing_file. Because of this patch's changes to bs->backing_file's behavior, we also need some reference output changes. Along with the changes to bs->backing_file, stop updating BDS.backing_format in bdrv_backing_attach() as well. This way, ImageInfo's backing-filename and backing-filename-format fields will represent what the image header says and nothing else. iotest 245 changes in behavior: With the backing node no longer overriding the parent node's backing_file string, you can now omit the @backing option when reopening a node with neither a default nor a current backing file even if it used to have a backing node at some point. 273 also changes: The base image is opened without a format layer, so ImageInfo.backing-filename-format used to report "file" for the base image's overlay after blockdev-snapshot. However, the image header never says "file" anywhere, so it now reports $IMGFMT. Signed-off-by: Max Reitz --- include/block/block_int.h | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'include') diff --git a/include/block/block_int.h b/include/block/block_int.h index 8b9d769e14..38cad9d15c 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -847,11 +847,20 @@ struct BlockDriverState { bool walking_aio_notifiers; /* to make removal during iteration safe */ char filename[PATH_MAX]; - char backing_file[PATH_MAX]; /* if non zero, the image is a diff of - this file image */ - /* The backing filename indicated by the image header; if we ever - * open this file, then this is replaced by the resulting BDS's - * filename (i.e. after a bdrv_refresh_filename() run). */ + /* + * If not empty, this image is a diff in relation to backing_file. + * Note that this is the name given in the image header and + * therefore may or may not be equal to .backing->bs->filename. + * If this field contains a relative path, it is to be resolved + * relatively to the overlay's location. + */ + char backing_file[PATH_MAX]; + /* + * The backing filename indicated by the image header. Contrary + * to backing_file, if we ever open this file, auto_backing_file + * is replaced by the resulting BDS's filename (i.e. after a + * bdrv_refresh_filename() run). + */ char auto_backing_file[PATH_MAX]; char backing_format[16]; /* if non-zero and backing_file exists */ @@ -1053,6 +1062,8 @@ BlockDriver *bdrv_probe_all(const uint8_t *buf, int buf_size, void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix, QDict *options); +bool bdrv_backing_overridden(BlockDriverState *bs); + /** * bdrv_add_before_write_notifier: -- cgit v1.2.3