Re: [Qemu-block] [Qemu-devel] [PATCH v3] iotests: Add test for dataplane mirroring

[Max Reitz]

On 2017-09-29 19:33, Eric Blake wrote:
> On 09/29/2017 12:08 PM, Max Reitz wrote:
>> Signed-off-by: Max Reitz <mre...@redhat.com>
>> ---
>> v3: Make the test actually pass (the reference output from v2 only
>> worked with v1 of the test...)
>> ---
>>  tests/qemu-iotests/127 | 97 
>> ++
>>  tests/qemu-iotests/127.out | 14 +++
>>  tests/qemu-iotests/group   |  1 +
>>  3 files changed, 112 insertions(+)
>>  create mode 100755 tests/qemu-iotests/127
>>  create mode 100644 tests/qemu-iotests/127.out
>>
> 
>> +_cleanup()
>> +{
>> +_cleanup_qemu
>> +_cleanup_test_img
>> +_rm_test_img "$TEST_IMG.overlay0"
>> +_rm_test_img "$TEST_IMG.overlay1"
>> +}
>> +trap "_cleanup; exit \$status" 0 1 2 3 15
> 
> Semantic conflict with Jeff's patches for './check -s' to preserve
> intermediate files.  Otherwise,
> Reviewed-by: Eric Blake <ebl...@redhat.com>

True, but as long as v4 of his series is not around, I feel OK to put
the burden of having to rebase on him. :-)

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH] block/mirror: check backing in bdrv_mirror_top_flush

[Max Reitz]

On 2017-09-29 17:22, Vladimir Sementsov-Ogievskiy wrote:
> Backing may be zero after failed bdrv_append in mirror_start_job,
> which leads to SIGSEGV.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
> 
> similar SIGSEGV.
> looks like (I guess by code, don't have full back-trace because of
> coroutine switch on bdrv_flush):
> mirror_start_job,
>   bdrv_append failed, backing is not set 
>   bdrv_unref
> bdrv_delete
>   bdrv_close
> bdrv_flush
>  ...
>  bdrv_mirror_top_flush 
>Segmentation fault on
>return bdrv_co_flush(bs->backing->bs);
>as bs->backing = 0
> 
>  block/mirror.c | 4 
>  1 file changed, 4 insertions(+)

Thanks, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

[Qemu-block] [PATCH v6 07/25] block: bdrv_get_full_backing_filename's ret. val.

[Max Reitz]

Make bdrv_get_full_backing_filename() return an allocated string instead
of placing the result in a caller-provided buffer.

Signed-off-by: Max Reitz <mre...@redhat.com>
---
 include/block/block.h |  3 +--
 block.c   | 47 +--
 block/qapi.c  | 12 ++--
 3 files changed, 20 insertions(+), 42 deletions(-)

diff --git a/include/block/block.h b/include/block/block.h
index 3002236967..65b15ecd18 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -482,8 +482,7 @@ void bdrv_round_to_clusters(BlockDriverState *bs,
 const char *bdrv_get_encrypted_filename(BlockDriverState *bs);
 void bdrv_get_backing_filename(BlockDriverState *bs,
char *filename, int filename_size);
-void bdrv_get_full_backing_filename(BlockDriverState *bs,
-char *dest, size_t sz, Error **errp);
+char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp);
 char *bdrv_get_full_backing_filename_from_filename(const char *backed,
const char *backing,
Error **errp);
diff --git a/block.c b/block.c
index 4ca56983bb..488abfbfc4 100644
--- a/block.c
+++ b/block.c
@@ -308,24 +308,13 @@ char *bdrv_get_full_backing_filename_from_filename(const 
char *backed,
 }
 }
 
-void bdrv_get_full_backing_filename(BlockDriverState *bs, char *dest, size_t 
sz,
-Error **errp)
+char *bdrv_get_full_backing_filename(BlockDriverState *bs, Error **errp)
 {
 char *backed = bs->exact_filename[0] ? bs->exact_filename : bs->filename;
-char *full_name;
-Error *local_error = NULL;
 
-full_name = bdrv_get_full_backing_filename_from_filename(backed,
- bs->backing_file,
- _error);
-if (full_name) {
-pstrcpy(dest, sz, full_name);
-g_free(full_name);
-} else if (local_error) {
-error_propagate(errp, local_error);
-} else if (sz > 0) {
-*dest = '\0';
-}
+return bdrv_get_full_backing_filename_from_filename(backed,
+bs->backing_file,
+errp);
 }
 
 void bdrv_register(BlockDriver *bdrv)
@@ -2189,7 +2178,7 @@ out:
 int bdrv_open_backing_file(BlockDriverState *bs, QDict *parent_options,
const char *bdref_key, Error **errp)
 {
-char *backing_filename = g_malloc0(PATH_MAX);
+char *backing_filename = NULL;
 char *bdref_key_dot;
 const char *reference = NULL;
 int ret = 0;
@@ -2223,7 +2212,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*parent_options,
  */
 reference = qdict_get_try_str(parent_options, bdref_key);
 if (reference || qdict_haskey(options, "file.filename")) {
-backing_filename[0] = '\0';
+/* keep backing_filename NULL */
 
 /* FIXME: Should also be set to true if @options contains other runtime
  *options which control the data that is read from the backing
@@ -2233,8 +,7 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*parent_options,
 QDECREF(options);
 goto free_exit;
 } else {
-bdrv_get_full_backing_filename(bs, backing_filename, PATH_MAX,
-   _err);
+backing_filename = bdrv_get_full_backing_filename(bs, _err);
 if (local_err) {
 ret = -EINVAL;
 error_propagate(errp, local_err);
@@ -2254,9 +2242,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict 
*parent_options,
 qdict_put_str(options, "driver", bs->backing_format);
 }
 
-backing_hd = bdrv_open_inherit(*backing_filename ? backing_filename : NULL,
-   reference, options, 0, bs, _backing,
-   errp);
+backing_hd = bdrv_open_inherit(backing_filename, reference, options, 0, bs,
+   _backing, errp);
 if (!backing_hd) {
 bs->open_flags |= BDRV_O_NO_BACKING;
 error_prepend(errp, "Could not open backing file: ");
@@ -4062,7 +4049,6 @@ BlockDriverState 
*bdrv_find_backing_image(BlockDriverState *bs,
 int is_protocol = 0;
 BlockDriverState *curr_bs = NULL;
 BlockDriverState *retval = NULL;
-Error *local_error = NULL;
 
 if (!bs || !bs->drv || !backing_file) {
 return NULL;
@@ -4079,21 +4065,22 @@ BlockDriverState 
*bdrv_find_backing_image(BlockDriverState *bs,
 /* If either of the filename paths is actually a protocol, then
  * compare unmodified paths; otherwise make paths relative */
 if (is_protocol || path_has_protocol(curr_bs->backing_file)) {
+

[Qemu-block] [PATCH v6 22/25] block: Do not copy exact_filename from format file

[Max Reitz]

If the a format BDS's file BDS is in turn a format BDS, we cannot simply
use the same filename, because when opening a BDS tree based on a
filename alone, qemu will create only one format node on top of one
protocol node (disregarding a potential backing file).

Signed-off-by: Max Reitz <mre...@redhat.com>
---
 block.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/block.c b/block.c
index 415f9c49f0..cfcbd3a09f 100644
--- a/block.c
+++ b/block.c
@@ -5034,9 +5034,20 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 
 bs->exact_filename[0] = '\0';
 
-/* If no specific options have been given for this BDS, the filename of
- * the underlying file should suffice for this one as well */
-if (bs->file->bs->exact_filename[0] && !generate_json_filename) {
+/* We can use the underlying file's filename if:
+ * - it has a filename,
+ * - the file is a protocol BDS, and
+ * - opening that file (as this BDS's format) will automatically create
+ *   the BDS tree we have right now, that is:
+ *   - the user did not significantly change this BDS's behavior with
+ * some explicit options
+ *   - no non-file child of this BDS has been overridden by the user
+ *   Both of these conditions are represented by 
generate_json_filename.
+ */
+if (bs->file->bs->exact_filename[0] &&
+bs->file->bs->drv->bdrv_file_open &&
+!generate_json_filename)
+{
 strcpy(bs->exact_filename, bs->file->bs->exact_filename);
 }
 }
-- 
2.13.6

[Qemu-block] [PATCH v6 04/25] block: Respect backing bs in bdrv_refresh_filename

[Max Reitz]

Basically, bdrv_refresh_filename() should respect all children of a
BlockDriverState. However, generally those children are driver-specific,
so this function cannot handle the general case. On the other hand,
there are only few drivers which use other children than @file and
@backing (that being vmdk, quorum, and blkverify).

Most block drivers only use @file and/or @backing (if they use any
children at all). Both can be implemented directly in
bdrv_refresh_filename.

The user overriding the file's filename is already handled, however, the
user overriding the backing file is not. If this is done, opening the
BDS with the plain filename of its file will not be correct, so we may
not set bs->exact_filename in that case.

iotest 051 contains test cases for overwriting the backing file, and so
its output changes with this patch applied (which I consider a good
thing).

Signed-off-by: Max Reitz <mre...@redhat.com>
Reviewed-by: Eric Blake <ebl...@redhat.com>
---
 block.c   | 12 +++-
 tests/qemu-iotests/051.out|  8 
 tests/qemu-iotests/051.pc.out |  8 
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/block.c b/block.c
index 95258316ec..ba24845960 100644
--- a/block.c
+++ b/block.c
@@ -4928,6 +4928,7 @@ void bdrv_refresh_filename(BlockDriverState *bs)
 
 opts = qdict_new();
 has_open_options = append_open_options(opts, bs);
+has_open_options |= bs->backing_overridden;
 
 /* If no specific options have been given for this BDS, the filename of
  * the underlying file should suffice for this one as well */
@@ -4939,11 +4940,20 @@ void bdrv_refresh_filename(BlockDriverState *bs)
  * file BDS. The full options QDict of that file BDS should somehow
  * contain a representation of the filename, therefore the following
  * suffices without querying the (exact_)filename of this BDS. */
-if (bs->file->bs->full_open_options) {
+if (bs->file->bs->full_open_options &&
+(!bs->backing || bs->backing->bs->full_open_options))
+{
 qdict_put_str(opts, "driver", drv->format_name);
 QINCREF(bs->file->bs->full_open_options);
 qdict_put(opts, "file", bs->file->bs->full_open_options);
 
+if (bs->backing) {
+QINCREF(bs->backing->bs->full_open_options);
+qdict_put(opts, "backing", bs->backing->bs->full_open_options);
+} else if (bs->backing_overridden && !bs->backing) {
+qdict_put(opts, "backing", qstring_new());
+}
+
 bs->full_open_options = opts;
 } else {
 QDECREF(opts);
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index e3c6eaba57..50d5cd07c8 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -59,7 +59,7 @@ QEMU X.Y.Z monitor - type 'help' for more information
 Testing: -drive 
file=TEST_DIR/t.qcow2,driver=qcow2,backing.file.filename=TEST_DIR/t.qcow2.orig,if=none,id=drive0
 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": 
"file", "filename": "TEST_DIR/t.qcow2.orig"}}, "driver": "qcow2", "file": 
{"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
 Removable device: not locked, tray closed
 Cache mode:   writeback
 Backing file: TEST_DIR/t.qcow2.orig (chain depth: 1)
@@ -148,7 +148,7 @@ QEMU_PROG: -drive driver=null-co,cache=invalid_value: 
invalid cache option
 Testing: -drive 
file=TEST_DIR/t.qcow2,cache=writeback,backing.file.filename=TEST_DIR/t.qcow2.base,backing.cache.no-flush=on,backing.node-name=backing,backing.file.node-name=backing-file,file.node-name=file,if=none,id=drive0
 -nodefaults
 QEMU X.Y.Z monitor - type 'help' for more information
 (qemu) info block
-drive0 (NODE_NAME): TEST_DIR/t.qcow2 (qcow2)
+drive0 (NODE_NAME): json:{"backing": {"driver": "qcow2", "file": {"driver": 
"file", "filename": "TEST_DIR/t.qcow2.base"}}, "driver": "qcow2", "file": 
{"driver": "file", "filename": "TEST_DIR/t.qcow2"}} (qcow2)
 Removable device: not locked, tray closed
 Cache mode:   writeback
 Backing file: TEST_DIR/t.qcow2.base (chain depth: 1)
@@ -168,7 +168,7 @@ backing-file: TEST_DIR/t.qcow2.base (file, read-only)
 Testing: -drive 
file=TEST_DIR/t.qcow2,cache=writethrough,backing.file.filename=TEST_DIR/t.qcow2.base,backi

[Qemu-block] [PATCH v6 11/25] blkverify: Make bdrv_dirname() return NULL

[Max Reitz]

blkverify's BDSs have a file BDS, but we do not want this to be
preferred over the raw node. There is no way to decide between the two
(and not really a reason to, either), so just return NULL in blkverify's
implementation of bdrv_dirname().

Signed-off-by: Max Reitz <mre...@redhat.com>
Reviewed-by: Eric Blake <ebl...@redhat.com>
---
 block/blkverify.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/block/blkverify.c b/block/blkverify.c
index b2ed8cd70d..d5233eeaf9 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -309,6 +309,15 @@ static void blkverify_refresh_filename(BlockDriverState 
*bs, QDict *options)
 }
 }
 
+static char *blkverify_dirname(BlockDriverState *bs, Error **errp)
+{
+/* In general, there are two BDSs with different dirnames below this one;
+ * so there is no unique dirname we could return (unless both are equal by
+ * chance). Therefore, to be consistent, just always return NULL. */
+error_setg(errp, "Cannot generate a base directory for blkverify nodes");
+return NULL;
+}
+
 static BlockDriver bdrv_blkverify = {
 .format_name  = "blkverify",
 .protocol_name= "blkverify",
@@ -320,6 +329,7 @@ static BlockDriver bdrv_blkverify = {
 .bdrv_child_perm  = bdrv_filter_default_perms,
 .bdrv_getlength   = blkverify_getlength,
 .bdrv_refresh_filename= blkverify_refresh_filename,
+.bdrv_dirname = blkverify_dirname,
 
 .bdrv_co_preadv   = blkverify_co_preadv,
 .bdrv_co_pwritev  = blkverify_co_pwritev,
-- 
2.13.6

[Qemu-block] [PATCH v6 02/25] block: Use children list in bdrv_refresh_filename

[Max Reitz]

bdrv_refresh_filename() should invoke itself recursively on all
children, not just on file.

With that change, we can remove the manual invocations in blkverify,
quorum, commit, and mirror.

Signed-off-by: Max Reitz <mre...@redhat.com>
Reviewed-by: Alberto Garcia <be...@igalia.com>
---
 block.c   | 9 +
 block/blkverify.c | 3 ---
 block/commit.c| 1 -
 block/mirror.c| 1 -
 block/quorum.c| 1 -
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/block.c b/block.c
index 5c65fac672..5721168d1d 100644
--- a/block.c
+++ b/block.c
@@ -4877,16 +4877,17 @@ static bool append_open_options(QDict *d, 
BlockDriverState *bs)
 void bdrv_refresh_filename(BlockDriverState *bs)
 {
 BlockDriver *drv = bs->drv;
+BdrvChild *child;
 QDict *opts;
 
 if (!drv) {
 return;
 }
 
-/* This BDS's file name will most probably depend on its file's name, so
- * refresh that first */
-if (bs->file) {
-bdrv_refresh_filename(bs->file->bs);
+/* This BDS's file name may depend on any of its children's file names, so
+ * refresh those first */
+QLIST_FOREACH(child, >children, next) {
+bdrv_refresh_filename(child->bs);
 }
 
 if (drv->bdrv_refresh_filename) {
diff --git a/block/blkverify.c b/block/blkverify.c
index 06369f9eac..b2ed8cd70d 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -281,9 +281,6 @@ static void blkverify_refresh_filename(BlockDriverState 
*bs, QDict *options)
 {
 BDRVBlkverifyState *s = bs->opaque;
 
-/* bs->file->bs has already been refreshed */
-bdrv_refresh_filename(s->test_file->bs);
-
 if (bs->file->bs->full_open_options
 && s->test_file->bs->full_open_options)
 {
diff --git a/block/commit.c b/block/commit.c
index 8f0e83578a..1df8db5428 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -246,7 +246,6 @@ static int coroutine_fn 
bdrv_commit_top_preadv(BlockDriverState *bs,
 
 static void bdrv_commit_top_refresh_filename(BlockDriverState *bs, QDict *opts)
 {
-bdrv_refresh_filename(bs->backing->bs);
 pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
 bs->backing->bs->filename);
 }
diff --git a/block/mirror.c b/block/mirror.c
index 25e9354b4d..7aa873d86f 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1078,7 +1078,6 @@ static void 
bdrv_mirror_top_refresh_filename(BlockDriverState *bs, QDict *opts)
  * bdrv_set_backing_hd */
 return;
 }
-bdrv_refresh_filename(bs->backing->bs);
 pstrcpy(bs->exact_filename, sizeof(bs->exact_filename),
 bs->backing->bs->filename);
 }
diff --git a/block/quorum.c b/block/quorum.c
index 272f9a5b77..2f1a628449 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -1074,7 +1074,6 @@ static void quorum_refresh_filename(BlockDriverState *bs, 
QDict *options)
 int i;
 
 for (i = 0; i < s->num_children; i++) {
-bdrv_refresh_filename(s->children[i]->bs);
 if (!s->children[i]->bs->full_open_options) {
 return;
 }
-- 
2.13.6

[Qemu-block] [PATCH v6 00/25] block: Fix some filename generation issues

[Max Reitz]

name() return NULL'
013/25:[] [--] 'block/nbd: Make bdrv_dirname() return NULL'
014/25:[] [--] 'block/nfs: Implement bdrv_dirname()'
015/25:[0010] [FC] 'block: Use bdrv_dirname() for relative filenames'
016/25:[] [--] 'block: Add 'base-directory' BDS option'
017/25:[] [--] 'iotests: Add quorum case to test 110'
018/25:[0004] [FC] 'block: Add sgfnt_runtime_opts to BlockDriver'
019/25:[] [--] 'block: Add BlockDriver.bdrv_gather_child_options'
020/25:[] [--] 'block: Generically refresh runtime options'
021/25:[] [-C] 'block: Purify .bdrv_refresh_filename()'
022/25:[] [--] 'block: Do not copy exact_filename from format file'
023/25:[] [-C] 'block: Fix FIXME from "Add BDS.backing_overridden"'
024/25:[] [--] 'block/curl: Implement bdrv_refresh_filename()'
025/25:[] [--] 'block/null: Generate filename even with latency-ns'


Max Reitz (25):
  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: Make bdrv_dirname() return NULL
  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: Fix FIXME from "Add BDS.backing_overridden"
  block/curl: Implement bdrv_refresh_filename()
  block/null: Generate filename even with latency-ns

 qapi/block-core.json  |   9 +
 include/block/block.h |  15 +-
 include/block/block_int.h |  36 ++-
 block.c   | 519 +-
 block/blkdebug.c  |  58 ++---
 block/blkverify.c |  29 +--
 block/commit.c|   3 +-
 block/crypto.c|   5 +
 block/curl.c  |  39 
 block/gluster.c   |  19 ++
 block/iscsi.c |  18 ++
 block/mirror.c|  19 +-
 block/nbd.c   |  46 ++--
 block/nfs.c   |  46 ++--
 block/null.c  |  33 ++-
 block/qapi.c  |  12 +-
 block/quorum.c|  58 +++--
 block/rbd.c   |   5 +
 block/replication.c   |   5 +
 block/sheepdog.c  |  12 +
 block/ssh.c   |   5 +
 block/throttle.c  |   4 +
 block/vmdk.c  |  25 +-
 block/vpc.c   |   4 +
 block/vvfat.c |   4 +
 block/vxhs.c  |   8 +
 blockdev.c|  16 ++
 tests/qemu-iotests/051.out|   8 +-
 tests/qemu-iotests/051.pc.out |   8 +-
 tests/qemu-iotests/110|  51 -
 tests/qemu-iotests/110.out|  14 +-
 31 files changed, 789 insertions(+), 344 deletions(-)

-- 
2.13.6

Re: [Qemu-block] [PATCH] block/mirror: check backing in bdrv_mirror_top_refresh_filename

[Max Reitz]

On 2017-09-28 14:03, Vladimir Sementsov-Ogievskiy wrote:
> Backing may be zero after failed bdrv_attach_child in
> bdrv_set_backing_hd, which leads to SIGSEGV.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---

Thanks, applied to my block branch:

https://github.com/XanClic/qemu/commits/block

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-block] [PATCH v3 2/2] qcow2: truncate the tail of the image file after shrinking the image

[Max Reitz]

On 2017-09-28 11:27, Pavel Butsykin wrote:
> Now after shrinking the image, at the end of the image file, there might be a
> tail that probably will never be used. So we can find the last used cluster 
> and
> cut the tail.
> 
> Signed-off-by: Pavel Butsykin 
> Reviewed-by: John Snow 
> ---
>  block/qcow2-refcount.c | 22 ++
>  block/qcow2.c  | 23 +++
>  block/qcow2.h  |  1 +
>  3 files changed, 46 insertions(+)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 88d5a3f1ad..aa3fd6cf17 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -3181,3 +3181,25 @@ out:
>  g_free(reftable_tmp);
>  return ret;
>  }
> +
> +int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size)
> +{
> +BDRVQcow2State *s = bs->opaque;
> +int64_t i;
> +
> +for (i = size_to_clusters(s, size) - 1; i >= 0; i--) {
> +uint64_t refcount;
> +int ret = qcow2_get_refcount(bs, i, );
> +if (ret < 0) {
> +fprintf(stderr, "Can't get refcount for cluster %" PRId64 ": 
> %s\n",
> +i, strerror(-ret));
> +return ret;
> +}
> +if (refcount > 0) {
> +return i;
> +}
> +}
> +qcow2_signal_corruption(bs, true, -1, -1,
> +"There are no references in the refcount 
> table.");
> +return -EIO;
> +}
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8a4311d338..f08c69ccd9 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3106,6 +3106,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t 
> offset,
>  new_l1_size = size_to_l1(s, offset);
>  
>  if (offset < old_length) {
> +int64_t last_cluster, old_file_size;
>  if (prealloc != PREALLOC_MODE_OFF) {
>  error_setg(errp,
> "Preallocation can't be used for shrinking an image");
> @@ -3134,6 +3135,28 @@ static int qcow2_truncate(BlockDriverState *bs, 
> int64_t offset,
>   "Failed to discard unused refblocks");
>  return ret;
>  }
> +
> +old_file_size = bdrv_getlength(bs->file->bs);
> +if (old_file_size < 0) {
> +error_setg_errno(errp, -old_file_size,
> + "Failed to inquire current file length");
> +return old_file_size;
> +}
> +last_cluster = qcow2_get_last_cluster(bs, old_file_size);
> +if (last_cluster < 0) {
> +error_setg_errno(errp, -last_cluster,
> + "Failed to find the last cluster");
> +return last_cluster;
> +}
> +if ((last_cluster + 1) * s->cluster_size < old_file_size) {
> +ret = bdrv_truncate(bs->file, (last_cluster + 1) * 
> s->cluster_size,
> +PREALLOC_MODE_OFF, NULL);
> +if (ret < 0) {
> +warn_report("Failed to truncate the tail of the image: "
> +"ret = %d", ret);

How about using strerror(-ret) instead?

Max

> +ret = 0;
> +}
> +}
>  } else {
>  ret = qcow2_grow_l1_table(bs, new_l1_size, true);
>  if (ret < 0) {
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 5a289a81e2..782a206ecb 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -597,6 +597,7 @@ int qcow2_change_refcount_order(BlockDriverState *bs, int 
> refcount_order,
>  BlockDriverAmendStatusCB *status_cb,
>  void *cb_opaque, Error **errp);
>  int qcow2_shrink_reftable(BlockDriverState *bs);
> +int64_t qcow2_get_last_cluster(BlockDriverState *bs, int64_t size);
>  
>  /* qcow2-cluster.c functions */
>  int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
> 




signature.asc
Description: OpenPGP digital signature