* [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate()
@ 2019-09-18 9:51 Max Reitz
2019-09-18 9:51 ` [Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl Max Reitz
` (8 more replies)
0 siblings, 9 replies; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
Hi,
This series is supposed to pull out some of the problems from my
“Generic file creation fallback” series.
The blk_truncate_for_formatting() function added there was buggy, as
Maxim noted, in that it did not check whether blk_truncate() actually
resized the block node to the target offset. One way to fix this is to
add a parameter to it that forces the block driver to do so, and that is
done by this series.
I think this is generally useful (you can see the diff stat saldo is
only +23 lines), because it allows us to drop a special check in
qemu-img resize, and it fixes a bug in qed (which has relied on this
behavior for over 8 years, but unfortunately bdrv_truncate()’s behavior
changed quite exactly 8 years ago).
However, in the process I noticed we actually don’t need
blk_truncate_for_formatting(): The underlying problem is that some
format drivers truncate their underlying file node to 0 before
formatting it to drop all data. So they should pass exact=true, but
they cannot, because this would break creation on block devices. Hence
blk_truncate_for_formatting().
It turns out, though, that three of the four drivers in question don’t
need to truncate their file node at all. The remaining one is qed which
simply really should pass exact=true (it’s a bug fix).
(I do drop those blk_truncate() invocations in this series, because
otherwise I feel like it is impossible to decide whether they should get
exact=false or exact=true. Either way is wrong.)
Max Reitz (8):
block: Handle filter truncation like native impl.
block/cor: Drop cor_co_truncate()
block: Do not truncate file node when formatting
block: Add @exact parameter to bdrv_co_truncate()
block: Evaluate @exact in protocol drivers
block: Let format drivers pass @exact
block: Pass truncate exact=true where reasonable
Revert "qemu-img: Check post-truncation size"
include/block/block.h | 6 ++---
include/block/block_int.h | 17 ++++++++++++-
include/sysemu/block-backend.h | 4 +--
block/block-backend.c | 6 ++---
block/commit.c | 5 ++--
block/copy-on-read.c | 8 ------
block/crypto.c | 8 +++---
block/file-posix.c | 11 ++++++--
block/file-win32.c | 3 ++-
block/gluster.c | 1 +
block/io.c | 29 ++++++++++++---------
block/iscsi.c | 10 ++++++--
block/mirror.c | 4 +--
block/nfs.c | 2 +-
block/parallels.c | 18 +++++++------
block/qcow.c | 9 ++-----
block/qcow2-refcount.c | 2 +-
block/qcow2.c | 45 +++++++++++++++++++++------------
block/qed.c | 3 ++-
block/raw-format.c | 5 ++--
block/rbd.c | 1 +
block/sheepdog.c | 5 ++--
block/ssh.c | 3 ++-
block/vdi.c | 2 +-
block/vhdx-log.c | 4 +--
block/vhdx.c | 7 +++---
block/vmdk.c | 8 +++---
block/vpc.c | 2 +-
blockdev.c | 2 +-
qemu-img.c | 46 ++++++++--------------------------
qemu-io-cmds.c | 7 +++++-
tests/test-block-iothread.c | 8 +++---
32 files changed, 157 insertions(+), 134 deletions(-)
--
2.21.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl.
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:49 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 2/8] block/cor: Drop cor_co_truncate() Max Reitz
` (7 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
Make the filter truncation (passing it through to bs->file) a
first-class citizen and handle it exactly as if it was the filter
driver's native implementation of .bdrv_co_truncate().
I do not see a reason not to, it makes the code a bit shorter, and may
be even more correct because this gets us to finish the write_req that
we prepared before (may be important to e.g. bring dirty bitmaps to the
correct size).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/io.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/block/io.c b/block/io.c
index f8c3596131..723655c792 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3299,20 +3299,19 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
goto out;
}
- if (!drv->bdrv_co_truncate) {
- if (bs->file && drv->is_filter) {
- ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
- goto out;
- }
+ if (drv->bdrv_co_truncate) {
+ ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
+ } else if (bs->file && drv->is_filter) {
+ ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
+ } else {
error_setg(errp, "Image format driver does not support resize");
ret = -ENOTSUP;
goto out;
}
-
- ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
if (ret < 0) {
goto out;
}
+
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not refresh total sector count");
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 2/8] block/cor: Drop cor_co_truncate()
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
2019-09-18 9:51 ` [Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:49 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting Max Reitz
` (6 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
No other filter driver has a .bdrv_co_truncate() implementation, and
there is no need to because the general block layer code can handle it
just as well.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/copy-on-read.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/block/copy-on-read.c b/block/copy-on-read.c
index 6631f30205..e95223d3cb 100644
--- a/block/copy-on-read.c
+++ b/block/copy-on-read.c
@@ -73,13 +73,6 @@ static int64_t cor_getlength(BlockDriverState *bs)
}
-static int coroutine_fn cor_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
-{
- return bdrv_co_truncate(bs->file, offset, prealloc, errp);
-}
-
-
static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
uint64_t offset, uint64_t bytes,
QEMUIOVector *qiov, int flags)
@@ -139,7 +132,6 @@ static BlockDriver bdrv_copy_on_read = {
.bdrv_child_perm = cor_child_perm,
.bdrv_getlength = cor_getlength,
- .bdrv_co_truncate = cor_co_truncate,
.bdrv_co_preadv = cor_co_preadv,
.bdrv_co_pwritev = cor_co_pwritev,
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
2019-09-18 9:51 ` [Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl Max Reitz
2019-09-18 9:51 ` [Qemu-devel] [PATCH 2/8] block/cor: Drop cor_co_truncate() Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:50 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
` (5 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
There is no reason why the format drivers need to truncate the protocol
node when formatting it. When using the old .bdrv_co_create_ops()
interface, the file will be created with no size option anyway, which
generally gives it a size of 0. (Exceptions are block devices, which
cannot be truncated anyway.)
When using blockdev-create, the user must have given the file node some
size anyway, so there is no reason why we should override that.
qed is an exception, it needs the file to start completely empty (as
explained by c743849bee7333c7ef256b7e12e34ed6f907064f).
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/parallels.c | 5 -----
block/qcow.c | 5 -----
block/qcow2.c | 6 ------
3 files changed, 16 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 7cd2714b69..905cac35c6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -563,11 +563,6 @@ static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
blk_set_allow_write_beyond_eof(blk, true);
/* Create image format */
- ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
- if (ret < 0) {
- goto out;
- }
-
bat_entries = DIV_ROUND_UP(total_size, cl_size);
bat_sectors = DIV_ROUND_UP(bat_entry_off(bat_entries), cl_size);
bat_sectors = (bat_sectors * cl_size) >> BDRV_SECTOR_BITS;
diff --git a/block/qcow.c b/block/qcow.c
index 5bdf72ba33..17705015ca 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -858,11 +858,6 @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
blk_set_allow_write_beyond_eof(qcow_blk, true);
/* Create image format */
- ret = blk_truncate(qcow_blk, 0, PREALLOC_MODE_OFF, errp);
- if (ret < 0) {
- goto exit;
- }
-
memset(&header, 0, sizeof(header));
header.magic = cpu_to_be32(QCOW_MAGIC);
header.version = cpu_to_be32(QCOW_VERSION);
diff --git a/block/qcow2.c b/block/qcow2.c
index 4d16393e61..4978ccc7be 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3186,12 +3186,6 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
}
blk_set_allow_write_beyond_eof(blk, true);
- /* Clear the protocol layer and preallocate it if necessary */
- ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
- if (ret < 0) {
- goto out;
- }
-
/* Write the header */
QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
header = g_malloc0(cluster_size);
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate()
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
` (2 preceding siblings ...)
2019-09-18 9:51 ` [Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:50 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 5/8] block: Evaluate @exact in protocol drivers Max Reitz
` (4 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
We have two drivers (iscsi and file-posix) that (in some cases) return
success from their .bdrv_co_truncate() implementation if the block
device is larger than the requested offset, but cannot be shrunk. Some
callers do not want that behavior, so this patch adds a new parameter
that they can use to turn off that behavior.
This patch just adds the parameter and lets the block/io.c and
block/block-backend.c functions pass it around. All other callers
always pass false and none of the implementations evaluate it, so that
this patch does not change existing behavior. Future patches take care
of that.
Suggested-by: Maxim Levitsky <mlevitsk@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/block/block.h | 6 +++---
include/block/block_int.h | 17 ++++++++++++++++-
include/sysemu/block-backend.h | 4 ++--
block/block-backend.c | 6 +++---
block/commit.c | 5 +++--
block/crypto.c | 8 ++++----
block/file-posix.c | 3 ++-
block/file-win32.c | 3 ++-
block/gluster.c | 1 +
block/io.c | 20 +++++++++++++-------
block/iscsi.c | 3 ++-
block/mirror.c | 4 ++--
block/nfs.c | 2 +-
block/parallels.c | 6 +++---
block/qcow.c | 4 ++--
block/qcow2-refcount.c | 2 +-
block/qcow2.c | 22 ++++++++++++----------
block/qed.c | 3 ++-
block/raw-format.c | 5 +++--
block/rbd.c | 1 +
block/sheepdog.c | 5 +++--
block/ssh.c | 3 ++-
block/vdi.c | 2 +-
block/vhdx-log.c | 4 ++--
block/vhdx.c | 7 ++++---
block/vmdk.c | 8 ++++----
block/vpc.c | 2 +-
blockdev.c | 2 +-
qemu-img.c | 2 +-
qemu-io-cmds.c | 2 +-
tests/test-block-iothread.c | 8 ++++----
31 files changed, 102 insertions(+), 68 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 37c9de7446..2f905ae4b7 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -346,10 +346,10 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
const char *backing_file);
void bdrv_refresh_filename(BlockDriverState *bs);
-int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
+int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
PreallocMode prealloc, Error **errp);
-int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
- Error **errp);
+int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
+ PreallocMode prealloc, Error **errp);
int64_t bdrv_nb_sectors(BlockDriverState *bs);
int64_t bdrv_getlength(BlockDriverState *bs);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 0422acdf1c..197cc6e7c3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -334,8 +334,23 @@ struct BlockDriver {
* bdrv_parse_filename.
*/
const char *protocol_name;
+
+ /*
+ * Truncate @bs to @offset bytes using the given @prealloc mode
+ * when growing. Modes other than PREALLOC_MODE_OFF should be
+ * rejected when shrinking @bs.
+ *
+ * If @exact is true, @bs must be resized to exactly @offset.
+ * Otherwise, it is sufficient for @bs (if it is a host block
+ * device and thus there is no way to resize it) to be at least
+ * @offset bytes in length.
+ *
+ * If @exact is true and this function fails but would succeed
+ * with @exact = false, it should return -ENOTSUP.
+ */
int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp);
+ bool exact, PreallocMode prealloc,
+ Error **errp);
int64_t (*bdrv_getlength)(BlockDriverState *bs);
bool has_variable_length;
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 368d53af77..841804c3cb 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -233,8 +233,8 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
int bytes, BdrvRequestFlags flags);
int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
int bytes);
-int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
- Error **errp);
+int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
+ PreallocMode prealloc, Error **errp);
int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
int64_t pos, int size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 1c605d5444..e6d8240f40 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -2060,15 +2060,15 @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
BDRV_REQ_WRITE_COMPRESSED);
}
-int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
- Error **errp)
+int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
+ PreallocMode prealloc, Error **errp)
{
if (!blk_is_available(blk)) {
error_setg(errp, "No medium inserted");
return -ENOMEDIUM;
}
- return bdrv_truncate(blk->root, offset, prealloc, errp);
+ return bdrv_truncate(blk->root, offset, exact, prealloc, errp);
}
static void blk_pdiscard_entry(void *opaque)
diff --git a/block/commit.c b/block/commit.c
index bc8454463d..23c90b3b91 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -155,7 +155,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
}
if (base_len < len) {
- ret = blk_truncate(s->base, len, PREALLOC_MODE_OFF, NULL);
+ ret = blk_truncate(s->base, len, false, PREALLOC_MODE_OFF, NULL);
if (ret) {
goto out;
}
@@ -471,7 +471,8 @@ 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, PREALLOC_MODE_OFF, &local_err);
+ ret = blk_truncate(backing, length, false, PREALLOC_MODE_OFF,
+ &local_err);
if (ret < 0) {
error_report_err(local_err);
goto ro_cleanup;
diff --git a/block/crypto.c b/block/crypto.c
index 7eb698774e..e5a1a2cdf3 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -113,8 +113,8 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
* available to the guest, so we must take account of that
* which will be used by the crypto header
*/
- return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
- errp);
+ return blk_truncate(data->blk, data->size + headerlen, false,
+ data->prealloc, errp);
}
@@ -297,7 +297,7 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
}
static int coroutine_fn
-block_crypto_co_truncate(BlockDriverState *bs, int64_t offset,
+block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
PreallocMode prealloc, Error **errp)
{
BlockCrypto *crypto = bs->opaque;
@@ -311,7 +311,7 @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t offset,
offset += payload_offset;
- return bdrv_co_truncate(bs->file, offset, prealloc, errp);
+ return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
}
static void block_crypto_close(BlockDriverState *bs)
diff --git a/block/file-posix.c b/block/file-posix.c
index f12c06de2d..d8755c5fac 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2014,7 +2014,8 @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset,
}
static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
+ bool exact, PreallocMode prealloc,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
struct stat st;
diff --git a/block/file-win32.c b/block/file-win32.c
index 41f55dfece..77e8ff7b68 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -468,7 +468,8 @@ static void raw_close(BlockDriverState *bs)
}
static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
+ bool exact, PreallocMode prealloc,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
LONG low, high;
diff --git a/block/gluster.c b/block/gluster.c
index 64028b2cba..4fa4a77a47 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1225,6 +1225,7 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
static coroutine_fn int qemu_gluster_co_truncate(BlockDriverState *bs,
int64_t offset,
+ bool exact,
PreallocMode prealloc,
Error **errp)
{
diff --git a/block/io.c b/block/io.c
index 723655c792..5b02642b98 100644
--- a/block/io.c
+++ b/block/io.c
@@ -3243,8 +3243,12 @@ static void bdrv_parent_cb_resize(BlockDriverState *bs)
/**
* Truncate file to 'offset' bytes (needed only for file protocols)
+ *
+ * If 'exact' is true, the file must be resized to exactly the given
+ * 'offset'. Otherwise, it is sufficient for the node to be at least
+ * 'offset' bytes in length.
*/
-int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
+int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
PreallocMode prealloc, Error **errp)
{
BlockDriverState *bs = child->bs;
@@ -3300,9 +3304,9 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
}
if (drv->bdrv_co_truncate) {
- ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
+ ret = drv->bdrv_co_truncate(bs, offset, exact, prealloc, errp);
} else if (bs->file && drv->is_filter) {
- ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
+ ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
} else {
error_setg(errp, "Image format driver does not support resize");
ret = -ENOTSUP;
@@ -3333,6 +3337,7 @@ out:
typedef struct TruncateCo {
BdrvChild *child;
int64_t offset;
+ bool exact;
PreallocMode prealloc;
Error **errp;
int ret;
@@ -3341,18 +3346,19 @@ typedef struct TruncateCo {
static void coroutine_fn bdrv_truncate_co_entry(void *opaque)
{
TruncateCo *tco = opaque;
- tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc,
- tco->errp);
+ tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->exact,
+ tco->prealloc, tco->errp);
aio_wait_kick();
}
-int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
- Error **errp)
+int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
+ PreallocMode prealloc, Error **errp)
{
Coroutine *co;
TruncateCo tco = {
.child = child,
.offset = offset,
+ .exact = exact,
.prealloc = prealloc,
.errp = errp,
.ret = NOT_DONE,
diff --git a/block/iscsi.c b/block/iscsi.c
index 506bf5f875..a90426270a 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2122,7 +2122,8 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
}
static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
+ bool exact, PreallocMode prealloc,
+ Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
Error *local_err = NULL;
diff --git a/block/mirror.c b/block/mirror.c
index fe984efb90..2cfd035571 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -873,8 +873,8 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
}
if (s->bdev_length > base_length) {
- ret = blk_truncate(s->target, s->bdev_length, PREALLOC_MODE_OFF,
- NULL);
+ ret = blk_truncate(s->target, s->bdev_length, false,
+ PREALLOC_MODE_OFF, NULL);
if (ret < 0) {
goto immediate_exit;
}
diff --git a/block/nfs.c b/block/nfs.c
index f39acfdb28..ba8b41775a 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -750,7 +750,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
}
static int coroutine_fn
-nfs_file_co_truncate(BlockDriverState *bs, int64_t offset,
+nfs_file_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
PreallocMode prealloc, Error **errp)
{
NFSClient *client = bs->opaque;
diff --git a/block/parallels.c b/block/parallels.c
index 905cac35c6..a1a92c97a4 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -203,7 +203,7 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
} else {
ret = bdrv_truncate(bs->file,
(s->data_end + space) << BDRV_SECTOR_BITS,
- PREALLOC_MODE_OFF, NULL);
+ false, PREALLOC_MODE_OFF, NULL);
}
if (ret < 0) {
return ret;
@@ -487,7 +487,7 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
res->leaks += count;
if (fix & BDRV_FIX_LEAKS) {
Error *local_err = NULL;
- ret = bdrv_truncate(bs->file, res->image_end_offset,
+ ret = bdrv_truncate(bs->file, res->image_end_offset, false,
PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
@@ -880,7 +880,7 @@ static void parallels_close(BlockDriverState *bs)
if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
s->header->inuse = 0;
parallels_update_header(bs);
- bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
+ bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, false,
PREALLOC_MODE_OFF, NULL);
}
diff --git a/block/qcow.c b/block/qcow.c
index 17705015ca..fce8989868 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -480,7 +480,7 @@ static int get_cluster_offset(BlockDriverState *bs,
return -E2BIG;
}
ret = bdrv_truncate(bs->file, cluster_offset + s->cluster_size,
- PREALLOC_MODE_OFF, NULL);
+ false, PREALLOC_MODE_OFF, NULL);
if (ret < 0) {
return ret;
}
@@ -1033,7 +1033,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, false,
PREALLOC_MODE_OFF, NULL);
if (ret < 0)
return ret;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index ef965d7895..7b70d8683a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -2016,7 +2016,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
goto resize_fail;
}
- ret = bdrv_truncate(bs->file, offset + s->cluster_size,
+ ret = bdrv_truncate(bs->file, offset + s->cluster_size, false,
PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
diff --git a/block/qcow2.c b/block/qcow2.c
index 4978ccc7be..157b9b75d9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2869,8 +2869,8 @@ static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset,
if (mode == PREALLOC_MODE_METADATA) {
mode = PREALLOC_MODE_OFF;
}
- ret = bdrv_co_truncate(s->data_file, host_offset + cur_bytes, mode,
- errp);
+ ret = bdrv_co_truncate(s->data_file, host_offset + cur_bytes, false,
+ mode, errp);
if (ret < 0) {
return ret;
}
@@ -3284,7 +3284,8 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
}
/* Okay, now that we have a valid image, let's give it the right size */
- ret = blk_truncate(blk, qcow2_opts->size, qcow2_opts->preallocation, errp);
+ ret = blk_truncate(blk, qcow2_opts->size, false, qcow2_opts->preallocation,
+ errp);
if (ret < 0) {
error_prepend(errp, "Could not resize image: ");
goto out;
@@ -3732,7 +3733,8 @@ fail:
}
static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
+ bool exact, PreallocMode prealloc,
+ Error **errp)
{
BDRVQcow2State *s = bs->opaque;
uint64_t old_length;
@@ -3821,7 +3823,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
Error *local_err = NULL;
bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size,
- PREALLOC_MODE_OFF, &local_err);
+ false, PREALLOC_MODE_OFF, &local_err);
if (local_err) {
warn_reportf_err(local_err,
"Failed to truncate the tail of the image: ");
@@ -3838,7 +3840,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
switch (prealloc) {
case PREALLOC_MODE_OFF:
if (has_data_file(bs)) {
- ret = bdrv_co_truncate(s->data_file, offset, prealloc, errp);
+ ret = bdrv_co_truncate(s->data_file, offset, false, prealloc, errp);
if (ret < 0) {
goto fail;
}
@@ -3923,7 +3925,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
/* Allocate the data area */
new_file_size = allocation_start +
nb_new_data_clusters * s->cluster_size;
- ret = bdrv_co_truncate(bs->file, new_file_size, prealloc, errp);
+ ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, errp);
if (ret < 0) {
error_prepend(errp, "Failed to resize underlying file: ");
qcow2_free_clusters(bs, allocation_start,
@@ -4026,7 +4028,7 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
if (len < 0) {
return len;
}
- return bdrv_co_truncate(bs->file, len, PREALLOC_MODE_OFF, NULL);
+ return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL);
}
if (offset_into_cluster(s, offset)) {
@@ -4263,7 +4265,7 @@ 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, false,
PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
@@ -5042,7 +5044,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
return ret;
}
- ret = blk_truncate(blk, new_size, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, new_size, false, PREALLOC_MODE_OFF, errp);
blk_unref(blk);
if (ret < 0) {
return ret;
diff --git a/block/qed.c b/block/qed.c
index 0d8fd507aa..7c2a65af40 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -674,7 +674,7 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
l1_size = header.cluster_size * header.table_size;
/* File must start empty and grow, check truncate is supported */
- ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, 0, false, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto out;
}
@@ -1461,6 +1461,7 @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
static int coroutine_fn bdrv_qed_co_truncate(BlockDriverState *bs,
int64_t offset,
+ bool exact,
PreallocMode prealloc,
Error **errp)
{
diff --git a/block/raw-format.c b/block/raw-format.c
index 42c28cc29a..57d84d0bae 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -370,7 +370,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
}
static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
+ bool exact, PreallocMode prealloc,
+ Error **errp)
{
BDRVRawState *s = bs->opaque;
@@ -386,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
s->size = offset;
offset += s->offset;
- return bdrv_co_truncate(bs->file, offset, prealloc, errp);
+ return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
}
static void raw_eject(BlockDriverState *bs, bool eject_flag)
diff --git a/block/rbd.c b/block/rbd.c
index 057af43d48..cdccf01d68 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1086,6 +1086,7 @@ static int64_t qemu_rbd_getlength(BlockDriverState *bs)
static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs,
int64_t offset,
+ bool exact,
PreallocMode prealloc,
Error **errp)
{
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 773dfc6ab1..cfa84338a2 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2285,7 +2285,8 @@ static int64_t sd_getlength(BlockDriverState *bs)
}
static int coroutine_fn sd_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
+ bool exact, PreallocMode prealloc,
+ Error **errp)
{
BDRVSheepdogState *s = bs->opaque;
int ret, fd;
@@ -2601,7 +2602,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
assert(!flags);
if (offset > s->inode.vdi_size) {
- ret = sd_co_truncate(bs, offset, PREALLOC_MODE_OFF, NULL);
+ ret = sd_co_truncate(bs, offset, false, PREALLOC_MODE_OFF, NULL);
if (ret < 0) {
return ret;
}
diff --git a/block/ssh.c b/block/ssh.c
index 84d01e892b..b4375cf7d2 100644
--- a/block/ssh.c
+++ b/block/ssh.c
@@ -1295,7 +1295,8 @@ static int64_t ssh_getlength(BlockDriverState *bs)
}
static int coroutine_fn ssh_co_truncate(BlockDriverState *bs, int64_t offset,
- PreallocMode prealloc, Error **errp)
+ bool exact, PreallocMode prealloc,
+ Error **errp)
{
BDRVSSHState *s = bs->opaque;
diff --git a/block/vdi.c b/block/vdi.c
index 806ba7f53c..0142da7233 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -874,7 +874,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
}
if (image_type == VDI_TYPE_STATIC) {
- ret = blk_truncate(blk, offset + blocks * block_size,
+ ret = blk_truncate(blk, offset + blocks * block_size, false,
PREALLOC_MODE_OFF, errp);
if (ret < 0) {
error_prepend(errp, "Failed to statically allocate file");
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index fdd3a7adc3..13a49c2a33 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -557,8 +557,8 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s,
ret = -EINVAL;
goto exit;
}
- ret = bdrv_truncate(bs->file, new_file_size, PREALLOC_MODE_OFF,
- NULL);
+ ret = bdrv_truncate(bs->file, new_file_size, false,
+ PREALLOC_MODE_OFF, NULL);
if (ret < 0) {
goto exit;
}
diff --git a/block/vhdx.c b/block/vhdx.c
index 6a09d0a55c..8ae14d97f9 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1180,7 +1180,7 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
return -EINVAL;
}
- return bdrv_truncate(bs->file, *new_offset + s->block_size,
+ return bdrv_truncate(bs->file, *new_offset + s->block_size, false,
PREALLOC_MODE_OFF, NULL);
}
@@ -1619,12 +1619,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, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, data_file_offset, false, PREALLOC_MODE_OFF,
+ errp);
if (ret < 0) {
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, false,
PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
diff --git a/block/vmdk.c b/block/vmdk.c
index fed3b50c8a..20e909d997 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -2076,7 +2076,7 @@ vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
return length;
}
length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE);
- ret = bdrv_truncate(s->extents[i].file, length,
+ ret = bdrv_truncate(s->extents[i].file, length, false,
PREALLOC_MODE_OFF, NULL);
if (ret < 0) {
return ret;
@@ -2118,7 +2118,7 @@ static int vmdk_init_extent(BlockBackend *blk,
int gd_buf_size;
if (flat) {
- ret = blk_truncate(blk, filesize, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, filesize, false, PREALLOC_MODE_OFF, errp);
goto exit;
}
magic = cpu_to_be32(VMDK4_MAGIC);
@@ -2181,7 +2181,7 @@ static int vmdk_init_extent(BlockBackend *blk,
goto exit;
}
- ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9,
+ ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9, false,
PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
@@ -2523,7 +2523,7 @@ static int coroutine_fn vmdk_co_do_create(int64_t size,
/* bdrv_pwrite write padding zeros to align to sector, we don't need that
* for description file */
if (desc_offset == 0) {
- ret = blk_truncate(blk, desc_len, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, desc_len, false, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
}
diff --git a/block/vpc.c b/block/vpc.c
index 5cd3890780..a65550298e 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -898,7 +898,7 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
/* Add footer to total size */
total_size += HEADER_SIZE;
- ret = blk_truncate(blk, total_size, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, total_size, false, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
return ret;
}
diff --git a/blockdev.c b/blockdev.c
index fbef6845c8..6b04eca2a4 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -3225,7 +3225,7 @@ void qmp_block_resize(bool has_device, const char *device,
}
bdrv_drained_begin(bs);
- ret = blk_truncate(blk, size, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, size, false, PREALLOC_MODE_OFF, errp);
bdrv_drained_end(bs);
out:
diff --git a/qemu-img.c b/qemu-img.c
index 384c6f38bc..f8694f4f72 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3823,7 +3823,7 @@ static int img_resize(int argc, char **argv)
}
}
- ret = blk_truncate(blk, total_size, prealloc, &err);
+ ret = blk_truncate(blk, total_size, false, prealloc, &err);
if (ret < 0) {
error_report_err(err);
goto out;
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 349256a5fe..5e9017c979 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1710,7 +1710,7 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv)
return offset;
}
- ret = blk_truncate(blk, offset, PREALLOC_MODE_OFF, &local_err);
+ ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
return ret;
diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index cfe30bab21..0c861809f0 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -45,7 +45,7 @@ static int coroutine_fn bdrv_test_co_pdiscard(BlockDriverState *bs,
}
static int coroutine_fn
-bdrv_test_co_truncate(BlockDriverState *bs, int64_t offset,
+bdrv_test_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
PreallocMode prealloc, Error **errp)
{
return 0;
@@ -185,18 +185,18 @@ static void test_sync_op_truncate(BdrvChild *c)
int ret;
/* Normal success path */
- ret = bdrv_truncate(c, 65536, PREALLOC_MODE_OFF, NULL);
+ ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, NULL);
g_assert_cmpint(ret, ==, 0);
/* Early error: Negative offset */
- ret = bdrv_truncate(c, -2, PREALLOC_MODE_OFF, NULL);
+ ret = bdrv_truncate(c, -2, false, PREALLOC_MODE_OFF, NULL);
g_assert_cmpint(ret, ==, -EINVAL);
/* Error: Read-only image */
c->bs->read_only = true;
c->bs->open_flags &= ~BDRV_O_RDWR;
- ret = bdrv_truncate(c, 65536, PREALLOC_MODE_OFF, NULL);
+ ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, NULL);
g_assert_cmpint(ret, ==, -EACCES);
c->bs->read_only = false;
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 5/8] block: Evaluate @exact in protocol drivers
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
` (3 preceding siblings ...)
2019-09-18 9:51 ` [Qemu-devel] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:51 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact Max Reitz
` (3 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
We have two protocol drivers that return success when trying to shrink a
block device even though they cannot shrink it. This behavior is now
only allowed with exact=false, so they should return an error with
exact=true.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/file-posix.c | 8 +++++++-
block/iscsi.c | 7 ++++++-
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index d8755c5fac..a85f3486d1 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -2028,6 +2028,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
}
if (S_ISREG(st.st_mode)) {
+ /* Always resizes to the exact @offset */
return raw_regular_truncate(bs, s->fd, offset, prealloc, errp);
}
@@ -2038,7 +2039,12 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
}
if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
- if (offset > raw_getlength(bs)) {
+ int64_t cur_length = raw_getlength(bs);
+
+ if (offset != cur_length && exact) {
+ error_setg(errp, "Cannot resize device files");
+ return -ENOTSUP;
+ } else if (offset > cur_length) {
error_setg(errp, "Cannot grow device files");
return -EINVAL;
}
diff --git a/block/iscsi.c b/block/iscsi.c
index a90426270a..cc2072ff6c 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2126,6 +2126,7 @@ static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
+ int64_t cur_length;
Error *local_err = NULL;
if (prealloc != PREALLOC_MODE_OFF) {
@@ -2145,7 +2146,11 @@ static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
return -EIO;
}
- if (offset > iscsi_getlength(bs)) {
+ cur_length = iscsi_getlength(bs);
+ if (offset != cur_length && exact) {
+ error_setg(errp, "Cannot resize iSCSI devices");
+ return -ENOTSUP;
+ } else if (offset > cur_length) {
error_setg(errp, "Cannot grow iSCSI devices");
return -EINVAL;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
` (4 preceding siblings ...)
2019-09-18 9:51 ` [Qemu-devel] [PATCH 5/8] block: Evaluate @exact in protocol drivers Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:51 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 7/8] block: Pass truncate exact=true where reasonable Max Reitz
` (2 subsequent siblings)
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
When truncating a format node, the @exact parameter is generally handled
simply by virtue of the format storing the new size in the image
metadata. Such formats do not need to pass on the parameter to their
file nodes.
There are exceptions, though:
- raw and crypto cannot store the image size, and thus must pass on
@exact.
- When using qcow2 with an external data file, it just makes sense to
keep its size in sync with the qcow2 virtual disk (because the
external data file is the virtual disk). Therefore, we should pass
@exact when truncating it.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/crypto.c | 2 +-
block/qcow2.c | 15 ++++++++++++++-
block/raw-format.c | 2 +-
3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/block/crypto.c b/block/crypto.c
index e5a1a2cdf3..24823835c1 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -311,7 +311,7 @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
offset += payload_offset;
- return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
+ return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
}
static void block_crypto_close(BlockDriverState *bs)
diff --git a/block/qcow2.c b/block/qcow2.c
index 157b9b75d9..4ef19dd29a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3822,6 +3822,13 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
if ((last_cluster + 1) * s->cluster_size < old_file_size) {
Error *local_err = NULL;
+ /*
+ * Do not pass @exact here: It will not help the user if
+ * we get an error here just because they wanted to shrink
+ * their qcow2 image (on a block device) with qemu-img.
+ * (And on the qcow2 layer, the @exact requirement is
+ * always fulfilled, so there is no need to pass it on.)
+ */
bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size,
false, PREALLOC_MODE_OFF, &local_err);
if (local_err) {
@@ -3840,7 +3847,12 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
switch (prealloc) {
case PREALLOC_MODE_OFF:
if (has_data_file(bs)) {
- ret = bdrv_co_truncate(s->data_file, offset, false, prealloc, errp);
+ /*
+ * If the caller wants an exact resize, the external data
+ * file should be resized to the exact target size, too,
+ * so we pass @exact here.
+ */
+ ret = bdrv_co_truncate(s->data_file, offset, exact, prealloc, errp);
if (ret < 0) {
goto fail;
}
@@ -3925,6 +3937,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
/* Allocate the data area */
new_file_size = allocation_start +
nb_new_data_clusters * s->cluster_size;
+ /* Image file grows, so @exact does not matter */
ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, errp);
if (ret < 0) {
error_prepend(errp, "Failed to resize underlying file: ");
diff --git a/block/raw-format.c b/block/raw-format.c
index 57d84d0bae..3a76ec7dd2 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -387,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
s->size = offset;
offset += s->offset;
- return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
+ return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
}
static void raw_eject(BlockDriverState *bs, bool eject_flag)
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 7/8] block: Pass truncate exact=true where reasonable
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
` (5 preceding siblings ...)
2019-09-18 9:51 ` [Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:52 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 8/8] Revert "qemu-img: Check post-truncation size" Max Reitz
2019-10-28 11:10 ` [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
This is a change in behavior, so all instances need a good
justification. The comments added here should explain my reasoning.
qed already had a comment that suggests it always expected
bdrv_truncate()/blk_truncate() to behave as if exact=true were passed
(c743849bee7 came eight months before 55b949c8476), so it was simply
broken until now.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/parallels.c | 11 +++++++++--
block/qcow2.c | 6 +++++-
block/qed.c | 2 +-
qemu-img.c | 7 ++++++-
qemu-io-cmds.c | 7 ++++++-
5 files changed, 27 insertions(+), 6 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index a1a92c97a4..603211f83c 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -487,7 +487,12 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
res->leaks += count;
if (fix & BDRV_FIX_LEAKS) {
Error *local_err = NULL;
- ret = bdrv_truncate(bs->file, res->image_end_offset, false,
+
+ /*
+ * In order to really repair the image, we must shrink it.
+ * That means we have to pass exact=true.
+ */
+ ret = bdrv_truncate(bs->file, res->image_end_offset, true,
PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
@@ -880,7 +885,9 @@ static void parallels_close(BlockDriverState *bs)
if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
s->header->inuse = 0;
parallels_update_header(bs);
- bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, false,
+
+ /* errors are ignored, so we might as well pass exact=true */
+ bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
PREALLOC_MODE_OFF, NULL);
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 4ef19dd29a..eba165de7f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5057,7 +5057,11 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
return ret;
}
- ret = blk_truncate(blk, new_size, false, PREALLOC_MODE_OFF, errp);
+ /*
+ * Amending image options should ensure that the image has
+ * exactly the given new values, so pass exact=true here.
+ */
+ ret = blk_truncate(blk, new_size, true, PREALLOC_MODE_OFF, errp);
blk_unref(blk);
if (ret < 0) {
return ret;
diff --git a/block/qed.c b/block/qed.c
index 7c2a65af40..8005cfc305 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -674,7 +674,7 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
l1_size = header.cluster_size * header.table_size;
/* File must start empty and grow, check truncate is supported */
- ret = blk_truncate(blk, 0, false, PREALLOC_MODE_OFF, errp);
+ ret = blk_truncate(blk, 0, true, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto out;
}
diff --git a/qemu-img.c b/qemu-img.c
index f8694f4f72..a3169b6113 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3823,7 +3823,12 @@ static int img_resize(int argc, char **argv)
}
}
- ret = blk_truncate(blk, total_size, false, prealloc, &err);
+ /*
+ * The user expects the image to have the desired size after
+ * resizing, so pass @exact=true. It is of no use to report
+ * success when the image has not actually been resized.
+ */
+ ret = blk_truncate(blk, total_size, true, prealloc, &err);
if (ret < 0) {
error_report_err(err);
goto out;
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 5e9017c979..1b7e700020 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1710,7 +1710,12 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv)
return offset;
}
- ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, &local_err);
+ /*
+ * qemu-io is a debugging tool, so let us be strict here and pass
+ * exact=true. It is better to err on the "emit more errors" side
+ * than to be overly permissive.
+ */
+ ret = blk_truncate(blk, offset, true, PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
return ret;
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [Qemu-devel] [PATCH 8/8] Revert "qemu-img: Check post-truncation size"
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
` (6 preceding siblings ...)
2019-09-18 9:51 ` [Qemu-devel] [PATCH 7/8] block: Pass truncate exact=true where reasonable Max Reitz
@ 2019-09-18 9:51 ` Max Reitz
2019-09-18 20:52 ` Maxim Levitsky
2019-10-28 11:10 ` [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
8 siblings, 1 reply; 20+ messages in thread
From: Max Reitz @ 2019-09-18 9:51 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Maxim Levitsky,
Stefan Hajnoczi, Max Reitz
This reverts commit 5279b30392da7a3248b320c75f20c61e3a95863c.
We no longer need this check because exact=true forces the block driver
to give the image the exact size requested by the user.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 39 ++++-----------------------------------
1 file changed, 4 insertions(+), 35 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index a3169b6113..148f6b8b0e 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3648,7 +3648,7 @@ static int img_resize(int argc, char **argv)
Error *err = NULL;
int c, ret, relative;
const char *filename, *fmt, *size;
- int64_t n, total_size, current_size, new_size;
+ int64_t n, total_size, current_size;
bool quiet = false;
BlockBackend *blk = NULL;
PreallocMode prealloc = PREALLOC_MODE_OFF;
@@ -3829,42 +3829,11 @@ static int img_resize(int argc, char **argv)
* success when the image has not actually been resized.
*/
ret = blk_truncate(blk, total_size, true, prealloc, &err);
- if (ret < 0) {
+ if (!ret) {
+ qprintf(quiet, "Image resized.\n");
+ } else {
error_report_err(err);
- goto out;
- }
-
- new_size = blk_getlength(blk);
- if (new_size < 0) {
- error_report("Failed to verify truncated image length: %s",
- strerror(-new_size));
- ret = -1;
- goto out;
}
-
- /* Some block drivers implement a truncation method, but only so
- * the user can cause qemu to refresh the image's size from disk.
- * The idea is that the user resizes the image outside of qemu and
- * then invokes block_resize to inform qemu about it.
- * (This includes iscsi and file-posix for device files.)
- * Of course, that is not the behavior someone invoking
- * qemu-img resize would find useful, so we catch that behavior
- * here and tell the user. */
- if (new_size != total_size && new_size == current_size) {
- error_report("Image was not resized; resizing may not be supported "
- "for this image");
- ret = -1;
- goto out;
- }
-
- if (new_size != total_size) {
- warn_report("Image should have been resized to %" PRIi64
- " bytes, but was resized to %" PRIi64 " bytes",
- total_size, new_size);
- }
-
- qprintf(quiet, "Image resized.\n");
-
out:
blk_unref(blk);
if (ret) {
--
2.21.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl.
2019-09-18 9:51 ` [Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl Max Reitz
@ 2019-09-18 20:49 ` Maxim Levitsky
0 siblings, 0 replies; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:49 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> Make the filter truncation (passing it through to bs->file) a
> first-class citizen and handle it exactly as if it was the filter
> driver's native implementation of .bdrv_co_truncate().
>
> I do not see a reason not to, it makes the code a bit shorter, and may
> be even more correct because this gets us to finish the write_req that
> we prepared before (may be important to e.g. bring dirty bitmaps to the
> correct size).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/io.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index f8c3596131..723655c792 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3299,20 +3299,19 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
> goto out;
> }
>
> - if (!drv->bdrv_co_truncate) {
> - if (bs->file && drv->is_filter) {
> - ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
> - goto out;
> - }
> + if (drv->bdrv_co_truncate) {
> + ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
> + } else if (bs->file && drv->is_filter) {
> + ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
> + } else {
> error_setg(errp, "Image format driver does not support resize");
> ret = -ENOTSUP;
> goto out;
> }
> -
> - ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
> if (ret < 0) {
> goto out;
> }
> +
>
I would say that those are unrelated whitespace changes, but I myself
don't mind this :-)
> ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Could not refresh total sector count");
Looks all right to me, although I don't know the block filters well yet.
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 2/8] block/cor: Drop cor_co_truncate()
2019-09-18 9:51 ` [Qemu-devel] [PATCH 2/8] block/cor: Drop cor_co_truncate() Max Reitz
@ 2019-09-18 20:49 ` Maxim Levitsky
0 siblings, 0 replies; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:49 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> No other filter driver has a .bdrv_co_truncate() implementation, and
> there is no need to because the general block layer code can handle it
> just as well.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/copy-on-read.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 6631f30205..e95223d3cb 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -73,13 +73,6 @@ static int64_t cor_getlength(BlockDriverState *bs)
> }
>
>
> -static int coroutine_fn cor_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> -{
> - return bdrv_co_truncate(bs->file, offset, prealloc, errp);
> -}
> -
> -
> static int coroutine_fn cor_co_preadv(BlockDriverState *bs,
> uint64_t offset, uint64_t bytes,
> QEMUIOVector *qiov, int flags)
> @@ -139,7 +132,6 @@ static BlockDriver bdrv_copy_on_read = {
> .bdrv_child_perm = cor_child_perm,
>
> .bdrv_getlength = cor_getlength,
> - .bdrv_co_truncate = cor_co_truncate,
>
> .bdrv_co_preadv = cor_co_preadv,
> .bdrv_co_pwritev = cor_co_pwritev,
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting
2019-09-18 9:51 ` [Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting Max Reitz
@ 2019-09-18 20:50 ` Maxim Levitsky
0 siblings, 0 replies; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:50 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> There is no reason why the format drivers need to truncate the protocol
> node when formatting it. When using the old .bdrv_co_create_ops()
> interface, the file will be created with no size option anyway, which
> generally gives it a size of 0. (Exceptions are block devices, which
> cannot be truncated anyway.)
>
> When using blockdev-create, the user must have given the file node some
> size anyway, so there is no reason why we should override that.
>
> qed is an exception, it needs the file to start completely empty (as
> explained by c743849bee7333c7ef256b7e12e34ed6f907064f).
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/parallels.c | 5 -----
> block/qcow.c | 5 -----
> block/qcow2.c | 6 ------
> 3 files changed, 16 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 7cd2714b69..905cac35c6 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -563,11 +563,6 @@ static int coroutine_fn parallels_co_create(BlockdevCreateOptions* opts,
> blk_set_allow_write_beyond_eof(blk, true);
>
> /* Create image format */
> - ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
> - if (ret < 0) {
> - goto out;
> - }
> -
> bat_entries = DIV_ROUND_UP(total_size, cl_size);
> bat_sectors = DIV_ROUND_UP(bat_entry_off(bat_entries), cl_size);
> bat_sectors = (bat_sectors * cl_size) >> BDRV_SECTOR_BITS;
> diff --git a/block/qcow.c b/block/qcow.c
> index 5bdf72ba33..17705015ca 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -858,11 +858,6 @@ static int coroutine_fn qcow_co_create(BlockdevCreateOptions *opts,
> blk_set_allow_write_beyond_eof(qcow_blk, true);
>
> /* Create image format */
> - ret = blk_truncate(qcow_blk, 0, PREALLOC_MODE_OFF, errp);
> - if (ret < 0) {
> - goto exit;
> - }
> -
> memset(&header, 0, sizeof(header));
> header.magic = cpu_to_be32(QCOW_MAGIC);
> header.version = cpu_to_be32(QCOW_VERSION);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 4d16393e61..4978ccc7be 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3186,12 +3186,6 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> }
> blk_set_allow_write_beyond_eof(blk, true);
>
> - /* Clear the protocol layer and preallocate it if necessary */
> - ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
> - if (ret < 0) {
> - goto out;
> - }
> -
> /* Write the header */
> QEMU_BUILD_BUG_ON((1 << MIN_CLUSTER_BITS) < sizeof(*header));
> header = g_malloc0(cluster_size);
As long as bdrv_co_create_ops still creates the underlying file with bdrv_create_file or so,
I also don't see a reason to truncate it to 0 afterward.
Especially for block devices...
So,
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate()
2019-09-18 9:51 ` [Qemu-devel] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
@ 2019-09-18 20:50 ` Maxim Levitsky
2019-10-28 11:05 ` Max Reitz
0 siblings, 1 reply; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:50 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> We have two drivers (iscsi and file-posix) that (in some cases) return
> success from their .bdrv_co_truncate() implementation if the block
> device is larger than the requested offset, but cannot be shrunk. Some
> callers do not want that behavior, so this patch adds a new parameter
> that they can use to turn off that behavior.
>
> This patch just adds the parameter and lets the block/io.c and
> block/block-backend.c functions pass it around. All other callers
> always pass false and none of the implementations evaluate it, so that
> this patch does not change existing behavior. Future patches take care
> of that.
>
> Suggested-by: Maxim Levitsky <mlevitsk@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> include/block/block.h | 6 +++---
> include/block/block_int.h | 17 ++++++++++++++++-
> include/sysemu/block-backend.h | 4 ++--
> block/block-backend.c | 6 +++---
> block/commit.c | 5 +++--
> block/crypto.c | 8 ++++----
> block/file-posix.c | 3 ++-
> block/file-win32.c | 3 ++-
> block/gluster.c | 1 +
> block/io.c | 20 +++++++++++++-------
> block/iscsi.c | 3 ++-
> block/mirror.c | 4 ++--
> block/nfs.c | 2 +-
> block/parallels.c | 6 +++---
> block/qcow.c | 4 ++--
> block/qcow2-refcount.c | 2 +-
> block/qcow2.c | 22 ++++++++++++----------
> block/qed.c | 3 ++-
> block/raw-format.c | 5 +++--
> block/rbd.c | 1 +
> block/sheepdog.c | 5 +++--
> block/ssh.c | 3 ++-
> block/vdi.c | 2 +-
> block/vhdx-log.c | 4 ++--
> block/vhdx.c | 7 ++++---
> block/vmdk.c | 8 ++++----
> block/vpc.c | 2 +-
> blockdev.c | 2 +-
> qemu-img.c | 2 +-
> qemu-io-cmds.c | 2 +-
> tests/test-block-iothread.c | 8 ++++----
> 31 files changed, 102 insertions(+), 68 deletions(-)
>
> diff --git a/include/block/block.h b/include/block/block.h
> index 37c9de7446..2f905ae4b7 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -346,10 +346,10 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
> const char *backing_file);
> void bdrv_refresh_filename(BlockDriverState *bs);
>
> -int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
> +int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
> PreallocMode prealloc, Error **errp);
> -int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
> - Error **errp);
> +int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
> + PreallocMode prealloc, Error **errp);
>
> int64_t bdrv_nb_sectors(BlockDriverState *bs);
> int64_t bdrv_getlength(BlockDriverState *bs);
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index 0422acdf1c..197cc6e7c3 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -334,8 +334,23 @@ struct BlockDriver {
> * bdrv_parse_filename.
> */
> const char *protocol_name;
> +
> + /*
> + * Truncate @bs to @offset bytes using the given @prealloc mode
> + * when growing. Modes other than PREALLOC_MODE_OFF should be
> + * rejected when shrinking @bs.
> + *
> + * If @exact is true, @bs must be resized to exactly @offset.
> + * Otherwise, it is sufficient for @bs (if it is a host block
> + * device and thus there is no way to resize it) to be at least
> + * @offset bytes in length.
> + *
> + * If @exact is true and this function fails but would succeed
> + * with @exact = false, it should return -ENOTSUP.
> + */
Thanks for adding a documentation here!
One minor nitpick:
I would write
Otherwise, it is sufficient for @bs (for example if it is a host block
device and thus there is no way to resize it) to be at least @offset bytes in length.
> int coroutine_fn (*bdrv_co_truncate)(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp);
> + bool exact, PreallocMode prealloc,
> + Error **errp);
>
> int64_t (*bdrv_getlength)(BlockDriverState *bs);
> bool has_variable_length;
> diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
> index 368d53af77..841804c3cb 100644
> --- a/include/sysemu/block-backend.h
> +++ b/include/sysemu/block-backend.h
> @@ -233,8 +233,8 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
> int bytes, BdrvRequestFlags flags);
> int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
> int bytes);
> -int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
> - Error **errp);
> +int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
> + PreallocMode prealloc, Error **errp);
> int blk_pdiscard(BlockBackend *blk, int64_t offset, int bytes);
> int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
> int64_t pos, int size);
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 1c605d5444..e6d8240f40 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -2060,15 +2060,15 @@ int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
> BDRV_REQ_WRITE_COMPRESSED);
> }
>
> -int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
> - Error **errp)
> +int blk_truncate(BlockBackend *blk, int64_t offset, bool exact,
> + PreallocMode prealloc, Error **errp)
> {
> if (!blk_is_available(blk)) {
> error_setg(errp, "No medium inserted");
> return -ENOMEDIUM;
> }
>
> - return bdrv_truncate(blk->root, offset, prealloc, errp);
> + return bdrv_truncate(blk->root, offset, exact, prealloc, errp);
> }
>
> static void blk_pdiscard_entry(void *opaque)
> diff --git a/block/commit.c b/block/commit.c
> index bc8454463d..23c90b3b91 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -155,7 +155,7 @@ static int coroutine_fn commit_run(Job *job, Error **errp)
> }
>
> if (base_len < len) {
> - ret = blk_truncate(s->base, len, PREALLOC_MODE_OFF, NULL);
> + ret = blk_truncate(s->base, len, false, PREALLOC_MODE_OFF, NULL);
> if (ret) {
> goto out;
> }
> @@ -471,7 +471,8 @@ 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, PREALLOC_MODE_OFF, &local_err);
> + ret = blk_truncate(backing, length, false, PREALLOC_MODE_OFF,
> + &local_err);
> if (ret < 0) {
> error_report_err(local_err);
> goto ro_cleanup;
> diff --git a/block/crypto.c b/block/crypto.c
> index 7eb698774e..e5a1a2cdf3 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -113,8 +113,8 @@ static ssize_t block_crypto_init_func(QCryptoBlock *block,
> * available to the guest, so we must take account of that
> * which will be used by the crypto header
> */
> - return blk_truncate(data->blk, data->size + headerlen, data->prealloc,
> - errp);
> + return blk_truncate(data->blk, data->size + headerlen, false,
> + data->prealloc, errp);
> }
>
>
> @@ -297,7 +297,7 @@ static int block_crypto_co_create_generic(BlockDriverState *bs,
> }
>
> static int coroutine_fn
> -block_crypto_co_truncate(BlockDriverState *bs, int64_t offset,
> +block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
> PreallocMode prealloc, Error **errp)
> {
> BlockCrypto *crypto = bs->opaque;
> @@ -311,7 +311,7 @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t offset,
>
> offset += payload_offset;
>
> - return bdrv_co_truncate(bs->file, offset, prealloc, errp);
> + return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
> }
>
> static void block_crypto_close(BlockDriverState *bs)
> diff --git a/block/file-posix.c b/block/file-posix.c
> index f12c06de2d..d8755c5fac 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2014,7 +2014,8 @@ raw_regular_truncate(BlockDriverState *bs, int fd, int64_t offset,
> }
>
> static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> + bool exact, PreallocMode prealloc,
> + Error **errp)
> {
> BDRVRawState *s = bs->opaque;
> struct stat st;
> diff --git a/block/file-win32.c b/block/file-win32.c
> index 41f55dfece..77e8ff7b68 100644
> --- a/block/file-win32.c
> +++ b/block/file-win32.c
> @@ -468,7 +468,8 @@ static void raw_close(BlockDriverState *bs)
> }
>
> static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> + bool exact, PreallocMode prealloc,
> + Error **errp)
> {
> BDRVRawState *s = bs->opaque;
> LONG low, high;
> diff --git a/block/gluster.c b/block/gluster.c
> index 64028b2cba..4fa4a77a47 100644
> --- a/block/gluster.c
> +++ b/block/gluster.c
> @@ -1225,6 +1225,7 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
>
> static coroutine_fn int qemu_gluster_co_truncate(BlockDriverState *bs,
> int64_t offset,
> + bool exact,
> PreallocMode prealloc,
> Error **errp)
> {
> diff --git a/block/io.c b/block/io.c
> index 723655c792..5b02642b98 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -3243,8 +3243,12 @@ static void bdrv_parent_cb_resize(BlockDriverState *bs)
>
> /**
> * Truncate file to 'offset' bytes (needed only for file protocols)
> + *
> + * If 'exact' is true, the file must be resized to exactly the given
> + * 'offset'. Otherwise, it is sufficient for the node to be at least
> + * 'offset' bytes in length.
> */
> -int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
> +int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
> PreallocMode prealloc, Error **errp)
> {
> BlockDriverState *bs = child->bs;
> @@ -3300,9 +3304,9 @@ int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
> }
>
> if (drv->bdrv_co_truncate) {
> - ret = drv->bdrv_co_truncate(bs, offset, prealloc, errp);
> + ret = drv->bdrv_co_truncate(bs, offset, exact, prealloc, errp);
> } else if (bs->file && drv->is_filter) {
> - ret = bdrv_co_truncate(bs->file, offset, prealloc, errp);
> + ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
> } else {
> error_setg(errp, "Image format driver does not support resize");
> ret = -ENOTSUP;
> @@ -3333,6 +3337,7 @@ out:
> typedef struct TruncateCo {
> BdrvChild *child;
> int64_t offset;
> + bool exact;
> PreallocMode prealloc;
> Error **errp;
> int ret;
> @@ -3341,18 +3346,19 @@ typedef struct TruncateCo {
> static void coroutine_fn bdrv_truncate_co_entry(void *opaque)
> {
> TruncateCo *tco = opaque;
> - tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->prealloc,
> - tco->errp);
> + tco->ret = bdrv_co_truncate(tco->child, tco->offset, tco->exact,
> + tco->prealloc, tco->errp);
> aio_wait_kick();
> }
>
> -int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
> - Error **errp)
> +int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
> + PreallocMode prealloc, Error **errp)
> {
> Coroutine *co;
> TruncateCo tco = {
> .child = child,
> .offset = offset,
> + .exact = exact,
> .prealloc = prealloc,
> .errp = errp,
> .ret = NOT_DONE,
> diff --git a/block/iscsi.c b/block/iscsi.c
> index 506bf5f875..a90426270a 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -2122,7 +2122,8 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
> }
>
> static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> + bool exact, PreallocMode prealloc,
> + Error **errp)
> {
> IscsiLun *iscsilun = bs->opaque;
> Error *local_err = NULL;
> diff --git a/block/mirror.c b/block/mirror.c
> index fe984efb90..2cfd035571 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -873,8 +873,8 @@ static int coroutine_fn mirror_run(Job *job, Error **errp)
> }
>
> if (s->bdev_length > base_length) {
> - ret = blk_truncate(s->target, s->bdev_length, PREALLOC_MODE_OFF,
> - NULL);
> + ret = blk_truncate(s->target, s->bdev_length, false,
> + PREALLOC_MODE_OFF, NULL);
> if (ret < 0) {
> goto immediate_exit;
> }
> diff --git a/block/nfs.c b/block/nfs.c
> index f39acfdb28..ba8b41775a 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -750,7 +750,7 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
> }
>
> static int coroutine_fn
> -nfs_file_co_truncate(BlockDriverState *bs, int64_t offset,
> +nfs_file_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
> PreallocMode prealloc, Error **errp)
> {
> NFSClient *client = bs->opaque;
> diff --git a/block/parallels.c b/block/parallels.c
> index 905cac35c6..a1a92c97a4 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -203,7 +203,7 @@ static int64_t allocate_clusters(BlockDriverState *bs, int64_t sector_num,
> } else {
> ret = bdrv_truncate(bs->file,
> (s->data_end + space) << BDRV_SECTOR_BITS,
> - PREALLOC_MODE_OFF, NULL);
> + false, PREALLOC_MODE_OFF, NULL);
> }
> if (ret < 0) {
> return ret;
> @@ -487,7 +487,7 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
> res->leaks += count;
> if (fix & BDRV_FIX_LEAKS) {
> Error *local_err = NULL;
> - ret = bdrv_truncate(bs->file, res->image_end_offset,
> + ret = bdrv_truncate(bs->file, res->image_end_offset, false,
> PREALLOC_MODE_OFF, &local_err);
> if (ret < 0) {
> error_report_err(local_err);
> @@ -880,7 +880,7 @@ static void parallels_close(BlockDriverState *bs)
> if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
> s->header->inuse = 0;
> parallels_update_header(bs);
> - bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
> + bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, false,
> PREALLOC_MODE_OFF, NULL);
> }
>
> diff --git a/block/qcow.c b/block/qcow.c
> index 17705015ca..fce8989868 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c
> @@ -480,7 +480,7 @@ static int get_cluster_offset(BlockDriverState *bs,
> return -E2BIG;
> }
> ret = bdrv_truncate(bs->file, cluster_offset + s->cluster_size,
> - PREALLOC_MODE_OFF, NULL);
> + false, PREALLOC_MODE_OFF, NULL);
> if (ret < 0) {
> return ret;
> }
> @@ -1033,7 +1033,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, false,
> PREALLOC_MODE_OFF, NULL);
> if (ret < 0)
> return ret;
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index ef965d7895..7b70d8683a 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -2016,7 +2016,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
> goto resize_fail;
> }
>
> - ret = bdrv_truncate(bs->file, offset + s->cluster_size,
> + ret = bdrv_truncate(bs->file, offset + s->cluster_size, false,
> PREALLOC_MODE_OFF, &local_err);
> if (ret < 0) {
> error_report_err(local_err);
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 4978ccc7be..157b9b75d9 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2869,8 +2869,8 @@ static int coroutine_fn preallocate_co(BlockDriverState *bs, uint64_t offset,
> if (mode == PREALLOC_MODE_METADATA) {
> mode = PREALLOC_MODE_OFF;
> }
> - ret = bdrv_co_truncate(s->data_file, host_offset + cur_bytes, mode,
> - errp);
> + ret = bdrv_co_truncate(s->data_file, host_offset + cur_bytes, false,
> + mode, errp);
> if (ret < 0) {
> return ret;
> }
> @@ -3284,7 +3284,8 @@ qcow2_co_create(BlockdevCreateOptions *create_options, Error **errp)
> }
>
> /* Okay, now that we have a valid image, let's give it the right size */
> - ret = blk_truncate(blk, qcow2_opts->size, qcow2_opts->preallocation, errp);
> + ret = blk_truncate(blk, qcow2_opts->size, false, qcow2_opts->preallocation,
> + errp);
> if (ret < 0) {
> error_prepend(errp, "Could not resize image: ");
> goto out;
> @@ -3732,7 +3733,8 @@ fail:
> }
>
> static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> + bool exact, PreallocMode prealloc,
> + Error **errp)
> {
> BDRVQcow2State *s = bs->opaque;
> uint64_t old_length;
> @@ -3821,7 +3823,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> Error *local_err = NULL;
>
> bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size,
> - PREALLOC_MODE_OFF, &local_err);
> + false, PREALLOC_MODE_OFF, &local_err);
> if (local_err) {
> warn_reportf_err(local_err,
> "Failed to truncate the tail of the image: ");
> @@ -3838,7 +3840,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> switch (prealloc) {
> case PREALLOC_MODE_OFF:
> if (has_data_file(bs)) {
> - ret = bdrv_co_truncate(s->data_file, offset, prealloc, errp);
> + ret = bdrv_co_truncate(s->data_file, offset, false, prealloc, errp);
> if (ret < 0) {
> goto fail;
> }
> @@ -3923,7 +3925,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> /* Allocate the data area */
> new_file_size = allocation_start +
> nb_new_data_clusters * s->cluster_size;
> - ret = bdrv_co_truncate(bs->file, new_file_size, prealloc, errp);
> + ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, errp);
> if (ret < 0) {
> error_prepend(errp, "Failed to resize underlying file: ");
> qcow2_free_clusters(bs, allocation_start,
> @@ -4026,7 +4028,7 @@ qcow2_co_pwritev_compressed_part(BlockDriverState *bs,
> if (len < 0) {
> return len;
> }
> - return bdrv_co_truncate(bs->file, len, PREALLOC_MODE_OFF, NULL);
> + return bdrv_co_truncate(bs->file, len, false, PREALLOC_MODE_OFF, NULL);
> }
>
> if (offset_into_cluster(s, offset)) {
> @@ -4263,7 +4265,7 @@ 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, false,
> PREALLOC_MODE_OFF, &local_err);
> if (ret < 0) {
> error_report_err(local_err);
> @@ -5042,7 +5044,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> return ret;
> }
>
> - ret = blk_truncate(blk, new_size, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, new_size, false, PREALLOC_MODE_OFF, errp);
> blk_unref(blk);
> if (ret < 0) {
> return ret;
> diff --git a/block/qed.c b/block/qed.c
> index 0d8fd507aa..7c2a65af40 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -674,7 +674,7 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
> l1_size = header.cluster_size * header.table_size;
>
> /* File must start empty and grow, check truncate is supported */
> - ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, 0, false, PREALLOC_MODE_OFF, errp);
> if (ret < 0) {
> goto out;
> }
> @@ -1461,6 +1461,7 @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
>
> static int coroutine_fn bdrv_qed_co_truncate(BlockDriverState *bs,
> int64_t offset,
> + bool exact,
> PreallocMode prealloc,
> Error **errp)
> {
> diff --git a/block/raw-format.c b/block/raw-format.c
> index 42c28cc29a..57d84d0bae 100644
> --- a/block/raw-format.c
> +++ b/block/raw-format.c
> @@ -370,7 +370,8 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
> }
>
> static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> + bool exact, PreallocMode prealloc,
> + Error **errp)
> {
> BDRVRawState *s = bs->opaque;
>
> @@ -386,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
>
> s->size = offset;
> offset += s->offset;
> - return bdrv_co_truncate(bs->file, offset, prealloc, errp);
> + return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
> }
>
> static void raw_eject(BlockDriverState *bs, bool eject_flag)
> diff --git a/block/rbd.c b/block/rbd.c
> index 057af43d48..cdccf01d68 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -1086,6 +1086,7 @@ static int64_t qemu_rbd_getlength(BlockDriverState *bs)
>
> static int coroutine_fn qemu_rbd_co_truncate(BlockDriverState *bs,
> int64_t offset,
> + bool exact,
> PreallocMode prealloc,
> Error **errp)
> {
> diff --git a/block/sheepdog.c b/block/sheepdog.c
> index 773dfc6ab1..cfa84338a2 100644
> --- a/block/sheepdog.c
> +++ b/block/sheepdog.c
> @@ -2285,7 +2285,8 @@ static int64_t sd_getlength(BlockDriverState *bs)
> }
>
> static int coroutine_fn sd_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> + bool exact, PreallocMode prealloc,
> + Error **errp)
> {
> BDRVSheepdogState *s = bs->opaque;
> int ret, fd;
> @@ -2601,7 +2602,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
>
> assert(!flags);
> if (offset > s->inode.vdi_size) {
> - ret = sd_co_truncate(bs, offset, PREALLOC_MODE_OFF, NULL);
> + ret = sd_co_truncate(bs, offset, false, PREALLOC_MODE_OFF, NULL);
> if (ret < 0) {
> return ret;
> }
> diff --git a/block/ssh.c b/block/ssh.c
> index 84d01e892b..b4375cf7d2 100644
> --- a/block/ssh.c
> +++ b/block/ssh.c
> @@ -1295,7 +1295,8 @@ static int64_t ssh_getlength(BlockDriverState *bs)
> }
>
> static int coroutine_fn ssh_co_truncate(BlockDriverState *bs, int64_t offset,
> - PreallocMode prealloc, Error **errp)
> + bool exact, PreallocMode prealloc,
> + Error **errp)
> {
> BDRVSSHState *s = bs->opaque;
>
> diff --git a/block/vdi.c b/block/vdi.c
> index 806ba7f53c..0142da7233 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -874,7 +874,7 @@ static int coroutine_fn vdi_co_do_create(BlockdevCreateOptions *create_options,
> }
>
> if (image_type == VDI_TYPE_STATIC) {
> - ret = blk_truncate(blk, offset + blocks * block_size,
> + ret = blk_truncate(blk, offset + blocks * block_size, false,
> PREALLOC_MODE_OFF, errp);
> if (ret < 0) {
> error_prepend(errp, "Failed to statically allocate file");
> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
> index fdd3a7adc3..13a49c2a33 100644
> --- a/block/vhdx-log.c
> +++ b/block/vhdx-log.c
> @@ -557,8 +557,8 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s,
> ret = -EINVAL;
> goto exit;
> }
> - ret = bdrv_truncate(bs->file, new_file_size, PREALLOC_MODE_OFF,
> - NULL);
> + ret = bdrv_truncate(bs->file, new_file_size, false,
> + PREALLOC_MODE_OFF, NULL);
> if (ret < 0) {
> goto exit;
> }
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 6a09d0a55c..8ae14d97f9 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -1180,7 +1180,7 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
> return -EINVAL;
> }
>
> - return bdrv_truncate(bs->file, *new_offset + s->block_size,
> + return bdrv_truncate(bs->file, *new_offset + s->block_size, false,
> PREALLOC_MODE_OFF, NULL);
> }
>
> @@ -1619,12 +1619,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, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, data_file_offset, false, PREALLOC_MODE_OFF,
> + errp);
> if (ret < 0) {
> 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, false,
> PREALLOC_MODE_OFF, errp);
> if (ret < 0) {
> goto exit;
> diff --git a/block/vmdk.c b/block/vmdk.c
> index fed3b50c8a..20e909d997 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -2076,7 +2076,7 @@ vmdk_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
> return length;
> }
> length = QEMU_ALIGN_UP(length, BDRV_SECTOR_SIZE);
> - ret = bdrv_truncate(s->extents[i].file, length,
> + ret = bdrv_truncate(s->extents[i].file, length, false,
> PREALLOC_MODE_OFF, NULL);
> if (ret < 0) {
> return ret;
> @@ -2118,7 +2118,7 @@ static int vmdk_init_extent(BlockBackend *blk,
> int gd_buf_size;
>
> if (flat) {
> - ret = blk_truncate(blk, filesize, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, filesize, false, PREALLOC_MODE_OFF, errp);
> goto exit;
> }
> magic = cpu_to_be32(VMDK4_MAGIC);
> @@ -2181,7 +2181,7 @@ static int vmdk_init_extent(BlockBackend *blk,
> goto exit;
> }
>
> - ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9,
> + ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9, false,
> PREALLOC_MODE_OFF, errp);
> if (ret < 0) {
> goto exit;
> @@ -2523,7 +2523,7 @@ static int coroutine_fn vmdk_co_do_create(int64_t size,
> /* bdrv_pwrite write padding zeros to align to sector, we don't need that
> * for description file */
> if (desc_offset == 0) {
> - ret = blk_truncate(blk, desc_len, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, desc_len, false, PREALLOC_MODE_OFF, errp);
> if (ret < 0) {
> goto exit;
> }
> diff --git a/block/vpc.c b/block/vpc.c
> index 5cd3890780..a65550298e 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -898,7 +898,7 @@ static int create_fixed_disk(BlockBackend *blk, uint8_t *buf,
> /* Add footer to total size */
> total_size += HEADER_SIZE;
>
> - ret = blk_truncate(blk, total_size, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, total_size, false, PREALLOC_MODE_OFF, errp);
> if (ret < 0) {
> return ret;
> }
> diff --git a/blockdev.c b/blockdev.c
> index fbef6845c8..6b04eca2a4 100644
> --- a/blockdev.c
> +++ b/blockdev.c
> @@ -3225,7 +3225,7 @@ void qmp_block_resize(bool has_device, const char *device,
> }
>
> bdrv_drained_begin(bs);
> - ret = blk_truncate(blk, size, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, size, false, PREALLOC_MODE_OFF, errp);
> bdrv_drained_end(bs);
>
> out:
> diff --git a/qemu-img.c b/qemu-img.c
> index 384c6f38bc..f8694f4f72 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3823,7 +3823,7 @@ static int img_resize(int argc, char **argv)
> }
> }
>
> - ret = blk_truncate(blk, total_size, prealloc, &err);
> + ret = blk_truncate(blk, total_size, false, prealloc, &err);
> if (ret < 0) {
> error_report_err(err);
> goto out;
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 349256a5fe..5e9017c979 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1710,7 +1710,7 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv)
> return offset;
> }
>
> - ret = blk_truncate(blk, offset, PREALLOC_MODE_OFF, &local_err);
> + ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, &local_err);
> if (ret < 0) {
> error_report_err(local_err);
> return ret;
> diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
> index cfe30bab21..0c861809f0 100644
> --- a/tests/test-block-iothread.c
> +++ b/tests/test-block-iothread.c
> @@ -45,7 +45,7 @@ static int coroutine_fn bdrv_test_co_pdiscard(BlockDriverState *bs,
> }
>
> static int coroutine_fn
> -bdrv_test_co_truncate(BlockDriverState *bs, int64_t offset,
> +bdrv_test_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
> PreallocMode prealloc, Error **errp)
> {
> return 0;
> @@ -185,18 +185,18 @@ static void test_sync_op_truncate(BdrvChild *c)
> int ret;
>
> /* Normal success path */
> - ret = bdrv_truncate(c, 65536, PREALLOC_MODE_OFF, NULL);
> + ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, NULL);
> g_assert_cmpint(ret, ==, 0);
>
> /* Early error: Negative offset */
> - ret = bdrv_truncate(c, -2, PREALLOC_MODE_OFF, NULL);
> + ret = bdrv_truncate(c, -2, false, PREALLOC_MODE_OFF, NULL);
> g_assert_cmpint(ret, ==, -EINVAL);
>
> /* Error: Read-only image */
> c->bs->read_only = true;
> c->bs->open_flags &= ~BDRV_O_RDWR;
>
> - ret = bdrv_truncate(c, 65536, PREALLOC_MODE_OFF, NULL);
> + ret = bdrv_truncate(c, 65536, false, PREALLOC_MODE_OFF, NULL);
> g_assert_cmpint(ret, ==, -EACCES);
>
> c->bs->read_only = false;
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 5/8] block: Evaluate @exact in protocol drivers
2019-09-18 9:51 ` [Qemu-devel] [PATCH 5/8] block: Evaluate @exact in protocol drivers Max Reitz
@ 2019-09-18 20:51 ` Maxim Levitsky
0 siblings, 0 replies; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:51 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> We have two protocol drivers that return success when trying to shrink a
> block device even though they cannot shrink it. This behavior is now
> only allowed with exact=false, so they should return an error with
> exact=true.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/file-posix.c | 8 +++++++-
> block/iscsi.c | 7 ++++++-
> 2 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index d8755c5fac..a85f3486d1 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -2028,6 +2028,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
> }
>
> if (S_ISREG(st.st_mode)) {
> + /* Always resizes to the exact @offset */
> return raw_regular_truncate(bs, s->fd, offset, prealloc, errp);
> }
>
> @@ -2038,7 +2039,12 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
> }
>
> if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
> - if (offset > raw_getlength(bs)) {
> + int64_t cur_length = raw_getlength(bs);
> +
> + if (offset != cur_length && exact) {
> + error_setg(errp, "Cannot resize device files");
> + return -ENOTSUP;
> + } else if (offset > cur_length) {
> error_setg(errp, "Cannot grow device files");
> return -EINVAL;
> }
> diff --git a/block/iscsi.c b/block/iscsi.c
> index a90426270a..cc2072ff6c 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -2126,6 +2126,7 @@ static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
> Error **errp)
> {
> IscsiLun *iscsilun = bs->opaque;
> + int64_t cur_length;
> Error *local_err = NULL;
>
> if (prealloc != PREALLOC_MODE_OFF) {
> @@ -2145,7 +2146,11 @@ static int coroutine_fn iscsi_co_truncate(BlockDriverState *bs, int64_t offset,
> return -EIO;
> }
>
> - if (offset > iscsi_getlength(bs)) {
> + cur_length = iscsi_getlength(bs);
> + if (offset != cur_length && exact) {
> + error_setg(errp, "Cannot resize iSCSI devices");
> + return -ENOTSUP;
> + } else if (offset > cur_length) {
> error_setg(errp, "Cannot grow iSCSI devices");
> return -EINVAL;
> }
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact
2019-09-18 9:51 ` [Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact Max Reitz
@ 2019-09-18 20:51 ` Maxim Levitsky
0 siblings, 0 replies; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:51 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> When truncating a format node, the @exact parameter is generally handled
> simply by virtue of the format storing the new size in the image
> metadata. Such formats do not need to pass on the parameter to their
> file nodes.
>
> There are exceptions, though:
> - raw and crypto cannot store the image size, and thus must pass on
> @exact.
>
> - When using qcow2 with an external data file, it just makes sense to
> keep its size in sync with the qcow2 virtual disk (because the
> external data file is the virtual disk). Therefore, we should pass
> @exact when truncating it.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/crypto.c | 2 +-
> block/qcow2.c | 15 ++++++++++++++-
> block/raw-format.c | 2 +-
> 3 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/block/crypto.c b/block/crypto.c
> index e5a1a2cdf3..24823835c1 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -311,7 +311,7 @@ block_crypto_co_truncate(BlockDriverState *bs, int64_t offset, bool exact,
>
> offset += payload_offset;
>
> - return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
> + return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
> }
>
> static void block_crypto_close(BlockDriverState *bs)
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 157b9b75d9..4ef19dd29a 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3822,6 +3822,13 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> if ((last_cluster + 1) * s->cluster_size < old_file_size) {
> Error *local_err = NULL;
>
> + /*
> + * Do not pass @exact here: It will not help the user if
> + * we get an error here just because they wanted to shrink
> + * their qcow2 image (on a block device) with qemu-img.
> + * (And on the qcow2 layer, the @exact requirement is
> + * always fulfilled, so there is no need to pass it on.)
> + */
> bdrv_co_truncate(bs->file, (last_cluster + 1) * s->cluster_size,
> false, PREALLOC_MODE_OFF, &local_err);
> if (local_err) {
> @@ -3840,7 +3847,12 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> switch (prealloc) {
> case PREALLOC_MODE_OFF:
> if (has_data_file(bs)) {
> - ret = bdrv_co_truncate(s->data_file, offset, false, prealloc, errp);
> + /*
> + * If the caller wants an exact resize, the external data
> + * file should be resized to the exact target size, too,
> + * so we pass @exact here.
> + */
> + ret = bdrv_co_truncate(s->data_file, offset, exact, prealloc, errp);
> if (ret < 0) {
> goto fail;
> }
> @@ -3925,6 +3937,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
> /* Allocate the data area */
> new_file_size = allocation_start +
> nb_new_data_clusters * s->cluster_size;
> + /* Image file grows, so @exact does not matter */
> ret = bdrv_co_truncate(bs->file, new_file_size, false, prealloc, errp);
>
> if (ret < 0) {
> error_prepend(errp, "Failed to resize underlying file: ");
> diff --git a/block/raw-format.c b/block/raw-format.c
> index 57d84d0bae..3a76ec7dd2 100644
> --- a/block/raw-format.c
> +++ b/block/raw-format.c
> @@ -387,7 +387,7 @@ static int coroutine_fn raw_co_truncate(BlockDriverState *bs, int64_t offset,
>
> s->size = offset;
> offset += s->offset;
> - return bdrv_co_truncate(bs->file, offset, false, prealloc, errp);
> + return bdrv_co_truncate(bs->file, offset, exact, prealloc, errp);
> }
>
> static void raw_eject(BlockDriverState *bs, bool eject_flag)
Looks all right.
Reviewed-by: Maxim Levitsky <maximlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 7/8] block: Pass truncate exact=true where reasonable
2019-09-18 9:51 ` [Qemu-devel] [PATCH 7/8] block: Pass truncate exact=true where reasonable Max Reitz
@ 2019-09-18 20:52 ` Maxim Levitsky
2019-10-28 11:08 ` Max Reitz
0 siblings, 1 reply; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:52 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> This is a change in behavior, so all instances need a good
> justification. The comments added here should explain my reasoning.
>
> qed already had a comment that suggests it always expected
> bdrv_truncate()/blk_truncate() to behave as if exact=true were passed
> (c743849bee7 came eight months before 55b949c8476), so it was simply
> broken until now.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/parallels.c | 11 +++++++++--
> block/qcow2.c | 6 +++++-
> block/qed.c | 2 +-
> qemu-img.c | 7 ++++++-
> qemu-io-cmds.c | 7 ++++++-
> 5 files changed, 27 insertions(+), 6 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index a1a92c97a4..603211f83c 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -487,7 +487,12 @@ static int coroutine_fn parallels_co_check(BlockDriverState *bs,
> res->leaks += count;
> if (fix & BDRV_FIX_LEAKS) {
> Error *local_err = NULL;
> - ret = bdrv_truncate(bs->file, res->image_end_offset, false,
> +
> + /*
> + * In order to really repair the image, we must shrink it.
> + * That means we have to pass exact=true.
> + */
> + ret = bdrv_truncate(bs->file, res->image_end_offset, true,
> PREALLOC_MODE_OFF, &local_err);
I am not familiar with the parallels format, but in theory,
a partial fix is better that nothing.
> if (ret < 0) {
> error_report_err(local_err);
> @@ -880,7 +885,9 @@ static void parallels_close(BlockDriverState *bs)
> if ((bs->open_flags & BDRV_O_RDWR) && !(bs->open_flags & BDRV_O_INACTIVE)) {
> s->header->inuse = 0;
> parallels_update_header(bs);
> - bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, false,
> +
> + /* errors are ignored, so we might as well pass exact=true */
> + bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, true,
> PREALLOC_MODE_OFF, NULL);
Fair enough.
> }
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 4ef19dd29a..eba165de7f 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -5057,7 +5057,11 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
> return ret;
> }
>
> - ret = blk_truncate(blk, new_size, false, PREALLOC_MODE_OFF, errp);
> + /*
> + * Amending image options should ensure that the image has
> + * exactly the given new values, so pass exact=true here.
> + */
Agree.
> + ret = blk_truncate(blk, new_size, true, PREALLOC_MODE_OFF, errp);
> blk_unref(blk);
> if (ret < 0) {
> return ret;
> diff --git a/block/qed.c b/block/qed.c
> index 7c2a65af40..8005cfc305 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -674,7 +674,7 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
> l1_size = header.cluster_size * header.table_size;
>
> /* File must start empty and grow, check truncate is supported */
I would update the above comment, with something like
"QED format requires the underlying file to have the exact expected length,
which is 0 on creation"
Or something similar.
> - ret = blk_truncate(blk, 0, false, PREALLOC_MODE_OFF, errp);
> + ret = blk_truncate(blk, 0, true, PREALLOC_MODE_OFF, errp);
> if (ret < 0) {
> goto out;
> }
> diff --git a/qemu-img.c b/qemu-img.c
> index f8694f4f72..a3169b6113 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -3823,7 +3823,12 @@ static int img_resize(int argc, char **argv)
> }
> }
>
> - ret = blk_truncate(blk, total_size, false, prealloc, &err);
> + /*
> + * The user expects the image to have the desired size after
> + * resizing, so pass @exact=true. It is of no use to report
> + * success when the image has not actually been resized.
> + */
Agree.
> + ret = blk_truncate(blk, total_size, true, prealloc, &err);
> if (ret < 0) {
> error_report_err(err);
> goto out;
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index 5e9017c979..1b7e700020 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
> @@ -1710,7 +1710,12 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv)
> return offset;
> }
>
> - ret = blk_truncate(blk, offset, false, PREALLOC_MODE_OFF, &local_err);
> + /*
> + * qemu-io is a debugging tool, so let us be strict here and pass
> + * exact=true. It is better to err on the "emit more errors" side
> + * than to be overly permissive.
> + */
Also agree.
> + ret = blk_truncate(blk, offset, true, PREALLOC_MODE_OFF, &local_err);
> if (ret < 0) {
> error_report_err(local_err);
> return ret;
Other than few nitpicks which probably aren't important,
Reviewed-by: Maxim Levitsky <mlevitsky@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [Qemu-devel] [PATCH 8/8] Revert "qemu-img: Check post-truncation size"
2019-09-18 9:51 ` [Qemu-devel] [PATCH 8/8] Revert "qemu-img: Check post-truncation size" Max Reitz
@ 2019-09-18 20:52 ` Maxim Levitsky
0 siblings, 0 replies; 20+ messages in thread
From: Maxim Levitsky @ 2019-09-18 20:52 UTC (permalink / raw)
To: Max Reitz, qemu-block; +Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
> This reverts commit 5279b30392da7a3248b320c75f20c61e3a95863c.
>
> We no longer need this check because exact=true forces the block driver
> to give the image the exact size requested by the user.
Looks very good to me.
Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate()
2019-09-18 20:50 ` Maxim Levitsky
@ 2019-10-28 11:05 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-10-28 11:05 UTC (permalink / raw)
To: Maxim Levitsky, qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
[-- Attachment #1.1: Type: text/plain, Size: 5394 bytes --]
On 18.09.19 22:50, Maxim Levitsky wrote:
> On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
>> We have two drivers (iscsi and file-posix) that (in some cases) return
>> success from their .bdrv_co_truncate() implementation if the block
>> device is larger than the requested offset, but cannot be shrunk. Some
>> callers do not want that behavior, so this patch adds a new parameter
>> that they can use to turn off that behavior.
>>
>> This patch just adds the parameter and lets the block/io.c and
>> block/block-backend.c functions pass it around. All other callers
>> always pass false and none of the implementations evaluate it, so that
>> this patch does not change existing behavior. Future patches take care
>> of that.
>>
>> Suggested-by: Maxim Levitsky <mlevitsk@redhat.com>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> include/block/block.h | 6 +++---
>> include/block/block_int.h | 17 ++++++++++++++++-
>> include/sysemu/block-backend.h | 4 ++--
>> block/block-backend.c | 6 +++---
>> block/commit.c | 5 +++--
>> block/crypto.c | 8 ++++----
>> block/file-posix.c | 3 ++-
>> block/file-win32.c | 3 ++-
>> block/gluster.c | 1 +
>> block/io.c | 20 +++++++++++++-------
>> block/iscsi.c | 3 ++-
>> block/mirror.c | 4 ++--
>> block/nfs.c | 2 +-
>> block/parallels.c | 6 +++---
>> block/qcow.c | 4 ++--
>> block/qcow2-refcount.c | 2 +-
>> block/qcow2.c | 22 ++++++++++++----------
>> block/qed.c | 3 ++-
>> block/raw-format.c | 5 +++--
>> block/rbd.c | 1 +
>> block/sheepdog.c | 5 +++--
>> block/ssh.c | 3 ++-
>> block/vdi.c | 2 +-
>> block/vhdx-log.c | 4 ++--
>> block/vhdx.c | 7 ++++---
>> block/vmdk.c | 8 ++++----
>> block/vpc.c | 2 +-
>> blockdev.c | 2 +-
>> qemu-img.c | 2 +-
>> qemu-io-cmds.c | 2 +-
>> tests/test-block-iothread.c | 8 ++++----
>> 31 files changed, 102 insertions(+), 68 deletions(-)
>>
>> diff --git a/include/block/block.h b/include/block/block.h
>> index 37c9de7446..2f905ae4b7 100644
>> --- a/include/block/block.h
>> +++ b/include/block/block.h
>> @@ -346,10 +346,10 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
>> const char *backing_file);
>> void bdrv_refresh_filename(BlockDriverState *bs);
>>
>> -int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset,
>> +int coroutine_fn bdrv_co_truncate(BdrvChild *child, int64_t offset, bool exact,
>> PreallocMode prealloc, Error **errp);
>> -int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
>> - Error **errp);
>> +int bdrv_truncate(BdrvChild *child, int64_t offset, bool exact,
>> + PreallocMode prealloc, Error **errp);
>>
>> int64_t bdrv_nb_sectors(BlockDriverState *bs);
>> int64_t bdrv_getlength(BlockDriverState *bs);
>> diff --git a/include/block/block_int.h b/include/block/block_int.h
>> index 0422acdf1c..197cc6e7c3 100644
>> --- a/include/block/block_int.h
>> +++ b/include/block/block_int.h
>> @@ -334,8 +334,23 @@ struct BlockDriver {
>> * bdrv_parse_filename.
>> */
>> const char *protocol_name;
>> +
>> + /*
>> + * Truncate @bs to @offset bytes using the given @prealloc mode
>> + * when growing. Modes other than PREALLOC_MODE_OFF should be
>> + * rejected when shrinking @bs.
>> + *
>> + * If @exact is true, @bs must be resized to exactly @offset.
>> + * Otherwise, it is sufficient for @bs (if it is a host block
>> + * device and thus there is no way to resize it) to be at least
>> + * @offset bytes in length.
>> + *
>> + * If @exact is true and this function fails but would succeed
>> + * with @exact = false, it should return -ENOTSUP.
>> + */
> Thanks for adding a documentation here!
> One minor nitpick:
> I would write
>
> Otherwise, it is sufficient for @bs (for example if it is a host block
> device and thus there is no way to resize it) to be at least @offset bytes in length.
Hm, so just add the “for example”? I’d rather not add it. This would
then read as if it were OK for files that aren’t block devices to also
return success when they cannot be shrunk just because we don’t support
it. But it isn’t. If the protocol theoretically allows it and it makes
sense, drivers shouldn’t return success with exact=false simply because
we haven’t implemented it.
For example, you can shrink files over ssh, I’m sure. But our driver
doesn’t allow it. It should thus return ENOTSUP even with exact=false.
The “Return success for shrinking even when the file cannot be shrunk”
really is only for block devices, because then the user will have no
expectation that the shrinking will actually work. (For ssh, they will
expect it to work, so we must not pretend it succeeded when it didn’t.)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 7/8] block: Pass truncate exact=true where reasonable
2019-09-18 20:52 ` Maxim Levitsky
@ 2019-10-28 11:08 ` Max Reitz
0 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-10-28 11:08 UTC (permalink / raw)
To: Maxim Levitsky, qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi
[-- Attachment #1.1: Type: text/plain, Size: 1510 bytes --]
On 18.09.19 22:52, Maxim Levitsky wrote:
> On Wed, 2019-09-18 at 11:51 +0200, Max Reitz wrote:
>> This is a change in behavior, so all instances need a good
>> justification. The comments added here should explain my reasoning.
>>
>> qed already had a comment that suggests it always expected
>> bdrv_truncate()/blk_truncate() to behave as if exact=true were passed
>> (c743849bee7 came eight months before 55b949c8476), so it was simply
>> broken until now.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block/parallels.c | 11 +++++++++--
>> block/qcow2.c | 6 +++++-
>> block/qed.c | 2 +-
>> qemu-img.c | 7 ++++++-
>> qemu-io-cmds.c | 7 ++++++-
>> 5 files changed, 27 insertions(+), 6 deletions(-)
[...]
>> diff --git a/block/qed.c b/block/qed.c
>> index 7c2a65af40..8005cfc305 100644
>> --- a/block/qed.c
>> +++ b/block/qed.c
>> @@ -674,7 +674,7 @@ static int coroutine_fn bdrv_qed_co_create(BlockdevCreateOptions *opts,
>> l1_size = header.cluster_size * header.table_size;
>>
>> /* File must start empty and grow, check truncate is supported */
> I would update the above comment, with something like
>
> "QED format requires the underlying file to have the exact expected length,
> which is 0 on creation"
> Or something similar.
I’ll change it to:
The QED format associates file length with allocation status,
so a new file (which is empty) must have a length of 0.
Hope that’s OK. :-)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate()
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
` (7 preceding siblings ...)
2019-09-18 9:51 ` [Qemu-devel] [PATCH 8/8] Revert "qemu-img: Check post-truncation size" Max Reitz
@ 2019-10-28 11:10 ` Max Reitz
8 siblings, 0 replies; 20+ messages in thread
From: Max Reitz @ 2019-10-28 11:10 UTC (permalink / raw)
To: qemu-block
Cc: Kevin Wolf, Fam Zheng, qemu-devel, Stefan Hajnoczi, Maxim Levitsky
[-- Attachment #1.1: Type: text/plain, Size: 1846 bytes --]
On 18.09.19 11:51, Max Reitz wrote:
> Hi,
>
> This series is supposed to pull out some of the problems from my
> “Generic file creation fallback” series.
>
> The blk_truncate_for_formatting() function added there was buggy, as
> Maxim noted, in that it did not check whether blk_truncate() actually
> resized the block node to the target offset. One way to fix this is to
> add a parameter to it that forces the block driver to do so, and that is
> done by this series.
>
> I think this is generally useful (you can see the diff stat saldo is
> only +23 lines), because it allows us to drop a special check in
> qemu-img resize, and it fixes a bug in qed (which has relied on this
> behavior for over 8 years, but unfortunately bdrv_truncate()’s behavior
> changed quite exactly 8 years ago).
>
> However, in the process I noticed we actually don’t need
> blk_truncate_for_formatting(): The underlying problem is that some
> format drivers truncate their underlying file node to 0 before
> formatting it to drop all data. So they should pass exact=true, but
> they cannot, because this would break creation on block devices. Hence
> blk_truncate_for_formatting().
>
> It turns out, though, that three of the four drivers in question don’t
> need to truncate their file node at all. The remaining one is qed which
> simply really should pass exact=true (it’s a bug fix).
>
> (I do drop those blk_truncate() invocations in this series, because
> otherwise I feel like it is impossible to decide whether they should get
> exact=false or exact=true. Either way is wrong.)
Thanks for the review, I’ve applied the series to my block branch and
changed the comment in qed.c as requested and suggested by Maxim on patch 7:
https://git.xanclic.moe/XanClic/qemu/commits/branch/block
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-10-28 11:27 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-18 9:51 [Qemu-devel] [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
2019-09-18 9:51 ` [Qemu-devel] [PATCH 1/8] block: Handle filter truncation like native impl Max Reitz
2019-09-18 20:49 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 2/8] block/cor: Drop cor_co_truncate() Max Reitz
2019-09-18 20:49 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 3/8] block: Do not truncate file node when formatting Max Reitz
2019-09-18 20:50 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 4/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
2019-09-18 20:50 ` Maxim Levitsky
2019-10-28 11:05 ` Max Reitz
2019-09-18 9:51 ` [Qemu-devel] [PATCH 5/8] block: Evaluate @exact in protocol drivers Max Reitz
2019-09-18 20:51 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 6/8] block: Let format drivers pass @exact Max Reitz
2019-09-18 20:51 ` Maxim Levitsky
2019-09-18 9:51 ` [Qemu-devel] [PATCH 7/8] block: Pass truncate exact=true where reasonable Max Reitz
2019-09-18 20:52 ` Maxim Levitsky
2019-10-28 11:08 ` Max Reitz
2019-09-18 9:51 ` [Qemu-devel] [PATCH 8/8] Revert "qemu-img: Check post-truncation size" Max Reitz
2019-09-18 20:52 ` Maxim Levitsky
2019-10-28 11:10 ` [PATCH 0/8] block: Add @exact parameter to bdrv_co_truncate() Max Reitz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).