From 55b9392b98e500399f2da1edc1d110bbfd40fb05 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 28 Mar 2017 22:51:26 +0200 Subject: [PATCH 01/12] block/vhdx: Make vhdx_create() always set errp This patch makes vhdx_create() always set errp in case of an error. It also adds errp parameters to vhdx_create_bat() and vhdx_create_new_region_table() so we can pass on the error object generated by blk_truncate() as of a future commit. Signed-off-by: Max Reitz Reviewed-by: Kevin Wolf Message-id: 20170328205129.15138-2-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- block/vhdx.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/block/vhdx.c b/block/vhdx.c index 052a753159..d25bcd91de 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1586,7 +1586,7 @@ exit: static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s, uint64_t image_size, VHDXImageType type, bool use_zero_blocks, uint64_t file_offset, - uint32_t length) + uint32_t length, Error **errp) { int ret = 0; uint64_t data_file_offset; @@ -1609,14 +1609,19 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s, * is the furthest thing we have written yet */ ret = blk_truncate(blk, data_file_offset); if (ret < 0) { + error_setg_errno(errp, -ret, + "Failed to resize the underlying file"); goto exit; } } else if (type == VHDX_TYPE_FIXED) { ret = blk_truncate(blk, data_file_offset + image_size); if (ret < 0) { + error_setg_errno(errp, -ret, + "Failed to resize the underlying file"); goto exit; } } else { + error_setg(errp, "Unsupported image type"); ret = -ENOTSUP; goto exit; } @@ -1627,6 +1632,7 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s, /* for a fixed file, the default BAT entry is not zero */ s->bat = g_try_malloc0(length); if (length && s->bat == NULL) { + error_setg(errp, "Failed to allocate memory for the BAT"); ret = -ENOMEM; goto exit; } @@ -1646,6 +1652,7 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s, } ret = blk_pwrite(blk, file_offset, s->bat, length, 0); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to write the BAT"); goto exit; } } @@ -1671,7 +1678,8 @@ static int vhdx_create_new_region_table(BlockBackend *blk, uint32_t log_size, bool use_zero_blocks, VHDXImageType type, - uint64_t *metadata_offset) + uint64_t *metadata_offset, + Error **errp) { int ret = 0; uint32_t offset = 0; @@ -1740,7 +1748,7 @@ static int vhdx_create_new_region_table(BlockBackend *blk, /* The region table gives us the data we need to create the BAT, * so do that now */ ret = vhdx_create_bat(blk, s, image_size, type, use_zero_blocks, - bat_file_offset, bat_length); + bat_file_offset, bat_length, errp); if (ret < 0) { goto exit; } @@ -1749,12 +1757,14 @@ static int vhdx_create_new_region_table(BlockBackend *blk, ret = blk_pwrite(blk, VHDX_REGION_TABLE_OFFSET, buffer, VHDX_HEADER_BLOCK_SIZE, 0); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to write first region table"); goto exit; } ret = blk_pwrite(blk, VHDX_REGION_TABLE2_OFFSET, buffer, VHDX_HEADER_BLOCK_SIZE, 0); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to write second region table"); goto exit; } @@ -1825,6 +1835,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp) ret = -ENOTSUP; goto exit; } else { + error_setg(errp, "Invalid subformat '%s'", type); ret = -EINVAL; goto exit; } @@ -1879,12 +1890,14 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp) ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET, &signature, sizeof(signature), 0); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to write file signature"); goto delete_and_exit; } if (creator) { ret = blk_pwrite(blk, VHDX_FILE_ID_OFFSET + sizeof(signature), creator, creator_items * sizeof(gunichar2), 0); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to write creator field"); goto delete_and_exit; } } @@ -1893,13 +1906,14 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp) /* Creates (B),(C) */ ret = vhdx_create_new_headers(blk, image_size, log_size); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to write image headers"); goto delete_and_exit; } /* Creates (D),(E),(G) explicitly. (F) created as by-product */ ret = vhdx_create_new_region_table(blk, image_size, block_size, 512, log_size, use_zero_blocks, image_type, - &metadata_offset); + &metadata_offset, errp); if (ret < 0) { goto delete_and_exit; } @@ -1908,6 +1922,7 @@ static int vhdx_create(const char *filename, QemuOpts *opts, Error **errp) ret = vhdx_create_new_metadata(blk, image_size, block_size, 512, metadata_offset, image_type); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to initialize metadata"); goto delete_and_exit; } From ed3d2ec98a33fbdeabc471b11ff807075f07e996 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 28 Mar 2017 22:51:27 +0200 Subject: [PATCH 02/12] block: Add errp to b{lk,drv}_truncate() For one thing, this allows us to drop the error message generation from qemu-img.c and blockdev.c and instead have it unified in bdrv_truncate(). Signed-off-by: Max Reitz Message-id: 20170328205129.15138-3-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- block.c | 16 ++++++++++++---- block/blkdebug.c | 2 +- block/block-backend.c | 5 +++-- block/commit.c | 5 +++-- block/crypto.c | 2 +- block/mirror.c | 2 +- block/parallels.c | 13 ++++++++----- block/qcow.c | 6 +++--- block/qcow2-refcount.c | 5 ++++- block/qcow2.c | 14 +++++++++----- block/qed.c | 2 +- block/raw-format.c | 2 +- block/vdi.c | 4 ++-- block/vhdx-log.c | 2 +- block/vhdx.c | 10 +++------- block/vmdk.c | 13 +++---------- block/vpc.c | 13 +++++++------ blockdev.c | 21 +-------------------- include/block/block.h | 2 +- include/sysemu/block-backend.h | 2 +- qemu-img.c | 17 ++++------------- qemu-io-cmds.c | 5 +++-- 22 files changed, 73 insertions(+), 90 deletions(-) diff --git a/block.c b/block.c index 7b557f3e04..6a1937ead3 100644 --- a/block.c +++ b/block.c @@ -3307,7 +3307,7 @@ exit: /** * Truncate file to 'offset' bytes (needed only for file protocols) */ -int bdrv_truncate(BdrvChild *child, int64_t offset) +int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp) { BlockDriverState *bs = child->bs; BlockDriver *drv = bs->drv; @@ -3315,12 +3315,18 @@ int bdrv_truncate(BdrvChild *child, int64_t offset) assert(child->perm & BLK_PERM_RESIZE); - if (!drv) + if (!drv) { + error_setg(errp, "No medium inserted"); return -ENOMEDIUM; - if (!drv->bdrv_truncate) + } + if (!drv->bdrv_truncate) { + error_setg(errp, "Image format driver does not support resize"); return -ENOTSUP; - if (bs->read_only) + } + if (bs->read_only) { + error_setg(errp, "Image is read-only"); return -EACCES; + } ret = drv->bdrv_truncate(bs, offset); if (ret == 0) { @@ -3328,6 +3334,8 @@ int bdrv_truncate(BdrvChild *child, int64_t offset) bdrv_dirty_bitmap_truncate(bs); bdrv_parent_cb_resize(bs); ++bs->write_gen; + } else { + error_setg_errno(errp, -ret, "Failed to resize image"); } return ret; } diff --git a/block/blkdebug.c b/block/blkdebug.c index cc4a146e84..9bd066e0df 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -661,7 +661,7 @@ static int64_t blkdebug_getlength(BlockDriverState *bs) static int blkdebug_truncate(BlockDriverState *bs, int64_t offset) { - return bdrv_truncate(bs->file, offset); + return bdrv_truncate(bs->file, offset, NULL); } static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options) diff --git a/block/block-backend.c b/block/block-backend.c index ae3d7713ce..f5bf13eec9 100644 --- a/block/block-backend.c +++ b/block/block-backend.c @@ -1746,13 +1746,14 @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf, BDRV_REQ_WRITE_COMPRESSED); } -int blk_truncate(BlockBackend *blk, int64_t offset) +int blk_truncate(BlockBackend *blk, int64_t offset, Error **errp) { if (!blk_is_available(blk)) { + error_setg(errp, "No medium inserted"); return -ENOMEDIUM; } - return bdrv_truncate(blk->root, offset); + return bdrv_truncate(blk->root, offset, errp); } static void blk_pdiscard_entry(void *opaque) diff --git a/block/commit.c b/block/commit.c index 91d2c344f6..76a0d98c6f 100644 --- a/block/commit.c +++ b/block/commit.c @@ -151,7 +151,7 @@ static void coroutine_fn commit_run(void *opaque) } if (base_len < s->common.len) { - ret = blk_truncate(s->base, s->common.len); + ret = blk_truncate(s->base, s->common.len, NULL); if (ret) { goto out; } @@ -511,8 +511,9 @@ int bdrv_commit(BlockDriverState *bs) * grow the backing file image if possible. If not possible, * we must return an error */ if (length > backing_length) { - ret = blk_truncate(backing, length); + ret = blk_truncate(backing, length, &local_err); if (ret < 0) { + error_report_err(local_err); goto ro_cleanup; } } diff --git a/block/crypto.c b/block/crypto.c index 34549b28a5..7ad64b5f93 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -389,7 +389,7 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset) offset += payload_offset; - return bdrv_truncate(bs->file, offset); + return bdrv_truncate(bs->file, offset, NULL); } static void block_crypto_close(BlockDriverState *bs) diff --git a/block/mirror.c b/block/mirror.c index 9f5eb692fd..e86f8f8ad7 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -724,7 +724,7 @@ static void coroutine_fn mirror_run(void *opaque) } if (s->bdev_length > base_length) { - ret = blk_truncate(s->target, s->bdev_length); + ret = blk_truncate(s->target, s->bdev_length, NULL); if (ret < 0) { goto immediate_exit; } diff --git a/block/parallels.c b/block/parallels.c index 90acf79687..8be46a7d48 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -223,7 +223,8 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num, space << BDRV_SECTOR_BITS, 0); } else { ret = bdrv_truncate(bs->file, - (s->data_end + space) << BDRV_SECTOR_BITS); + (s->data_end + space) << BDRV_SECTOR_BITS, + NULL); } if (ret < 0) { return ret; @@ -456,8 +457,10 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res, size - res->image_end_offset); res->leaks += count; if (fix & BDRV_FIX_LEAKS) { - ret = bdrv_truncate(bs->file, res->image_end_offset); + Error *local_err = NULL; + ret = bdrv_truncate(bs->file, res->image_end_offset, &local_err); if (ret < 0) { + error_report_err(local_err); res->check_errors++; return ret; } @@ -504,7 +507,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp) blk_set_allow_write_beyond_eof(file, true); - ret = blk_truncate(file, 0); + ret = blk_truncate(file, 0, errp); if (ret < 0) { goto exit; } @@ -696,7 +699,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags, } if (!(flags & BDRV_O_RESIZE) || !bdrv_has_zero_init(bs->file->bs) || - bdrv_truncate(bs->file, bdrv_getlength(bs->file->bs)) != 0) { + bdrv_truncate(bs->file, bdrv_getlength(bs->file->bs), NULL) != 0) { s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE; } @@ -739,7 +742,7 @@ static void parallels_close(BlockDriverState *bs) } if (bs->open_flags & BDRV_O_RDWR) { - bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS); + bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, NULL); } g_free(s->bat_dirty_bmap); diff --git a/block/qcow.c b/block/qcow.c index 9d6ac83959..5d147b962e 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -473,7 +473,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, /* round to cluster size */ cluster_offset = (cluster_offset + s->cluster_size - 1) & ~(s->cluster_size - 1); - bdrv_truncate(bs->file, cluster_offset + s->cluster_size); + bdrv_truncate(bs->file, cluster_offset + s->cluster_size, NULL); /* if encrypted, we must initialize the cluster content which won't be written */ if (bs->encrypted && @@ -833,7 +833,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp) blk_set_allow_write_beyond_eof(qcow_blk, true); - ret = blk_truncate(qcow_blk, 0); + ret = blk_truncate(qcow_blk, 0, errp); if (ret < 0) { goto exit; } @@ -916,7 +916,7 @@ static int qcow_make_empty(BlockDriverState *bs) if (bdrv_pwrite_sync(bs->file, s->l1_table_offset, s->l1_table, l1_length) < 0) return -1; - ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length); + ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length, NULL); if (ret < 0) return ret; diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 9e96f64c8b..4efca7ebdb 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1728,14 +1728,17 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res, if (fix & BDRV_FIX_ERRORS) { int64_t new_nb_clusters; + Error *local_err = NULL; if (offset > INT64_MAX - s->cluster_size) { ret = -EINVAL; goto resize_fail; } - ret = bdrv_truncate(bs->file, offset + s->cluster_size); + ret = bdrv_truncate(bs->file, offset + s->cluster_size, + &local_err); if (ret < 0) { + error_report_err(local_err); goto resize_fail; } size = bdrv_getlength(bs->file->bs); diff --git a/block/qcow2.c b/block/qcow2.c index 6a92d2ef3f..845eee4bd9 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2294,9 +2294,9 @@ static int qcow2_create2(const char *filename, int64_t total_size, } /* Okay, now that we have a valid image, let's give it the right size */ - ret = blk_truncate(blk, total_size); + ret = blk_truncate(blk, total_size, errp); if (ret < 0) { - error_setg_errno(errp, -ret, "Could not resize image"); + error_prepend(errp, "Could not resize image: "); goto out; } @@ -2584,7 +2584,7 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset, /* align end of file to a sector boundary to ease reading with sector based I/Os */ cluster_offset = bdrv_getlength(bs->file->bs); - return bdrv_truncate(bs->file, cluster_offset); + return bdrv_truncate(bs->file, cluster_offset, NULL); } buf = qemu_blockalign(bs, s->cluster_size); @@ -2674,6 +2674,7 @@ fail: static int make_completely_empty(BlockDriverState *bs) { BDRVQcow2State *s = bs->opaque; + Error *local_err = NULL; int ret, l1_clusters; int64_t offset; uint64_t *new_reftable = NULL; @@ -2798,8 +2799,10 @@ static int make_completely_empty(BlockDriverState *bs) goto fail; } - ret = bdrv_truncate(bs->file, (3 + l1_clusters) * s->cluster_size); + ret = bdrv_truncate(bs->file, (3 + l1_clusters) * s->cluster_size, + &local_err); if (ret < 0) { + error_report_err(local_err); goto fail; } @@ -3273,9 +3276,10 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts, return ret; } - ret = blk_truncate(blk, new_size); + ret = blk_truncate(blk, new_size, &local_err); blk_unref(blk); if (ret < 0) { + error_report_err(local_err); return ret; } } diff --git a/block/qed.c b/block/qed.c index 5ec7fd83f2..53199ac8d7 100644 --- a/block/qed.c +++ b/block/qed.c @@ -635,7 +635,7 @@ static int qed_create(const char *filename, uint32_t cluster_size, blk_set_allow_write_beyond_eof(blk, true); /* File must start empty and grow, check truncate is supported */ - ret = blk_truncate(blk, 0); + ret = blk_truncate(blk, 0, errp); if (ret < 0) { goto out; } diff --git a/block/raw-format.c b/block/raw-format.c index 86fbc657eb..a80073369a 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -341,7 +341,7 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset) s->size = offset; offset += s->offset; - return bdrv_truncate(bs->file, offset); + return bdrv_truncate(bs->file, offset, NULL); } static int raw_media_changed(BlockDriverState *bs) diff --git a/block/vdi.c b/block/vdi.c index 9b4f70e977..d12d9cdc79 100644 --- a/block/vdi.c +++ b/block/vdi.c @@ -832,9 +832,9 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp) } if (image_type == VDI_TYPE_STATIC) { - ret = blk_truncate(blk, offset + blocks * block_size); + ret = blk_truncate(blk, offset + blocks * block_size, errp); if (ret < 0) { - error_setg(errp, "Failed to statically allocate %s", filename); + error_prepend(errp, "Failed to statically allocate %s", filename); goto exit; } } diff --git a/block/vhdx-log.c b/block/vhdx-log.c index 67a91c0de5..3f4c2aa095 100644 --- a/block/vhdx-log.c +++ b/block/vhdx-log.c @@ -548,7 +548,7 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s, if (new_file_size % (1024*1024)) { /* round up to nearest 1MB boundary */ new_file_size = ((new_file_size >> 20) + 1) << 20; - bdrv_truncate(bs->file, new_file_size); + bdrv_truncate(bs->file, new_file_size, NULL); } } qemu_vfree(desc_entries); diff --git a/block/vhdx.c b/block/vhdx.c index d25bcd91de..e8fe3fb5e9 100644 --- a/block/vhdx.c +++ b/block/vhdx.c @@ -1171,7 +1171,7 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s, /* per the spec, the address for a block is in units of 1MB */ *new_offset = ROUND_UP(*new_offset, 1024 * 1024); - return bdrv_truncate(bs->file, *new_offset + s->block_size); + return bdrv_truncate(bs->file, *new_offset + s->block_size, NULL); } /* @@ -1607,17 +1607,13 @@ static int vhdx_create_bat(BlockBackend *blk, BDRVVHDXState *s, if (type == VHDX_TYPE_DYNAMIC) { /* All zeroes, so we can just extend the file - the end of the BAT * is the furthest thing we have written yet */ - ret = blk_truncate(blk, data_file_offset); + ret = blk_truncate(blk, data_file_offset, errp); if (ret < 0) { - error_setg_errno(errp, -ret, - "Failed to resize the underlying file"); goto exit; } } else if (type == VHDX_TYPE_FIXED) { - ret = blk_truncate(blk, data_file_offset + image_size); + ret = blk_truncate(blk, data_file_offset + image_size, errp); if (ret < 0) { - error_setg_errno(errp, -ret, - "Failed to resize the underlying file"); goto exit; } } else { diff --git a/block/vmdk.c b/block/vmdk.c index a9bd22bf93..c61b9cc8e0 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1714,10 +1714,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, blk_set_allow_write_beyond_eof(blk, true); if (flat) { - ret = blk_truncate(blk, filesize); - if (ret < 0) { - error_setg_errno(errp, -ret, "Could not truncate file"); - } + ret = blk_truncate(blk, filesize, errp); goto exit; } magic = cpu_to_be32(VMDK4_MAGIC); @@ -1780,9 +1777,8 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, goto exit; } - ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9); + ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9, errp); if (ret < 0) { - error_setg_errno(errp, -ret, "Could not truncate file"); goto exit; } @@ -2090,10 +2086,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp) /* bdrv_pwrite write padding zeros to align to sector, we don't need that * for description file */ if (desc_offset == 0) { - ret = blk_truncate(new_blk, desc_len); - if (ret < 0) { - error_setg_errno(errp, -ret, "Could not truncate file"); - } + ret = blk_truncate(new_blk, desc_len, errp); } exit: if (new_blk) { diff --git a/block/vpc.c b/block/vpc.c index f591d4be38..ecfee77149 100644 --- a/block/vpc.c +++ b/block/vpc.c @@ -851,20 +851,21 @@ static int create_dynamic_disk(BlockBackend *blk, uint8_t *buf, } static int create_fixed_disk(BlockBackend *blk, uint8_t *buf, - int64_t total_size) + int64_t total_size, Error **errp) { int ret; /* Add footer to total size */ total_size += HEADER_SIZE; - ret = blk_truncate(blk, total_size); + ret = blk_truncate(blk, total_size, errp); if (ret < 0) { return ret; } ret = blk_pwrite(blk, total_size - HEADER_SIZE, buf, HEADER_SIZE, 0); if (ret < 0) { + error_setg_errno(errp, -ret, "Unable to write VHD header"); return ret; } @@ -996,11 +997,11 @@ static int vpc_create(const char *filename, QemuOpts *opts, Error **errp) if (disk_type == VHD_DYNAMIC) { ret = create_dynamic_disk(blk, buf, total_sectors); + if (ret < 0) { + error_setg(errp, "Unable to create or write VHD header"); + } } else { - ret = create_fixed_disk(blk, buf, total_size); - } - if (ret < 0) { - error_setg(errp, "Unable to create or write VHD header"); + ret = create_fixed_disk(blk, buf, total_size, errp); } out: diff --git a/blockdev.c b/blockdev.c index 64282065d8..4d8cdedd54 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2930,26 +2930,7 @@ void qmp_block_resize(bool has_device, const char *device, /* complete all in-flight operations before resizing the device */ bdrv_drain_all(); - ret = blk_truncate(blk, size); - switch (ret) { - case 0: - break; - case -ENOMEDIUM: - error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device); - break; - case -ENOTSUP: - error_setg(errp, QERR_UNSUPPORTED); - break; - case -EACCES: - error_setg(errp, "Device '%s' is read only", device); - break; - case -EBUSY: - error_setg(errp, QERR_DEVICE_IN_USE, device); - break; - default: - error_setg_errno(errp, -ret, "Could not resize"); - break; - } + ret = blk_truncate(blk, size, errp); out: blk_unref(blk); diff --git a/include/block/block.h b/include/block/block.h index 144df0ddfb..862eb56fc7 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -294,7 +294,7 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs, const char *backing_file); int bdrv_get_backing_file_depth(BlockDriverState *bs); void bdrv_refresh_filename(BlockDriverState *bs); -int bdrv_truncate(BdrvChild *child, int64_t offset); +int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp); int64_t bdrv_nb_sectors(BlockDriverState *bs); int64_t bdrv_getlength(BlockDriverState *bs); int64_t bdrv_get_allocated_file_size(BlockDriverState *bs); diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h index 133e542f20..840ad6134c 100644 --- a/include/sysemu/block-backend.h +++ b/include/sysemu/block-backend.h @@ -225,7 +225,7 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset, int count, BdrvRequestFlags flags); int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf, int count); -int blk_truncate(BlockBackend *blk, int64_t offset); +int blk_truncate(BlockBackend *blk, int64_t offset, Error **errp); int blk_pdiscard(BlockBackend *blk, int64_t offset, int count); int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf, int64_t pos, int size); diff --git a/qemu-img.c b/qemu-img.c index 704488484d..9eb82830f7 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -3464,20 +3464,11 @@ static int img_resize(int argc, char **argv) goto out; } - ret = blk_truncate(blk, total_size); - switch (ret) { - case 0: + ret = blk_truncate(blk, total_size, &err); + if (!ret) { qprintf(quiet, "Image resized.\n"); - break; - case -ENOTSUP: - error_report("This image does not support resize"); - break; - case -EACCES: - error_report("Image is read-only"); - break; - default: - error_report("Error resizing image: %s", strerror(-ret)); - break; + } else { + error_report_err(err); } out: blk_unref(blk); diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c index 312fc6d157..21af9e65b2 100644 --- a/qemu-io-cmds.c +++ b/qemu-io-cmds.c @@ -1567,6 +1567,7 @@ static const cmdinfo_t flush_cmd = { static int truncate_f(BlockBackend *blk, int argc, char **argv) { + Error *local_err = NULL; int64_t offset; int ret; @@ -1576,9 +1577,9 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv) return 0; } - ret = blk_truncate(blk, offset); + ret = blk_truncate(blk, offset, &local_err); if (ret < 0) { - printf("truncate: %s\n", strerror(-ret)); + error_report_err(local_err); return 0; } From 4bff28b81a0ddb48a09d279e99ab847e8a292506 Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 28 Mar 2017 22:51:28 +0200 Subject: [PATCH 03/12] block: Add errp to BD.bdrv_truncate() Add an Error parameter to the block drivers' bdrv_truncate() interface. If a block driver does not set this in case of an error, the generic bdrv_truncate() implementation will do so. Where it is obvious, this patch also makes some block drivers set this value. Signed-off-by: Max Reitz Message-id: 20170328205129.15138-4-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- block.c | 4 ++-- block/blkdebug.c | 4 ++-- block/crypto.c | 5 +++-- block/file-posix.c | 2 +- block/file-win32.c | 6 +++--- block/gluster.c | 3 ++- block/iscsi.c | 4 ++-- block/nfs.c | 2 +- block/qcow2.c | 8 ++++---- block/qed.c | 2 +- block/raw-format.c | 4 ++-- block/rbd.c | 2 +- block/sheepdog.c | 14 ++++++-------- include/block/block_int.h | 2 +- 14 files changed, 31 insertions(+), 31 deletions(-) diff --git a/block.c b/block.c index 6a1937ead3..ff232a2a20 100644 --- a/block.c +++ b/block.c @@ -3328,13 +3328,13 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp) return -EACCES; } - ret = drv->bdrv_truncate(bs, offset); + ret = drv->bdrv_truncate(bs, offset, errp); if (ret == 0) { ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); bdrv_dirty_bitmap_truncate(bs); bdrv_parent_cb_resize(bs); ++bs->write_gen; - } else { + } else if (errp && !*errp) { error_setg_errno(errp, -ret, "Failed to resize image"); } return ret; diff --git a/block/blkdebug.c b/block/blkdebug.c index 9bd066e0df..d2a7561c4c 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -659,9 +659,9 @@ static int64_t blkdebug_getlength(BlockDriverState *bs) return bdrv_getlength(bs->file->bs); } -static int blkdebug_truncate(BlockDriverState *bs, int64_t offset) +static int blkdebug_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { - return bdrv_truncate(bs->file, offset, NULL); + return bdrv_truncate(bs->file, offset, errp); } static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options) diff --git a/block/crypto.c b/block/crypto.c index 7ad64b5f93..6828180840 100644 --- a/block/crypto.c +++ b/block/crypto.c @@ -381,7 +381,8 @@ static int block_crypto_create_generic(QCryptoBlockFormat format, return ret; } -static int block_crypto_truncate(BlockDriverState *bs, int64_t offset) +static int block_crypto_truncate(BlockDriverState *bs, int64_t offset, + Error **errp) { BlockCrypto *crypto = bs->opaque; size_t payload_offset = @@ -389,7 +390,7 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset) offset += payload_offset; - return bdrv_truncate(bs->file, offset, NULL); + return bdrv_truncate(bs->file, offset, errp); } static void block_crypto_close(BlockDriverState *bs) diff --git a/block/file-posix.c b/block/file-posix.c index ade71db0e2..9c0d70170d 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1407,7 +1407,7 @@ static void raw_close(BlockDriverState *bs) } } -static int raw_truncate(BlockDriverState *bs, int64_t offset) +static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { BDRVRawState *s = bs->opaque; struct stat st; diff --git a/block/file-win32.c b/block/file-win32.c index e132ba17da..7872e00a21 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -460,7 +460,7 @@ static void raw_close(BlockDriverState *bs) } } -static int raw_truncate(BlockDriverState *bs, int64_t offset) +static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { BDRVRawState *s = bs->opaque; LONG low, high; @@ -475,11 +475,11 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset) */ dwPtrLow = SetFilePointer(s->hfile, low, &high, FILE_BEGIN); if (dwPtrLow == INVALID_SET_FILE_POINTER && GetLastError() != NO_ERROR) { - fprintf(stderr, "SetFilePointer error: %lu\n", GetLastError()); + error_setg_win32(errp, GetLastError(), "SetFilePointer error"); return -EIO; } if (SetEndOfFile(s->hfile) == 0) { - fprintf(stderr, "SetEndOfFile error: %lu\n", GetLastError()); + error_setg_win32(errp, GetLastError(), "SetEndOfFile error"); return -EIO; } return 0; diff --git a/block/gluster.c b/block/gluster.c index cf29b5f9a4..be45c08b4c 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -1092,7 +1092,8 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs, return acb.ret; } -static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset) +static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset, + Error **errp) { int ret; BDRVGlusterState *s = bs->opaque; diff --git a/block/iscsi.c b/block/iscsi.c index 42fb0b019c..1ef38cf3d0 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -2059,7 +2059,7 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state) } } -static int iscsi_truncate(BlockDriverState *bs, int64_t offset) +static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { IscsiLun *iscsilun = bs->opaque; Error *local_err = NULL; @@ -2070,7 +2070,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset) iscsi_readcapacity_sync(iscsilun, &local_err); if (local_err != NULL) { - error_free(local_err); + error_propagate(errp, local_err); return -EIO; } diff --git a/block/nfs.c b/block/nfs.c index 6541dec1fc..5ae665abd4 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -764,7 +764,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs) return (task.ret < 0 ? task.ret : st.st_blocks * 512); } -static int nfs_file_truncate(BlockDriverState *bs, int64_t offset) +static int nfs_file_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { NFSClient *client = bs->opaque; return nfs_ftruncate(client->context, client->fh, offset); diff --git a/block/qcow2.c b/block/qcow2.c index 845eee4bd9..6c347989e3 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2525,26 +2525,26 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs, return ret; } -static int qcow2_truncate(BlockDriverState *bs, int64_t offset) +static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { BDRVQcow2State *s = bs->opaque; int64_t new_l1_size; int ret; if (offset & 511) { - error_report("The new size must be a multiple of 512"); + error_setg(errp, "The new size must be a multiple of 512"); return -EINVAL; } /* cannot proceed if image has snapshots */ if (s->nb_snapshots) { - error_report("Can't resize an image which has snapshots"); + error_setg(errp, "Can't resize an image which has snapshots"); return -ENOTSUP; } /* shrinking is currently not supported */ if (offset < bs->total_sectors * 512) { - error_report("qcow2 doesn't support shrinking images yet"); + error_setg(errp, "qcow2 doesn't support shrinking images yet"); return -ENOTSUP; } diff --git a/block/qed.c b/block/qed.c index 53199ac8d7..fa2aeee471 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1518,7 +1518,7 @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs, return cb.ret; } -static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset) +static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { BDRVQEDState *s = bs->opaque; uint64_t old_image_size; diff --git a/block/raw-format.c b/block/raw-format.c index a80073369a..9761bdaff8 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -327,7 +327,7 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) } } -static int raw_truncate(BlockDriverState *bs, int64_t offset) +static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { BDRVRawState *s = bs->opaque; @@ -341,7 +341,7 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset) s->size = offset; offset += s->offset; - return bdrv_truncate(bs->file, offset, NULL); + return bdrv_truncate(bs->file, offset, errp); } static int raw_media_changed(BlockDriverState *bs) diff --git a/block/rbd.c b/block/rbd.c index 6471f4fd2b..3e6e73e989 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -916,7 +916,7 @@ static int64_t qemu_rbd_getlength(BlockDriverState *bs) return info.size; } -static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset) +static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { BDRVRBDState *s = bs->opaque; int r; diff --git a/block/sheepdog.c b/block/sheepdog.c index b2a5998188..fe8fd923d5 100644 --- a/block/sheepdog.c +++ b/block/sheepdog.c @@ -2159,9 +2159,8 @@ static int64_t sd_getlength(BlockDriverState *bs) return s->inode.vdi_size; } -static int sd_truncate(BlockDriverState *bs, int64_t offset) +static int sd_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { - Error *local_err = NULL; BDRVSheepdogState *s = bs->opaque; int ret, fd; unsigned int datalen; @@ -2169,16 +2168,15 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) max_vdi_size = (UINT64_C(1) << s->inode.block_size_shift) * MAX_DATA_OBJS; if (offset < s->inode.vdi_size) { - error_report("shrinking is not supported"); + error_setg(errp, "shrinking is not supported"); return -EINVAL; } else if (offset > max_vdi_size) { - error_report("too big image size"); + error_setg(errp, "too big image size"); return -EINVAL; } - fd = connect_to_sdog(s, &local_err); + fd = connect_to_sdog(s, errp); if (fd < 0) { - error_report_err(local_err); return fd; } @@ -2191,7 +2189,7 @@ static int sd_truncate(BlockDriverState *bs, int64_t offset) close(fd); if (ret < 0) { - error_report("failed to update an inode."); + error_setg_errno(errp, -ret, "failed to update an inode"); } return ret; @@ -2456,7 +2454,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num, BDRVSheepdogState *s = bs->opaque; if (offset > s->inode.vdi_size) { - ret = sd_truncate(bs, offset); + ret = sd_truncate(bs, offset, NULL); if (ret < 0) { return ret; } diff --git a/include/block/block_int.h b/include/block/block_int.h index 4f8cd29ae4..dcde90a5d3 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -196,7 +196,7 @@ struct BlockDriver { int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs); const char *protocol_name; - int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset); + int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset, Error **errp); int64_t (*bdrv_getlength)(BlockDriverState *bs); bool has_variable_length; From f59adb325618a2d2ba16aece551e7ab6acadb0ae Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Tue, 28 Mar 2017 22:51:29 +0200 Subject: [PATCH 04/12] block: Add .bdrv_truncate() error messages Add missing error messages for the block driver implementations of .bdrv_truncate(); drop the generic one from block.c's bdrv_truncate(). Since one of these changes touches a mis-indented block in block/file-posix.c, this patch fixes that coding style issue along the way. Signed-off-by: Max Reitz Message-id: 20170328205129.15138-5-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- block.c | 2 -- block/file-posix.c | 17 ++++++++++++----- block/gluster.c | 4 +++- block/iscsi.c | 2 ++ block/nfs.c | 10 +++++++++- block/qcow2.c | 2 ++ block/qed.c | 4 +++- block/raw-format.c | 2 ++ block/rbd.c | 1 + 9 files changed, 34 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index ff232a2a20..76bf00f4b2 100644 --- a/block.c +++ b/block.c @@ -3334,8 +3334,6 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp) bdrv_dirty_bitmap_truncate(bs); bdrv_parent_cb_resize(bs); ++bs->write_gen; - } else if (errp && !*errp) { - error_setg_errno(errp, -ret, "Failed to resize image"); } return ret; } diff --git a/block/file-posix.c b/block/file-posix.c index 9c0d70170d..1941fb6749 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -1411,20 +1411,27 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { BDRVRawState *s = bs->opaque; struct stat st; + int ret; if (fstat(s->fd, &st)) { - return -errno; + ret = -errno; + error_setg_errno(errp, -ret, "Failed to fstat() the file"); + return ret; } if (S_ISREG(st.st_mode)) { if (ftruncate(s->fd, offset) < 0) { - return -errno; + ret = -errno; + error_setg_errno(errp, -ret, "Failed to resize the file"); + return ret; } } else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) { - if (offset > raw_getlength(bs)) { - return -EINVAL; - } + if (offset > raw_getlength(bs)) { + error_setg(errp, "Cannot grow device files"); + return -EINVAL; + } } else { + error_setg(errp, "Resizing this file is not supported"); return -ENOTSUP; } diff --git a/block/gluster.c b/block/gluster.c index be45c08b4c..1d4e2f7c52 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -1100,7 +1100,9 @@ static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset, ret = glfs_ftruncate(s->fd, offset); if (ret < 0) { - return -errno; + ret = -errno; + error_setg_errno(errp, -ret, "Failed to truncate file"); + return ret; } return 0; diff --git a/block/iscsi.c b/block/iscsi.c index 1ef38cf3d0..5daa201181 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -2065,6 +2065,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp) Error *local_err = NULL; if (iscsilun->type != TYPE_DISK) { + error_setg(errp, "Cannot resize non-disk iSCSI devices"); return -ENOTSUP; } @@ -2075,6 +2076,7 @@ static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp) } if (offset > iscsi_getlength(bs)) { + error_setg(errp, "Cannot grow iSCSI devices"); return -EINVAL; } diff --git a/block/nfs.c b/block/nfs.c index 5ae665abd4..76572ae546 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -767,7 +767,15 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs) static int nfs_file_truncate(BlockDriverState *bs, int64_t offset, Error **errp) { NFSClient *client = bs->opaque; - return nfs_ftruncate(client->context, client->fh, offset); + int ret; + + ret = nfs_ftruncate(client->context, client->fh, offset); + if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to truncate file"); + return ret; + } + + return 0; } /* Note that this will not re-establish a connection with the NFS server diff --git a/block/qcow2.c b/block/qcow2.c index 6c347989e3..4ca4cf04b0 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2551,6 +2551,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp) new_l1_size = size_to_l1(s, offset); ret = qcow2_grow_l1_table(bs, new_l1_size, true); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to grow the L1 table"); return ret; } @@ -2559,6 +2560,7 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp) ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size), &offset, sizeof(uint64_t)); if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to update the image size"); return ret; } diff --git a/block/qed.c b/block/qed.c index fa2aeee471..fd76817cbb 100644 --- a/block/qed.c +++ b/block/qed.c @@ -1526,11 +1526,12 @@ static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp) if (!qed_is_image_size_valid(offset, s->header.cluster_size, s->header.table_size)) { + error_setg(errp, "Invalid image size specified"); return -EINVAL; } - /* Shrinking is currently not supported */ if ((uint64_t)offset < s->header.image_size) { + error_setg(errp, "Shrinking images is currently not supported"); return -ENOTSUP; } @@ -1539,6 +1540,7 @@ static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp) ret = qed_write_header_sync(s); if (ret < 0) { s->header.image_size = old_image_size; + error_setg_errno(errp, -ret, "Failed to update the image size"); } return ret; } diff --git a/block/raw-format.c b/block/raw-format.c index 9761bdaff8..36e65036f0 100644 --- a/block/raw-format.c +++ b/block/raw-format.c @@ -332,10 +332,12 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp) BDRVRawState *s = bs->opaque; if (s->has_size) { + error_setg(errp, "Cannot resize fixed-size raw disks"); return -ENOTSUP; } if (INT64_MAX - offset < s->offset) { + error_setg(errp, "Disk size too large for the chosen offset"); return -EINVAL; } diff --git a/block/rbd.c b/block/rbd.c index 3e6e73e989..fbf30591d1 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -923,6 +923,7 @@ static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset, Error **errp) r = rbd_resize(s->image, offset); if (r < 0) { + error_setg_errno(errp, -r, "Failed to resize file"); return r; } From 048c5fd1bfc787adcb1b726ce997e87fe44545fd Mon Sep 17 00:00:00 2001 From: Eric Blake Date: Thu, 6 Apr 2017 20:37:09 -0500 Subject: [PATCH 05/12] qcow2: Allow discard of final unaligned cluster As mentioned in commit 0c1bd46, we ignored requests to discard the trailing cluster of an unaligned image. While discard is an advisory operation from the guest standpoint, (and we are therefore free to ignore any request), our qcow2 implementation exploits the fact that a discarded cluster reads back as 0. As long as we discard on cluster boundaries, we are fine; but that means we could observe non-zero data leaked at the tail of an unaligned image. Enhance iotest 66 to cover this case, and fix the implementation to honor a discard request on the final partial cluster. Signed-off-by: Eric Blake Message-id: 20170407013709.18440-1-eblake@redhat.com Signed-off-by: Max Reitz --- block/qcow2.c | 7 ++++++- tests/qemu-iotests/066 | 12 +++++++----- tests/qemu-iotests/066.out | 12 ++++++++---- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index 4ca4cf04b0..5c1573c999 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2515,7 +2515,12 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs, if (!QEMU_IS_ALIGNED(offset | count, s->cluster_size)) { assert(count < s->cluster_size); - return -ENOTSUP; + /* Ignore partial clusters, except for the special case of the + * complete partial cluster at the end of an unaligned file */ + if (!QEMU_IS_ALIGNED(offset, s->cluster_size) || + offset + count != bs->total_sectors * BDRV_SECTOR_SIZE) { + return -ENOTSUP; + } } qemu_co_mutex_lock(&s->lock); diff --git a/tests/qemu-iotests/066 b/tests/qemu-iotests/066 index 364166d3b2..c2116a3088 100755 --- a/tests/qemu-iotests/066 +++ b/tests/qemu-iotests/066 @@ -42,16 +42,18 @@ _supported_fmt qcow2 _supported_proto generic _supported_os Linux +# Intentionally create an unaligned image IMGOPTS="compat=1.1" -IMG_SIZE=64M +IMG_SIZE=$((64 * 1024 * 1024 + 512)) echo -echo "=== Testing snapshotting an image with zero clusters ===" +echo "=== Testing cluster discards ===" echo _make_test_img $IMG_SIZE -# Write some normal clusters, zero them (creating preallocated zero clusters) -# and discard those -$QEMU_IO -c "write 0 256k" -c "write -z 0 256k" -c "discard 0 256k" "$TEST_IMG" \ +# Write some normal clusters, zero some of them (creating preallocated +# zero clusters) and discard everything. Everything should now read as 0. +$QEMU_IO -c "write 0 256k" -c "write -z 0 256k" -c "write 64M 512" \ + -c "discard 0 $IMG_SIZE" -c "read -P 0 0 $IMG_SIZE" "$TEST_IMG" \ | _filter_qemu_io # Check the image (there shouldn't be any leaks) _check_test_img diff --git a/tests/qemu-iotests/066.out b/tests/qemu-iotests/066.out index 7bc9a107d5..7c1f31a1b1 100644 --- a/tests/qemu-iotests/066.out +++ b/tests/qemu-iotests/066.out @@ -1,13 +1,17 @@ QA output created by 066 -=== Testing snapshotting an image with zero clusters === +=== Testing cluster discards === -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67109376 wrote 262144/262144 bytes at offset 0 256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) wrote 262144/262144 bytes at offset 0 256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -discard 262144/262144 bytes at offset 0 -256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +wrote 512/512 bytes at offset 67108864 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +discard 67109376/67109376 bytes at offset 0 +64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +read 67109376/67109376 bytes at offset 0 +64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) No errors were found on the image. *** done From d4a7f45ec9a54ca52e6381a792dc2aaea656338a Mon Sep 17 00:00:00 2001 From: Klim Kireev Date: Wed, 5 Apr 2017 18:18:24 +0300 Subject: [PATCH 06/12] block: fix obvious coding style mistakes in block_int.h Signed-off-by: Klim Kireev Signed-off-by: Denis V. Lunev CC: Kevin Wolf CC: Max Reitz Message-id: 1491405505-31620-2-git-send-email-den@openvz.org Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- include/block/block_int.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/block/block_int.h b/include/block/block_int.h index dcde90a5d3..87739405d5 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -252,7 +252,7 @@ struct BlockDriver { * Returns 0 for completed check, -errno for internal errors. * The check results are stored in result. */ - int (*bdrv_check)(BlockDriverState* bs, BdrvCheckResult *result, + int (*bdrv_check)(BlockDriverState *bs, BdrvCheckResult *result, BdrvCheckMode fix); int (*bdrv_amend_options)(BlockDriverState *bs, QemuOpts *opts, @@ -454,13 +454,13 @@ struct BdrvChildRole { /* Returns a name that is supposedly more useful for human users than the * node name for identifying the node in question (in particular, a BB * name), or NULL if the parent can't provide a better name. */ - const char* (*get_name)(BdrvChild *child); + const char *(*get_name)(BdrvChild *child); /* Returns a malloced string that describes the parent of the child for a * human reader. This could be a node-name, BlockBackend name, qdev ID or * QOM path of the device owning the BlockBackend, job type and ID etc. The * caller is responsible for freeing the memory. */ - char* (*get_parent_desc)(BdrvChild *child); + char *(*get_parent_desc)(BdrvChild *child); /* * If this pair of functions is implemented, the parent doesn't issue new From 504c205a0d4a790dba2be0dc0aab8b8d1b905a7d Mon Sep 17 00:00:00 2001 From: "Denis V. Lunev" Date: Wed, 5 Apr 2017 18:18:25 +0300 Subject: [PATCH 07/12] block: assert no image modification under BDRV_O_INACTIVE As long as BDRV_O_INACTIVE is set, the image file is only opened so we have a file descriptor for it. We're definitely not supposed to modify the image, it's still owned by the migration source. This commit is an addition to 09e0c771 but the assert() is added to bdrv_truncate(). Signed-off-by: Denis V. Lunev CC: Kevin Wolf CC: Max Reitz Message-id: 1491405505-31620-3-git-send-email-den@openvz.org Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- block.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block.c b/block.c index 76bf00f4b2..6c6bb3ec7a 100644 --- a/block.c +++ b/block.c @@ -3328,6 +3328,8 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp) return -EACCES; } + assert(!(bs->open_flags & BDRV_O_INACTIVE)); + ret = drv->bdrv_truncate(bs, offset, errp); if (ret == 0) { ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS); From 9f1b92add20d244677c916e77d840b6282f691ac Mon Sep 17 00:00:00 2001 From: Vladimir Sementsov-Ogievskiy Date: Fri, 7 Apr 2017 14:34:04 +0300 Subject: [PATCH 08/12] qemu-img: improve convert_iteration_sectors() Do not do extra call to _get_block_status() Signed-off-by: Vladimir Sementsov-Ogievskiy Message-id: 20170407113404.9351-1-vsementsov@virtuozzo.com Reviewed-by: John Snow Signed-off-by: Max Reitz --- qemu-img.c | 32 ++++++++++---------------------- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 9eb82830f7..9b6e72893a 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1554,9 +1554,15 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) if (s->sector_next_status <= sector_num) { BlockDriverState *file; - ret = bdrv_get_block_status(blk_bs(s->src[src_cur]), - sector_num - src_cur_offset, - n, &n, &file); + if (s->target_has_backing) { + ret = bdrv_get_block_status(blk_bs(s->src[src_cur]), + sector_num - src_cur_offset, + n, &n, &file); + } else { + ret = bdrv_get_block_status_above(blk_bs(s->src[src_cur]), NULL, + sector_num - src_cur_offset, + n, &n, &file); + } if (ret < 0) { return ret; } @@ -1565,26 +1571,8 @@ static int convert_iteration_sectors(ImgConvertState *s, int64_t sector_num) s->status = BLK_ZERO; } else if (ret & BDRV_BLOCK_DATA) { s->status = BLK_DATA; - } else if (!s->target_has_backing) { - /* Without a target backing file we must copy over the contents of - * the backing file as well. */ - /* Check block status of the backing file chain to avoid - * needlessly reading zeroes and limiting the iteration to the - * buffer size */ - ret = bdrv_get_block_status_above(blk_bs(s->src[src_cur]), NULL, - sector_num - src_cur_offset, - n, &n, &file); - if (ret < 0) { - return ret; - } - - if (ret & BDRV_BLOCK_ZERO) { - s->status = BLK_ZERO; - } else { - s->status = BLK_DATA; - } } else { - s->status = BLK_BACKING_FILE; + s->status = s->target_has_backing ? BLK_BACKING_FILE : BLK_DATA; } s->sector_next_status = sector_num + n; From db933fbe0646fb75f93bbb8eb7d4d9db31c4d345 Mon Sep 17 00:00:00 2001 From: Lidong Chen Date: Thu, 27 Apr 2017 10:58:27 +0800 Subject: [PATCH 09/12] qemu-img: use blk_co_pwrite_zeroes for zero sectors when compressed When the buffer is zero, blk_co_pwrite_zeroes is more effective than blk_co_pwritev with BDRV_REQ_WRITE_COMPRESSED. This patch can reduce the time for converting qcow2 images with lots of zero data. Signed-off-by: Lidong Chen Message-id: 1493261907-18734-1-git-send-email-lidongchen@tencent.com Signed-off-by: Max Reitz --- qemu-img.c | 44 ++++++++++++++------------------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/qemu-img.c b/qemu-img.c index 9b6e72893a..c7196362df 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -1649,6 +1649,8 @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, while (nb_sectors > 0) { int n = nb_sectors; + BdrvRequestFlags flags = s->compressed ? BDRV_REQ_WRITE_COMPRESSED : 0; + switch (status) { case BLK_BACKING_FILE: /* If we have a backing file, leave clusters unallocated that are @@ -1658,43 +1660,24 @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, break; case BLK_DATA: - /* We must always write compressed clusters as a whole, so don't - * try to find zeroed parts in the buffer. We can only save the - * write if the buffer is completely zeroed and we're allowed to - * keep the target sparse. */ - if (s->compressed) { - if (s->has_zero_init && s->min_sparse && - buffer_is_zero(buf, n * BDRV_SECTOR_SIZE)) - { - assert(!s->target_has_backing); - break; - } - - iov.iov_base = buf; - iov.iov_len = n << BDRV_SECTOR_BITS; - qemu_iovec_init_external(&qiov, &iov, 1); - - ret = blk_co_pwritev(s->target, sector_num << BDRV_SECTOR_BITS, - n << BDRV_SECTOR_BITS, &qiov, - BDRV_REQ_WRITE_COMPRESSED); - if (ret < 0) { - return ret; - } - break; - } - - /* If there is real non-zero data or we're told to keep the target - * fully allocated (-S 0), we must write it. Otherwise we can treat - * it as zero sectors. */ + /* If we're told to keep the target fully allocated (-S 0) or there + * is real non-zero data, we must write it. Otherwise we can treat + * it as zero sectors. + * Compressed clusters need to be written as a whole, so in that + * case we can only save the write if the buffer is completely + * zeroed. */ if (!s->min_sparse || - is_allocated_sectors_min(buf, n, &n, s->min_sparse)) + (!s->compressed && + is_allocated_sectors_min(buf, n, &n, s->min_sparse)) || + (s->compressed && + !buffer_is_zero(buf, n * BDRV_SECTOR_SIZE))) { iov.iov_base = buf; iov.iov_len = n << BDRV_SECTOR_BITS; qemu_iovec_init_external(&qiov, &iov, 1); ret = blk_co_pwritev(s->target, sector_num << BDRV_SECTOR_BITS, - n << BDRV_SECTOR_BITS, &qiov, 0); + n << BDRV_SECTOR_BITS, &qiov, flags); if (ret < 0) { return ret; } @@ -1704,6 +1687,7 @@ static int coroutine_fn convert_co_write(ImgConvertState *s, int64_t sector_num, case BLK_ZERO: if (s->has_zero_init) { + assert(!s->target_has_backing); break; } ret = blk_co_pwrite_zeroes(s->target, From 4f38497b0fc03dcb24c9014c2cf34d324ab20c1e Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 Apr 2017 16:50:59 -0400 Subject: [PATCH 10/12] iotests: clarify help text Split the help text to highlight the groups of options a little better, carving out a clear "format" and "protocols" section. Signed-off-by: John Snow Message-id: 20170427205100.9505-2-jsnow@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- tests/qemu-iotests/common | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index 9c6f9721e5..fa8e69e74c 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -138,7 +138,7 @@ common options -v verbose -d debug -check options +image format options -raw test raw (default) -bochs test bochs -cloop test cloop @@ -150,14 +150,18 @@ check options -vpc test vpc -vhdx test vhdx -vmdk test vmdk + -luks test luks + +image protocol options -file test file (default) -rbd test rbd -sheepdog test sheepdog -nbd test nbd -ssh test ssh -nfs test nfs - -luks test luks -vxhs test vxhs + +other options -xdiff graphical mode diff -nocache use O_DIRECT on backing file -misalign misalign memory allocations From cc02e89eb4eb8b1cc43b2bc168dc0fca48ee721f Mon Sep 17 00:00:00 2001 From: John Snow Date: Thu, 27 Apr 2017 16:51:00 -0400 Subject: [PATCH 11/12] iotests: fix exclusion option If you are running out-of-tree, the -x option to exclude a certain iotest is broken. Replace porcelain usage of ls with a sturdier awk command. Reviewed-by: Fam Zheng Signed-off-by: John Snow Message-id: 20170427205100.9505-3-jsnow@redhat.com Reviewed-by: Eric Blake Signed-off-by: Max Reitz --- tests/qemu-iotests/common | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common index fa8e69e74c..f2a7199c4b 100644 --- a/tests/qemu-iotests/common +++ b/tests/qemu-iotests/common @@ -86,7 +86,8 @@ s/ .*//p elif $xgroup then # arg after -x - [ ! -s $tmp.list ] && ls [0-9][0-9][0-9] [0-9][0-9][0-9][0-9] >$tmp.list 2>/dev/null + # Populate $tmp.list with all tests + awk '/^[0-9]{3,}/ {print $1}' "${source_iotests}/group" > $tmp.list 2>/dev/null group_list=`sed -n <"$source_iotests/group" -e 's/$/ /' -e "/^[0-9][0-9][0-9].* $r /"'{ s/ .*//p }'` From 262fbae692722d5c8b647ba6b079409baefc3e3e Mon Sep 17 00:00:00 2001 From: Max Reitz Date: Wed, 8 Feb 2017 00:57:57 +0100 Subject: [PATCH 12/12] progress: Show current progress on SIGINFO Currently we only print progress information on retrieval of SIGUSR1. Some systems have a dedicated SIGINFO for this, however, so it should be handled appropriately if it is available. Buglink: https://bugs.launchpad.net/qemu/+bug/1662468 Signed-off-by: Max Reitz Message-id: 20170207235757.2026-1-mreitz@redhat.com Reviewed-by: Stefan Hajnoczi Signed-off-by: Max Reitz --- qemu-img.texi | 3 ++- util/qemu-progress.c | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/qemu-img.texi b/qemu-img.texi index 8c573ae010..50a2364e80 100644 --- a/qemu-img.texi +++ b/qemu-img.texi @@ -84,7 +84,8 @@ with or without a command shows help and lists the supported formats @item -p display progress bar (compare, convert and rebase commands only). If the @var{-p} option is not used for a command that supports it, the -progress is reported when the process receives a @code{SIGUSR1} signal. +progress is reported when the process receives a @code{SIGUSR1} or +@code{SIGINFO} signal. @item -q Quiet mode - do not print any output (except errors). There's no progress bar in case both @var{-q} and @var{-p} options are used. diff --git a/util/qemu-progress.c b/util/qemu-progress.c index f745233763..3c2223c1a2 100644 --- a/util/qemu-progress.c +++ b/util/qemu-progress.c @@ -88,6 +88,9 @@ static void progress_dummy_init(void) action.sa_handler = sigusr_print; action.sa_flags = 0; sigaction(SIGUSR1, &action, NULL); +#ifdef SIGINFO + sigaction(SIGINFO, &action, NULL); +#endif /* * SIGUSR1 is SIG_IPI and gets blocked in qemu_init_main_loop(). In the