Re: [Qemu-block] [PATCH v6 30/42] qemu-img: Use child access functions
09.08.2019 19:13, Max Reitz wrote:
> This changes iotest 204's output, because blkdebug on top of a COW node
> used to make qemu-img map disregard the rest of the backing chain (the
> backing chain was broken by the filter). With this patch, the
> allocation in the base image is reported correctly.
>
> Signed-off-by: Max Reitz
> ---
> qemu-img.c | 33 -
> tests/qemu-iotests/204.out | 1 +
> 2 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 79983772de..3b30c5ae70 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1012,7 +1012,7 @@ static int img_commit(int argc, char **argv)
> /* This is different from QMP, which by default uses the deepest
> file in
>* the backing chain (i.e., the very base); however, the traditional
>* behavior of qemu-img commit is using the immediate backing file.
> */
> -base_bs = backing_bs(bs);
> +base_bs = bdrv_backing_chain_next(bs);
> if (!base_bs) {
> error_setg(&local_err, "Image does not have a backing file");
> goto done;
> @@ -1632,18 +1632,20 @@ static int convert_iteration_sectors(ImgConvertState
> *s, int64_t sector_num)
> if (s->sector_next_status <= sector_num) {
> uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
> int64_t count;
> +BlockDriverState *src_bs = blk_bs(s->src[src_cur]);
> +BlockDriverState *base;
> +
> +if (s->target_has_backing) {
> +base = bdrv_filtered_cow_bs(bdrv_skip_rw_filters(src_bs));
> +} else {
> +base = NULL;
> +}
>
> do {
> count = n * BDRV_SECTOR_SIZE;
>
> -if (s->target_has_backing) {
> -ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
> -count, &count, NULL, NULL);
> -} else {
> -ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
> - offset, count, &count, NULL,
> - NULL);
> -}
> +ret = bdrv_block_status_above(src_bs, base, offset, count,
> &count,
> + NULL, NULL);
>
> if (ret < 0) {
> if (s->salvage) {
> @@ -2490,7 +2492,8 @@ static int img_convert(int argc, char **argv)
>* s.target_backing_sectors has to be negative, which it will
>* be automatically). The backing file length is used only
>* for optimizations, so such a case is not fatal. */
> -s.target_backing_sectors = bdrv_nb_sectors(out_bs->backing->bs);
> +s.target_backing_sectors =
> +bdrv_nb_sectors(bdrv_filtered_cow_bs(out_bs));
bdrv_nb_sectors(bdrv_backing_chain_next(out_bs))
> } else {
> s.target_backing_sectors = -1;
> }
> @@ -2853,6 +2856,7 @@ static int get_block_status(BlockDriverState *bs,
> int64_t offset,
>
> depth = 0;
> for (;;) {
> +bs = bdrv_skip_rw_filters(bs);
> ret = bdrv_block_status(bs, offset, bytes, &bytes, &map, &file);
> if (ret < 0) {
> return ret;
> @@ -2861,7 +2865,7 @@ static int get_block_status(BlockDriverState *bs,
> int64_t offset,
> if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
> break;
> }
> -bs = backing_bs(bs);
> +bs = bdrv_filtered_cow_bs(bs);
> if (bs == NULL) {
> ret = 0;
> break;
> @@ -3216,6 +3220,7 @@ static int img_rebase(int argc, char **argv)
> uint8_t *buf_old = NULL;
> uint8_t *buf_new = NULL;
> BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
> +BlockDriverState *unfiltered_bs;
> char *filename;
> const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
> int c, flags, src_flags, ret;
> @@ -3350,6 +3355,8 @@ static int img_rebase(int argc, char **argv)
> }
> bs = blk_bs(blk);
>
> +unfiltered_bs = bdrv_skip_rw_filters(bs);
> +
> if (out_basefmt != NULL) {
> if (bdrv_find_format(out_basefmt) == NULL) {
> error_report("Invalid format name: '%s'", out_basefmt);
> @@ -3361,7 +3368,7 @@ static int img_rebase(int argc, char **argv)
> /* For safe rebasing we need to compare old and new backing file */
> if (!unsafe) {
> QDict *options = NULL;
> -BlockDriverState *base_bs = backing_bs(bs);
> +BlockDriverState *base_bs = bdrv_filtered_cow_bs(unfiltered_bs);
>
> if (base_bs) {
> blk_old_backing = blk_new(qemu_get_aio_context(),
> @@ -3517,7 +3524,7 @@ static int img_rebase(int argc, char **argv)
>* If cluster wasn't changed since prefix_chain, we don't
> need
>* to take action
>
Re: [Qemu-block] [PATCH v6 30/42] qemu-img: Use child access functions
On 12.08.19 14:14, Vladimir Sementsov-Ogievskiy wrote: > 09.08.2019 19:13, Max Reitz wrote: >> This changes iotest 204's output, because blkdebug on top of a COW node >> used to make qemu-img map disregard the rest of the backing chain (the >> backing chain was broken by the filter). With this patch, the >> allocation in the base image is reported correctly. >> >> Signed-off-by: Max Reitz >> --- >> qemu-img.c | 33 - >> tests/qemu-iotests/204.out | 1 + >> 2 files changed, 21 insertions(+), 13 deletions(-) >> >> diff --git a/qemu-img.c b/qemu-img.c >> index 79983772de..3b30c5ae70 100644 >> --- a/qemu-img.c >> +++ b/qemu-img.c [...] >> @@ -2490,7 +2492,8 @@ static int img_convert(int argc, char **argv) >>* s.target_backing_sectors has to be negative, which it will >>* be automatically). The backing file length is used only >>* for optimizations, so such a case is not fatal. */ >> -s.target_backing_sectors = bdrv_nb_sectors(out_bs->backing->bs); >> +s.target_backing_sectors = >> +bdrv_nb_sectors(bdrv_filtered_cow_bs(out_bs)); > > why not skip_rw_filters? It will fail if out_bs is filter.. Because I forgot this place. :-) Although backing_chain_next() would be simpler here and do the same, effectively. Max signature.asc Description: OpenPGP digital signature
Re: [Qemu-block] [PATCH v6 30/42] qemu-img: Use child access functions
09.08.2019 19:13, Max Reitz wrote:
> This changes iotest 204's output, because blkdebug on top of a COW node
> used to make qemu-img map disregard the rest of the backing chain (the
> backing chain was broken by the filter). With this patch, the
> allocation in the base image is reported correctly.
>
> Signed-off-by: Max Reitz
> ---
> qemu-img.c | 33 -
> tests/qemu-iotests/204.out | 1 +
> 2 files changed, 21 insertions(+), 13 deletions(-)
>
> diff --git a/qemu-img.c b/qemu-img.c
> index 79983772de..3b30c5ae70 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -1012,7 +1012,7 @@ static int img_commit(int argc, char **argv)
> /* This is different from QMP, which by default uses the deepest
> file in
>* the backing chain (i.e., the very base); however, the traditional
>* behavior of qemu-img commit is using the immediate backing file.
> */
> -base_bs = backing_bs(bs);
> +base_bs = bdrv_backing_chain_next(bs);
> if (!base_bs) {
> error_setg(&local_err, "Image does not have a backing file");
> goto done;
> @@ -1632,18 +1632,20 @@ static int convert_iteration_sectors(ImgConvertState
> *s, int64_t sector_num)
> if (s->sector_next_status <= sector_num) {
> uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
> int64_t count;
> +BlockDriverState *src_bs = blk_bs(s->src[src_cur]);
> +BlockDriverState *base;
> +
> +if (s->target_has_backing) {
> +base = bdrv_filtered_cow_bs(bdrv_skip_rw_filters(src_bs));
> +} else {
> +base = NULL;
> +}
>
> do {
> count = n * BDRV_SECTOR_SIZE;
>
> -if (s->target_has_backing) {
> -ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
> -count, &count, NULL, NULL);
> -} else {
> -ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
> - offset, count, &count, NULL,
> - NULL);
> -}
> +ret = bdrv_block_status_above(src_bs, base, offset, count,
> &count,
> + NULL, NULL);
>
> if (ret < 0) {
> if (s->salvage) {
> @@ -2490,7 +2492,8 @@ static int img_convert(int argc, char **argv)
>* s.target_backing_sectors has to be negative, which it will
>* be automatically). The backing file length is used only
>* for optimizations, so such a case is not fatal. */
> -s.target_backing_sectors = bdrv_nb_sectors(out_bs->backing->bs);
> +s.target_backing_sectors =
> +bdrv_nb_sectors(bdrv_filtered_cow_bs(out_bs));
why not skip_rw_filters? It will fail if out_bs is filter..
> } else {
> s.target_backing_sectors = -1;
> }
> @@ -2853,6 +2856,7 @@ static int get_block_status(BlockDriverState *bs,
> int64_t offset,
>
> depth = 0;
> for (;;) {
> +bs = bdrv_skip_rw_filters(bs);
> ret = bdrv_block_status(bs, offset, bytes, &bytes, &map, &file);
> if (ret < 0) {
> return ret;
> @@ -2861,7 +2865,7 @@ static int get_block_status(BlockDriverState *bs,
> int64_t offset,
> if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
> break;
> }
> -bs = backing_bs(bs);
> +bs = bdrv_filtered_cow_bs(bs);
> if (bs == NULL) {
> ret = 0;
> break;
> @@ -3216,6 +3220,7 @@ static int img_rebase(int argc, char **argv)
> uint8_t *buf_old = NULL;
> uint8_t *buf_new = NULL;
> BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
> +BlockDriverState *unfiltered_bs;
> char *filename;
> const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
> int c, flags, src_flags, ret;
> @@ -3350,6 +3355,8 @@ static int img_rebase(int argc, char **argv)
> }
> bs = blk_bs(blk);
>
> +unfiltered_bs = bdrv_skip_rw_filters(bs);
> +
> if (out_basefmt != NULL) {
> if (bdrv_find_format(out_basefmt) == NULL) {
> error_report("Invalid format name: '%s'", out_basefmt);
> @@ -3361,7 +3368,7 @@ static int img_rebase(int argc, char **argv)
> /* For safe rebasing we need to compare old and new backing file */
> if (!unsafe) {
> QDict *options = NULL;
> -BlockDriverState *base_bs = backing_bs(bs);
> +BlockDriverState *base_bs = bdrv_filtered_cow_bs(unfiltered_bs);
>
> if (base_bs) {
> blk_old_backing = blk_new(qemu_get_aio_context(),
> @@ -3517,7 +3524,7 @@ static int img_rebase(int argc, char **argv)
>* If cluster wasn't changed since prefix_chain, we don't
> need
>* to take a
[Qemu-block] [PATCH v6 30/42] qemu-img: Use child access functions
This changes iotest 204's output, because blkdebug on top of a COW node
used to make qemu-img map disregard the rest of the backing chain (the
backing chain was broken by the filter). With this patch, the
allocation in the base image is reported correctly.
Signed-off-by: Max Reitz
---
qemu-img.c | 33 -
tests/qemu-iotests/204.out | 1 +
2 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 79983772de..3b30c5ae70 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1012,7 +1012,7 @@ static int img_commit(int argc, char **argv)
/* This is different from QMP, which by default uses the deepest file
in
* the backing chain (i.e., the very base); however, the traditional
* behavior of qemu-img commit is using the immediate backing file. */
-base_bs = backing_bs(bs);
+base_bs = bdrv_backing_chain_next(bs);
if (!base_bs) {
error_setg(&local_err, "Image does not have a backing file");
goto done;
@@ -1632,18 +1632,20 @@ static int convert_iteration_sectors(ImgConvertState
*s, int64_t sector_num)
if (s->sector_next_status <= sector_num) {
uint64_t offset = (sector_num - src_cur_offset) * BDRV_SECTOR_SIZE;
int64_t count;
+BlockDriverState *src_bs = blk_bs(s->src[src_cur]);
+BlockDriverState *base;
+
+if (s->target_has_backing) {
+base = bdrv_filtered_cow_bs(bdrv_skip_rw_filters(src_bs));
+} else {
+base = NULL;
+}
do {
count = n * BDRV_SECTOR_SIZE;
-if (s->target_has_backing) {
-ret = bdrv_block_status(blk_bs(s->src[src_cur]), offset,
-count, &count, NULL, NULL);
-} else {
-ret = bdrv_block_status_above(blk_bs(s->src[src_cur]), NULL,
- offset, count, &count, NULL,
- NULL);
-}
+ret = bdrv_block_status_above(src_bs, base, offset, count, &count,
+ NULL, NULL);
if (ret < 0) {
if (s->salvage) {
@@ -2490,7 +2492,8 @@ static int img_convert(int argc, char **argv)
* s.target_backing_sectors has to be negative, which it will
* be automatically). The backing file length is used only
* for optimizations, so such a case is not fatal. */
-s.target_backing_sectors = bdrv_nb_sectors(out_bs->backing->bs);
+s.target_backing_sectors =
+bdrv_nb_sectors(bdrv_filtered_cow_bs(out_bs));
} else {
s.target_backing_sectors = -1;
}
@@ -2853,6 +2856,7 @@ static int get_block_status(BlockDriverState *bs, int64_t
offset,
depth = 0;
for (;;) {
+bs = bdrv_skip_rw_filters(bs);
ret = bdrv_block_status(bs, offset, bytes, &bytes, &map, &file);
if (ret < 0) {
return ret;
@@ -2861,7 +2865,7 @@ static int get_block_status(BlockDriverState *bs, int64_t
offset,
if (ret & (BDRV_BLOCK_ZERO|BDRV_BLOCK_DATA)) {
break;
}
-bs = backing_bs(bs);
+bs = bdrv_filtered_cow_bs(bs);
if (bs == NULL) {
ret = 0;
break;
@@ -3216,6 +3220,7 @@ static int img_rebase(int argc, char **argv)
uint8_t *buf_old = NULL;
uint8_t *buf_new = NULL;
BlockDriverState *bs = NULL, *prefix_chain_bs = NULL;
+BlockDriverState *unfiltered_bs;
char *filename;
const char *fmt, *cache, *src_cache, *out_basefmt, *out_baseimg;
int c, flags, src_flags, ret;
@@ -3350,6 +3355,8 @@ static int img_rebase(int argc, char **argv)
}
bs = blk_bs(blk);
+unfiltered_bs = bdrv_skip_rw_filters(bs);
+
if (out_basefmt != NULL) {
if (bdrv_find_format(out_basefmt) == NULL) {
error_report("Invalid format name: '%s'", out_basefmt);
@@ -3361,7 +3368,7 @@ static int img_rebase(int argc, char **argv)
/* For safe rebasing we need to compare old and new backing file */
if (!unsafe) {
QDict *options = NULL;
-BlockDriverState *base_bs = backing_bs(bs);
+BlockDriverState *base_bs = bdrv_filtered_cow_bs(unfiltered_bs);
if (base_bs) {
blk_old_backing = blk_new(qemu_get_aio_context(),
@@ -3517,7 +3524,7 @@ static int img_rebase(int argc, char **argv)
* If cluster wasn't changed since prefix_chain, we don't need
* to take action
*/
-ret = bdrv_is_allocated_above(backing_bs(bs), prefix_chain_bs,
+ret = bdrv_is_allocated_above(unfiltered_bs, prefix_chain_bs,
false, offset, n, &n);
if (ret < 0) {
error_report("error while reading image metadata: %s",
diff --git a/tests/qemu-i
