* [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate
@ 2017-03-13 21:39 Max Reitz
2017-03-13 21:39 ` [Qemu-devel] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate() Max Reitz
` (15 more replies)
0 siblings, 16 replies; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:39 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
=== Series dependencies ===
This series is based on Kevin's block-next branch rebased onto master
with v2 of my "block: Add errp to b{lk,drv}_truncate()" series applied
on top.
=== Actual cover letter ===
This series adds preallocation to bdrv_truncate() and subsequently
qemu-img resize. This is implemented for qcow2 and raw only, just like
preallocation for newly created images is.
There is no runtime interface for this new parameter (yet). This is
because preallocation takes time and would thus require an asynchronous
QMP truncate command. Since I hope there is no immediate need for this I
put implementing this off for later (if at all).
If preallocation fails, the image is supposed to be returned to its
original size. However, this is not always trivially possible: For qcow2
for instance, metadata preallocation may fail and then it is rather
difficult to restore the original size. Since it is rather unlikely that
this step fails (at least in "falloc" or "full" mode where all of the
space should already be preallocated in the underlying file which means
that ENOSPC should be impossible to occur) I opted to not restore the
image size in that case.
(I have started a patch to undo the changes done by qcow2's
preallocate() function, but it turned out to be rather complicated for
(in my opinion) little gain, so I decided to abandon it. If you'd like
me to continue it and include in v2 anyway, I can certainly do so,
though.)
Also something that may be added later: The parallels block driver can
make the underlying file grow by using either blk_truncate() or by
writing zeroes (runtime "prealloc-mode" parameter). It would not be
entirely trivial to make use of this new bdrv_truncate() parameter there
(because most protocol block drivers just do not support it and that
case would still need to be covered), but it is possible and may be nice
to have.
Max Reitz (16):
block: Add PreallocMode to BD.bdrv_truncate()
block: Add PreallocMode to bdrv_truncate()
block: Add PreallocMode to blk_truncate()
qemu-img: Expose PreallocMode for resizing
block/file-posix: Small fixes in raw_create()
block/file-posix: Extract raw_regular_truncate()
block/file-posix: Generalize raw_regular_truncate
block/file-posix: Preallocation for truncate
block/qcow2: Generalize preallocate()
block/qcow2: Lock s->lock in preallocate()
block/qcow2: Metadata preallocation for truncate
block/qcow2: Extract qcow2_calc_size_usage()
block/qcow2: qcow2_calc_size_usage() for truncate
block/qcow2: falloc/full preallocating growth
iotests: Add preallocated resize test for raw
iotests: Add preallocated growth test for qcow2
include/block/block.h | 3 +-
include/block/block_int.h | 3 +-
include/sysemu/block-backend.h | 3 +-
block.c | 12 +-
block/blkdebug.c | 5 +-
block/block-backend.c | 5 +-
block/commit.c | 4 +-
block/crypto.c | 4 +-
block/file-posix.c | 209 ++++++++++++++--------
block/file-win32.c | 9 +-
block/gluster.c | 6 +-
block/iscsi.c | 7 +-
block/mirror.c | 3 +-
block/nfs.c | 8 +-
block/parallels.c | 13 +-
block/qcow.c | 8 +-
block/qcow2-refcount.c | 2 +-
block/qcow2.c | 279 +++++++++++++++++++++--------
block/qed.c | 11 +-
block/raw-format.c | 5 +-
block/rbd.c | 7 +-
block/sheepdog.c | 11 +-
block/vdi.c | 3 +-
block/vhdx-log.c | 2 +-
block/vhdx.c | 8 +-
block/vmdk.c | 7 +-
block/vpc.c | 2 +-
blockdev.c | 2 +-
qemu-img.c | 33 +++-
qemu-io-cmds.c | 2 +-
qemu-img.texi | 7 +-
tests/qemu-iotests/106 | 92 ++++++++++
tests/qemu-iotests/106.out | 50 ++++++
tests/qemu-iotests/125 | 130 ++++++++++++++
tests/qemu-iotests/125.out | 386 +++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 2 +
36 files changed, 1148 insertions(+), 195 deletions(-)
create mode 100755 tests/qemu-iotests/106
create mode 100644 tests/qemu-iotests/106.out
create mode 100755 tests/qemu-iotests/125
create mode 100644 tests/qemu-iotests/125.out
--
2.12.0
^ permalink raw reply [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
@ 2017-03-13 21:39 ` Max Reitz
2017-03-20 10:10 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-20 10:18 ` Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 02/16] block: Add PreallocMode to bdrv_truncate() Max Reitz
` (14 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:39 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
Add a PreallocMode parameter to the bdrv_truncate() function implemented
by each block driver. Currently, we always pass PREALLOC_MODE_OFF and no
driver accepts anything else.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/block/block_int.h | 3 ++-
block.c | 2 +-
block/blkdebug.c | 9 ++++++++-
block/crypto.c | 8 +++++++-
block/file-posix.c | 9 ++++++++-
block/file-win32.c | 9 ++++++++-
block/gluster.c | 6 +++++-
block/iscsi.c | 7 ++++++-
block/nfs.c | 8 +++++++-
block/qcow2.c | 9 ++++++++-
block/qed.c | 9 ++++++++-
block/raw-format.c | 9 ++++++++-
block/rbd.c | 7 ++++++-
block/sheepdog.c | 11 +++++++++--
14 files changed, 91 insertions(+), 15 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index ee55d5e9cd..2615356581 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -196,7 +196,8 @@ struct BlockDriver {
int coroutine_fn (*bdrv_co_flush_to_os)(BlockDriverState *bs);
const char *protocol_name;
- int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset, Error **errp);
+ int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp);
int64_t (*bdrv_getlength)(BlockDriverState *bs);
bool has_variable_length;
diff --git a/block.c b/block.c
index 267c761ff7..57ae40a84e 100644
--- a/block.c
+++ b/block.c
@@ -3237,7 +3237,7 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
return -EACCES;
}
- ret = drv->bdrv_truncate(bs, offset, errp);
+ ret = drv->bdrv_truncate(bs, offset, PREALLOC_MODE_OFF, errp);
if (ret == 0) {
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
bdrv_dirty_bitmap_truncate(bs);
diff --git a/block/blkdebug.c b/block/blkdebug.c
index c795ae9e72..31a71a34d3 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -661,8 +661,15 @@ static int64_t blkdebug_getlength(BlockDriverState *bs)
return bdrv_getlength(bs->file->bs);
}
-static int blkdebug_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int blkdebug_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
return bdrv_truncate(bs->file, offset, errp);
}
diff --git a/block/crypto.c b/block/crypto.c
index 17b3140998..fa61aef380 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -382,12 +382,18 @@ static int block_crypto_create_generic(QCryptoBlockFormat format,
}
static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,
- Error **errp)
+ PreallocMode prealloc, Error **errp)
{
BlockCrypto *crypto = bs->opaque;
size_t payload_offset =
qcrypto_block_get_payload_offset(crypto->block);
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
offset += payload_offset;
return bdrv_truncate(bs->file, offset, errp);
diff --git a/block/file-posix.c b/block/file-posix.c
index 3a402151f1..dde8c101c8 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1384,12 +1384,19 @@ static void raw_close(BlockDriverState *bs)
}
}
-static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int raw_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
BDRVRawState *s = bs->opaque;
struct stat st;
int ret;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
if (fstat(s->fd, &st)) {
ret = -errno;
error_setg_errno(errp, -ret, "Failed to fstat() the file");
diff --git a/block/file-win32.c b/block/file-win32.c
index 3f3925623f..bf6f20d3b0 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -461,12 +461,19 @@ static void raw_close(BlockDriverState *bs)
}
}
-static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int raw_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
BDRVRawState *s = bs->opaque;
LONG low, high;
DWORD dwPtrLow;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
low = offset;
high = offset >> 32;
diff --git a/block/gluster.c b/block/gluster.c
index 00b8240562..2d4c278500 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -1085,11 +1085,15 @@ static coroutine_fn int qemu_gluster_co_rw(BlockDriverState *bs,
}
static int qemu_gluster_truncate(BlockDriverState *bs, int64_t offset,
- Error **errp)
+ PreallocMode prealloc, Error **errp)
{
int ret;
BDRVGlusterState *s = bs->opaque;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ return -ENOTSUP;
+ }
+
ret = glfs_ftruncate(s->fd, offset);
if (ret < 0) {
return -errno;
diff --git a/block/iscsi.c b/block/iscsi.c
index ab559a6f71..5d6265c4a6 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
}
}
-static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
IscsiLun *iscsilun = bs->opaque;
Error *local_err = NULL;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ return -ENOTSUP;
+ }
+
if (iscsilun->type != TYPE_DISK) {
return -ENOTSUP;
}
diff --git a/block/nfs.c b/block/nfs.c
index 57d12efc51..a44c526b80 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -757,9 +757,15 @@ static int64_t nfs_get_allocated_file_size(BlockDriverState *bs)
return (task.ret < 0 ? task.ret : st.st_blocks * 512);
}
-static int nfs_file_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int nfs_file_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
NFSClient *client = bs->opaque;
+
+ if (prealloc != PREALLOC_MODE_OFF) {
+ return -ENOTSUP;
+ }
+
return nfs_ftruncate(client->context, client->fh, offset);
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 53b0bd61a7..8d180bbf9d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2524,12 +2524,19 @@ static coroutine_fn int qcow2_co_pdiscard(BlockDriverState *bs,
return ret;
}
-static int qcow2_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
BDRVQcow2State *s = bs->opaque;
int64_t new_l1_size;
int ret;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
if (offset & 511) {
error_setg(errp, "The new size must be a multiple of 512");
return -EINVAL;
diff --git a/block/qed.c b/block/qed.c
index eb346d645b..b5102f5d7f 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1518,12 +1518,19 @@ static int coroutine_fn bdrv_qed_co_pwrite_zeroes(BlockDriverState *bs,
return cb.ret;
}
-static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int bdrv_qed_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
BDRVQEDState *s = bs->opaque;
uint64_t old_image_size;
int ret;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
if (!qed_is_image_size_valid(offset, s->header.cluster_size,
s->header.table_size)) {
error_setg(errp, "Invalid image size specified");
diff --git a/block/raw-format.c b/block/raw-format.c
index 36e65036f0..aeaa13e3f5 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -327,10 +327,17 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
}
}
-static int raw_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int raw_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
BDRVRawState *s = bs->opaque;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
if (s->has_size) {
error_setg(errp, "Cannot resize fixed-size raw disks");
return -ENOTSUP;
diff --git a/block/rbd.c b/block/rbd.c
index f7d4dd93fe..fd31f8626e 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -1028,11 +1028,16 @@ static int64_t qemu_rbd_getlength(BlockDriverState *bs)
return info.size;
}
-static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int qemu_rbd_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
BDRVRBDState *s = bs->opaque;
int r;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ return -ENOTSUP;
+ }
+
r = rbd_resize(s->image, offset);
if (r < 0) {
return r;
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 27126ea474..546708adaf 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -2133,13 +2133,20 @@ static int64_t sd_getlength(BlockDriverState *bs)
return s->inode.vdi_size;
}
-static int sd_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
+static int sd_truncate(BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, Error **errp)
{
BDRVSheepdogState *s = bs->opaque;
int ret, fd;
unsigned int datalen;
uint64_t max_vdi_size;
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Unsupported preallocation mode '%s'",
+ PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
max_vdi_size = (UINT64_C(1) << s->inode.block_size_shift) * MAX_DATA_OBJS;
if (offset < s->inode.vdi_size) {
error_setg(errp, "shrinking is not supported");
@@ -2428,7 +2435,7 @@ static coroutine_fn int sd_co_writev(BlockDriverState *bs, int64_t sector_num,
BDRVSheepdogState *s = bs->opaque;
if (offset > s->inode.vdi_size) {
- ret = sd_truncate(bs, offset, NULL);
+ ret = sd_truncate(bs, offset, PREALLOC_MODE_OFF, NULL);
if (ret < 0) {
return ret;
}
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 02/16] block: Add PreallocMode to bdrv_truncate()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
2017-03-13 21:39 ` [Qemu-devel] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate() Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 11:33 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 03/16] block: Add PreallocMode to blk_truncate() Max Reitz
` (13 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
For block drivers that just pass a truncate request to the underlying
protocol, we can now pass the preallocation mode instead of aborting if
it is not PREALLOC_MODE_OFF.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/block/block.h | 3 ++-
block.c | 12 +++++++++---
block/blkdebug.c | 8 +-------
block/block-backend.c | 2 +-
block/crypto.c | 8 +-------
block/parallels.c | 11 +++++++----
block/qcow.c | 6 ++++--
block/qcow2-refcount.c | 2 +-
block/qcow2.c | 4 ++--
block/raw-format.c | 8 +-------
block/vhdx-log.c | 2 +-
block/vhdx.c | 3 ++-
12 files changed, 32 insertions(+), 37 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 4c9ed0e43c..5042b79dc9 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -294,7 +294,8 @@ BlockDriverState *bdrv_find_backing_image(BlockDriverState *bs,
const char *backing_file);
int bdrv_get_backing_file_depth(BlockDriverState *bs);
void bdrv_refresh_filename(BlockDriverState *bs);
-int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp);
+int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
+ Error **errp);
int64_t bdrv_nb_sectors(BlockDriverState *bs);
int64_t bdrv_getlength(BlockDriverState *bs);
int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
diff --git a/block.c b/block.c
index 57ae40a84e..9dc7ad85e2 100644
--- a/block.c
+++ b/block.c
@@ -3216,7 +3216,8 @@ exit:
/**
* Truncate file to 'offset' bytes (needed only for file protocols)
*/
-int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
+int bdrv_truncate(BdrvChild *child, int64_t offset, PreallocMode prealloc,
+ Error **errp)
{
BlockDriverState *bs = child->bs;
BlockDriver *drv = bs->drv;
@@ -3237,14 +3238,19 @@ int bdrv_truncate(BdrvChild *child, int64_t offset, Error **errp)
return -EACCES;
}
- ret = drv->bdrv_truncate(bs, offset, PREALLOC_MODE_OFF, errp);
+ ret = drv->bdrv_truncate(bs, offset, prealloc, errp);
if (ret == 0) {
ret = refresh_total_sectors(bs, offset >> BDRV_SECTOR_BITS);
bdrv_dirty_bitmap_truncate(bs);
bdrv_parent_cb_resize(bs);
++bs->write_gen;
} else if (errp && !*errp) {
- error_setg_errno(errp, -ret, "Failed to resize image");
+ if (ret == -ENOTSUP) {
+ error_setg(errp, "Resize or preallocation mode not supported for "
+ "this image");
+ } else {
+ error_setg_errno(errp, -ret, "Failed to resize image");
+ }
}
return ret;
}
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 31a71a34d3..f682cbdbcb 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -664,13 +664,7 @@ static int64_t blkdebug_getlength(BlockDriverState *bs)
static int blkdebug_truncate(BlockDriverState *bs, int64_t offset,
PreallocMode prealloc, Error **errp)
{
- if (prealloc != PREALLOC_MODE_OFF) {
- error_setg(errp, "Unsupported preallocation mode '%s'",
- PreallocMode_lookup[prealloc]);
- return -ENOTSUP;
- }
-
- return bdrv_truncate(bs->file, offset, errp);
+ return bdrv_truncate(bs->file, offset, prealloc, errp);
}
static void blkdebug_refresh_filename(BlockDriverState *bs, QDict *options)
diff --git a/block/block-backend.c b/block/block-backend.c
index 24a16aadca..1bebc7f1ba 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1705,7 +1705,7 @@ int blk_truncate(BlockBackend *blk, int64_t offset, Error **errp)
return -ENOMEDIUM;
}
- return bdrv_truncate(blk->root, offset, errp);
+ return bdrv_truncate(blk->root, offset, PREALLOC_MODE_OFF, errp);
}
static void blk_pdiscard_entry(void *opaque)
diff --git a/block/crypto.c b/block/crypto.c
index fa61aef380..c24102fa7f 100644
--- a/block/crypto.c
+++ b/block/crypto.c
@@ -388,15 +388,9 @@ static int block_crypto_truncate(BlockDriverState *bs, int64_t offset,
size_t payload_offset =
qcrypto_block_get_payload_offset(crypto->block);
- if (prealloc != PREALLOC_MODE_OFF) {
- error_setg(errp, "Unsupported preallocation mode '%s'",
- PreallocMode_lookup[prealloc]);
- return -ENOTSUP;
- }
-
offset += payload_offset;
- return bdrv_truncate(bs->file, offset, errp);
+ return bdrv_truncate(bs->file, offset, prealloc, errp);
}
static void block_crypto_close(BlockDriverState *bs)
diff --git a/block/parallels.c b/block/parallels.c
index 618fc860e2..180111f71e 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -217,7 +217,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,
- NULL);
+ PREALLOC_MODE_OFF, NULL);
}
if (ret < 0) {
return ret;
@@ -451,7 +451,8 @@ static int parallels_check(BlockDriverState *bs, BdrvCheckResult *res,
res->leaks += count;
if (fix & BDRV_FIX_LEAKS) {
Error *local_err = NULL;
- ret = bdrv_truncate(bs->file, res->image_end_offset, &local_err);
+ ret = bdrv_truncate(bs->file, res->image_end_offset,
+ PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
res->check_errors++;
@@ -691,7 +692,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
goto fail_options;
}
if (!bdrv_has_zero_init(bs->file->bs) ||
- bdrv_truncate(bs->file, bdrv_getlength(bs->file->bs), NULL) != 0) {
+ bdrv_truncate(bs->file, bdrv_getlength(bs->file->bs),
+ PREALLOC_MODE_OFF, NULL) != 0) {
s->prealloc_mode = PRL_PREALLOC_MODE_FALLOCATE;
}
@@ -734,7 +736,8 @@ static void parallels_close(BlockDriverState *bs)
}
if (bs->open_flags & BDRV_O_RDWR) {
- bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS, NULL);
+ bdrv_truncate(bs->file, s->data_end << BDRV_SECTOR_BITS,
+ PREALLOC_MODE_OFF, NULL);
}
g_free(s->bat_dirty_bmap);
diff --git a/block/qcow.c b/block/qcow.c
index 5d147b962e..45e3ff1a89 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -473,7 +473,8 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
/* round to cluster size */
cluster_offset = (cluster_offset + s->cluster_size - 1) &
~(s->cluster_size - 1);
- bdrv_truncate(bs->file, cluster_offset + s->cluster_size, NULL);
+ bdrv_truncate(bs->file, cluster_offset + s->cluster_size,
+ PREALLOC_MODE_OFF, NULL);
/* if encrypted, we must initialize the cluster
content which won't be written */
if (bs->encrypted &&
@@ -916,7 +917,8 @@ 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, NULL);
+ ret = bdrv_truncate(bs->file, s->l1_table_offset + l1_length,
+ PREALLOC_MODE_OFF, NULL);
if (ret < 0)
return ret;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 4efca7ebdb..c176ffed14 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1736,7 +1736,7 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
}
ret = bdrv_truncate(bs->file, offset + s->cluster_size,
- &local_err);
+ PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
goto resize_fail;
diff --git a/block/qcow2.c b/block/qcow2.c
index 8d180bbf9d..52b9ad75fe 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2592,7 +2592,7 @@ qcow2_co_pwritev_compressed(BlockDriverState *bs, uint64_t offset,
/* align end of file to a sector boundary to ease reading with
sector based I/Os */
cluster_offset = bdrv_getlength(bs->file->bs);
- return bdrv_truncate(bs->file, cluster_offset, NULL);
+ return bdrv_truncate(bs->file, cluster_offset, PREALLOC_MODE_OFF, NULL);
}
buf = qemu_blockalign(bs, s->cluster_size);
@@ -2808,7 +2808,7 @@ static int make_completely_empty(BlockDriverState *bs)
}
ret = bdrv_truncate(bs->file, (3 + l1_clusters) * s->cluster_size,
- &local_err);
+ PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
goto fail;
diff --git a/block/raw-format.c b/block/raw-format.c
index aeaa13e3f5..a7c5a7bce8 100644
--- a/block/raw-format.c
+++ b/block/raw-format.c
@@ -332,12 +332,6 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset,
{
BDRVRawState *s = bs->opaque;
- if (prealloc != PREALLOC_MODE_OFF) {
- error_setg(errp, "Unsupported preallocation mode '%s'",
- PreallocMode_lookup[prealloc]);
- return -ENOTSUP;
- }
-
if (s->has_size) {
error_setg(errp, "Cannot resize fixed-size raw disks");
return -ENOTSUP;
@@ -350,7 +344,7 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset,
s->size = offset;
offset += s->offset;
- return bdrv_truncate(bs->file, offset, errp);
+ return bdrv_truncate(bs->file, offset, prealloc, errp);
}
static int raw_media_changed(BlockDriverState *bs)
diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index 3f4c2aa095..01278f3fc9 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -548,7 +548,7 @@ static int vhdx_log_flush(BlockDriverState *bs, BDRVVHDXState *s,
if (new_file_size % (1024*1024)) {
/* round up to nearest 1MB boundary */
new_file_size = ((new_file_size >> 20) + 1) << 20;
- bdrv_truncate(bs->file, new_file_size, NULL);
+ bdrv_truncate(bs->file, new_file_size, PREALLOC_MODE_OFF, NULL);
}
}
qemu_vfree(desc_entries);
diff --git a/block/vhdx.c b/block/vhdx.c
index e8fe3fb5e9..967c93e996 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1171,7 +1171,8 @@ static int vhdx_allocate_block(BlockDriverState *bs, BDRVVHDXState *s,
/* per the spec, the address for a block is in units of 1MB */
*new_offset = ROUND_UP(*new_offset, 1024 * 1024);
- return bdrv_truncate(bs->file, *new_offset + s->block_size, NULL);
+ return bdrv_truncate(bs->file, *new_offset + s->block_size,
+ PREALLOC_MODE_OFF, NULL);
}
/*
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 03/16] block: Add PreallocMode to blk_truncate()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
2017-03-13 21:39 ` [Qemu-devel] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate() Max Reitz
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 02/16] block: Add PreallocMode to bdrv_truncate() Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 10:33 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 04/16] qemu-img: Expose PreallocMode for resizing Max Reitz
` (12 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
blk_truncate() itself will pass that value to bdrv_truncate(), and all
callers of blk_truncate() just set the parameter to PREALLOC_MODE_OFF
for now.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
include/sysemu/block-backend.h | 3 ++-
block/block-backend.c | 5 +++--
block/commit.c | 4 ++--
block/mirror.c | 3 ++-
block/parallels.c | 2 +-
block/qcow.c | 2 +-
block/qcow2.c | 4 ++--
block/qed.c | 2 +-
block/vdi.c | 3 ++-
block/vhdx.c | 5 +++--
block/vmdk.c | 7 ++++---
block/vpc.c | 2 +-
blockdev.c | 2 +-
qemu-img.c | 2 +-
qemu-io-cmds.c | 2 +-
15 files changed, 27 insertions(+), 21 deletions(-)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 5796d1d634..9bdb216127 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -217,7 +217,8 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
int count, BdrvRequestFlags flags);
int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
int count);
-int blk_truncate(BlockBackend *blk, int64_t offset, Error **errp);
+int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
+ Error **errp);
int blk_pdiscard(BlockBackend *blk, int64_t offset, int count);
int blk_save_vmstate(BlockBackend *blk, const uint8_t *buf,
int64_t pos, int size);
diff --git a/block/block-backend.c b/block/block-backend.c
index 1bebc7f1ba..ab0b649b6a 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -1698,14 +1698,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, Error **errp)
+int blk_truncate(BlockBackend *blk, int64_t offset, PreallocMode prealloc,
+ Error **errp)
{
if (!blk_is_available(blk)) {
error_setg(errp, "No medium inserted");
return -ENOMEDIUM;
}
- return bdrv_truncate(blk->root, offset, PREALLOC_MODE_OFF, errp);
+ return bdrv_truncate(blk->root, offset, prealloc, errp);
}
static void blk_pdiscard_entry(void *opaque)
diff --git a/block/commit.c b/block/commit.c
index bfdd1b4142..9a32338b82 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -151,7 +151,7 @@ static void coroutine_fn commit_run(void *opaque)
}
if (base_len < s->common.len) {
- ret = blk_truncate(s->base, s->common.len, NULL);
+ ret = blk_truncate(s->base, s->common.len, PREALLOC_MODE_OFF, NULL);
if (ret) {
goto out;
}
@@ -508,7 +508,7 @@ 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, &local_err);
+ ret = blk_truncate(backing, length, PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
goto ro_cleanup;
diff --git a/block/mirror.c b/block/mirror.c
index a520007800..cd187eaddb 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -722,7 +722,8 @@ static void coroutine_fn mirror_run(void *opaque)
}
if (s->bdev_length > base_length) {
- ret = blk_truncate(s->target, s->bdev_length, NULL);
+ ret = blk_truncate(s->target, s->bdev_length, PREALLOC_MODE_OFF,
+ NULL);
if (ret < 0) {
goto immediate_exit;
}
diff --git a/block/parallels.c b/block/parallels.c
index 180111f71e..e4c3f877f6 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -501,7 +501,7 @@ static int parallels_create(const char *filename, QemuOpts *opts, Error **errp)
blk_set_allow_write_beyond_eof(file, true);
- ret = blk_truncate(file, 0, errp);
+ ret = blk_truncate(file, 0, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
}
diff --git a/block/qcow.c b/block/qcow.c
index 45e3ff1a89..20d160a9ac 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -834,7 +834,7 @@ static int qcow_create(const char *filename, QemuOpts *opts, Error **errp)
blk_set_allow_write_beyond_eof(qcow_blk, true);
- ret = blk_truncate(qcow_blk, 0, errp);
+ ret = blk_truncate(qcow_blk, 0, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index 52b9ad75fe..210646b60f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2294,7 +2294,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
}
/* Okay, now that we have a valid image, let's give it the right size */
- ret = blk_truncate(blk, total_size, errp);
+ ret = blk_truncate(blk, total_size, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto out;
}
@@ -3284,7 +3284,7 @@ static int qcow2_amend_options(BlockDriverState *bs, QemuOpts *opts,
return ret;
}
- ret = blk_truncate(blk, new_size, &local_err);
+ ret = blk_truncate(blk, new_size, PREALLOC_MODE_OFF, &local_err);
blk_unref(blk);
if (ret < 0) {
error_report_err(local_err);
diff --git a/block/qed.c b/block/qed.c
index b5102f5d7f..7490ecf026 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -635,7 +635,7 @@ static int qed_create(const char *filename, uint32_t cluster_size,
blk_set_allow_write_beyond_eof(blk, true);
/* File must start empty and grow, check truncate is supported */
- ret = blk_truncate(blk, 0, errp);
+ ret = blk_truncate(blk, 0, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto out;
}
diff --git a/block/vdi.c b/block/vdi.c
index d12d9cdc79..2434a9cd8d 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -832,7 +832,8 @@ static int vdi_create(const char *filename, QemuOpts *opts, Error **errp)
}
if (image_type == VDI_TYPE_STATIC) {
- ret = blk_truncate(blk, offset + blocks * block_size, errp);
+ ret = blk_truncate(blk, offset + blocks * block_size,
+ PREALLOC_MODE_OFF, errp);
if (ret < 0) {
error_prepend(errp, "Failed to statically allocate %s", filename);
goto exit;
diff --git a/block/vhdx.c b/block/vhdx.c
index 967c93e996..96933ed2f0 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -1608,12 +1608,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, errp);
+ ret = blk_truncate(blk, data_file_offset, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
}
} else if (type == VHDX_TYPE_FIXED) {
- ret = blk_truncate(blk, data_file_offset + image_size, errp);
+ ret = blk_truncate(blk, data_file_offset + image_size,
+ PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
}
diff --git a/block/vmdk.c b/block/vmdk.c
index c61b9cc8e0..7decd3ed44 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1714,7 +1714,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
blk_set_allow_write_beyond_eof(blk, true);
if (flat) {
- ret = blk_truncate(blk, filesize, errp);
+ ret = blk_truncate(blk, filesize, PREALLOC_MODE_OFF, errp);
goto exit;
}
magic = cpu_to_be32(VMDK4_MAGIC);
@@ -1777,7 +1777,8 @@ static int vmdk_create_extent(const char *filename, int64_t filesize,
goto exit;
}
- ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9, errp);
+ ret = blk_truncate(blk, le64_to_cpu(header.grain_offset) << 9,
+ PREALLOC_MODE_OFF, errp);
if (ret < 0) {
goto exit;
}
@@ -2086,7 +2087,7 @@ static int vmdk_create(const char *filename, QemuOpts *opts, Error **errp)
/* bdrv_pwrite write padding zeros to align to sector, we don't need that
* for description file */
if (desc_offset == 0) {
- ret = blk_truncate(new_blk, desc_len, errp);
+ ret = blk_truncate(new_blk, desc_len, PREALLOC_MODE_OFF, errp);
}
exit:
if (new_blk) {
diff --git a/block/vpc.c b/block/vpc.c
index ecfee77149..de71398eb4 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -858,7 +858,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, errp);
+ ret = blk_truncate(blk, total_size, PREALLOC_MODE_OFF, errp);
if (ret < 0) {
return ret;
}
diff --git a/blockdev.c b/blockdev.c
index de50d461d2..77f14f6a10 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2928,7 +2928,7 @@ void qmp_block_resize(bool has_device, const char *device,
/* complete all in-flight operations before resizing the device */
bdrv_drain_all();
- ret = blk_truncate(blk, size, errp);
+ ret = blk_truncate(blk, size, PREALLOC_MODE_OFF, errp);
out:
blk_unref(blk);
diff --git a/qemu-img.c b/qemu-img.c
index 0b0178949d..3e637a932b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -3440,7 +3440,7 @@ static int img_resize(int argc, char **argv)
goto out;
}
- ret = blk_truncate(blk, total_size, &err);
+ ret = blk_truncate(blk, total_size, PREALLOC_MODE_OFF, &err);
if (!ret) {
qprintf(quiet, "Image resized.\n");
} else {
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index 05bb0b34ec..b5d9b293b0 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -1570,7 +1570,7 @@ static int truncate_f(BlockBackend *blk, int argc, char **argv)
return 0;
}
- ret = blk_truncate(blk, offset, &local_err);
+ ret = blk_truncate(blk, offset, PREALLOC_MODE_OFF, &local_err);
if (ret < 0) {
error_report_err(local_err);
return 0;
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 04/16] qemu-img: Expose PreallocMode for resizing
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (2 preceding siblings ...)
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 03/16] block: Add PreallocMode to blk_truncate() Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 10:47 ` Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 05/16] block/file-posix: Small fixes in raw_create() Max Reitz
` (11 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
Add a --preallocation command line option to qemu-img resize which can
be used to set the PreallocMode parameter of blk_truncate().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
qemu-img.c | 33 ++++++++++++++++++++++++++++++---
qemu-img.texi | 7 ++++++-
2 files changed, 36 insertions(+), 4 deletions(-)
diff --git a/qemu-img.c b/qemu-img.c
index 3e637a932b..a5fdd30202 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -24,6 +24,7 @@
#include "qemu/osdep.h"
#include "qemu-version.h"
#include "qapi/error.h"
+#include "qapi/util.h"
#include "qapi-visit.h"
#include "qapi/qobject-output-visitor.h"
#include "qapi/qmp/qerror.h"
@@ -59,6 +60,7 @@ enum {
OPTION_PATTERN = 260,
OPTION_FLUSH_INTERVAL = 261,
OPTION_NO_DRAIN = 262,
+ OPTION_PREALLOCATION = 263,
};
typedef enum OutputFormat {
@@ -3317,9 +3319,10 @@ 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;
+ int64_t n, total_size, current_size;
bool quiet = false;
BlockBackend *blk = NULL;
+ PreallocMode prealloc = PREALLOC_MODE_OFF;
QemuOpts *param;
static QemuOptsList resize_options = {
@@ -3353,6 +3356,7 @@ static int img_resize(int argc, char **argv)
{"help", no_argument, 0, 'h'},
{"object", required_argument, 0, OPTION_OBJECT},
{"image-opts", no_argument, 0, OPTION_IMAGE_OPTS},
+ {"preallocation", required_argument, 0, OPTION_PREALLOCATION},
{0, 0, 0, 0}
};
c = getopt_long(argc, argv, "f:hq",
@@ -3382,6 +3386,15 @@ static int img_resize(int argc, char **argv)
case OPTION_IMAGE_OPTS:
image_opts = true;
break;
+ case OPTION_PREALLOCATION:
+ prealloc = qapi_enum_parse(PreallocMode_lookup, optarg,
+ PREALLOC_MODE__MAX, PREALLOC_MODE__MAX,
+ NULL);
+ if (prealloc == PREALLOC_MODE__MAX) {
+ error_report("Invalid preallocation mode '%s'", optarg);
+ return 1;
+ }
+ break;
}
}
if (optind != argc - 1) {
@@ -3429,8 +3442,16 @@ static int img_resize(int argc, char **argv)
goto out;
}
+ current_size = blk_getlength(blk);
+ if (current_size < 0) {
+ error_report("Failed to inquire current image length: %s\n",
+ strerror(-current_size));
+ ret = -1;
+ goto out;
+ }
+
if (relative) {
- total_size = blk_getlength(blk) + n * relative;
+ total_size = current_size + n * relative;
} else {
total_size = n;
}
@@ -3440,7 +3461,13 @@ static int img_resize(int argc, char **argv)
goto out;
}
- ret = blk_truncate(blk, total_size, PREALLOC_MODE_OFF, &err);
+ if (total_size <= current_size && prealloc != PREALLOC_MODE_OFF) {
+ error_report("Preallocation can only be used for growing images");
+ ret = -1;
+ goto out;
+ }
+
+ ret = blk_truncate(blk, total_size, prealloc, &err);
if (!ret) {
qprintf(quiet, "Image resized.\n");
} else {
diff --git a/qemu-img.texi b/qemu-img.texi
index c81db3e81c..ad46fa5f05 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -490,7 +490,7 @@ qemu-img rebase -b base.img diff.qcow2
At this point, @code{modified.img} can be discarded, since
@code{base.img + diff.qcow2} contains the same information.
-@item resize @var{filename} [+ | -]@var{size}
+@item resize [--preallocation=@var{prealloc}] @var{filename} [+ | -]@var{size}
Change the disk image as if it had been created with @var{size}.
@@ -502,6 +502,11 @@ After using this command to grow a disk image, you must use file system and
partitioning tools inside the VM to actually begin using the new space on the
device.
+When growing an image, the @code{--preallocation} option may be used to specify
+how the additional image area should be allocated on the host. See the format
+description in the @code{NOTES} section which values are allowed. Using this
+option may result in more data being allocated than necessary.
+
@item amend [-p] [-f @var{fmt}] [-t @var{cache}] -o @var{options} @var{filename}
Amends the image format specific @var{options} for the image file
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 05/16] block/file-posix: Small fixes in raw_create()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (3 preceding siblings ...)
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 04/16] qemu-img: Expose PreallocMode for resizing Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 10:48 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 06/16] block/file-posix: Extract raw_regular_truncate() Max Reitz
` (10 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
Variables should be declared at the start of a block, and if a certain
parameter value is not supported it may be better to return -ENOTSUP
instead of -EINVAL.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/file-posix.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index dde8c101c8..b02d8fc37b 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1670,6 +1670,8 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
#endif
case PREALLOC_MODE_FULL:
{
+ int64_t num = 0, left = total_size;
+
/*
* Knowing the final size from the beginning could allow the file
* system driver to do less allocations and possibly avoid
@@ -1681,7 +1683,6 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
goto out_close;
}
- int64_t num = 0, left = total_size;
buf = g_malloc0(65536);
while (left > 0) {
@@ -1713,7 +1714,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
}
break;
default:
- result = -EINVAL;
+ result = -ENOTSUP;
error_setg(errp, "Unsupported preallocation mode: %s",
PreallocMode_lookup[prealloc]);
break;
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 06/16] block/file-posix: Extract raw_regular_truncate()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (4 preceding siblings ...)
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 05/16] block/file-posix: Small fixes in raw_create() Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 10:49 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate Max Reitz
` (9 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
This functionality is part of raw_create() which we will be able to
reuse nicely in raw_truncate().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/file-posix.c | 144 +++++++++++++++++++++++++++++------------------------
1 file changed, 78 insertions(+), 66 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index b02d8fc37b..35a9e43f3e 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1384,6 +1384,81 @@ static void raw_close(BlockDriverState *bs)
}
}
+static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
+ Error **errp)
+{
+ int result = 0;
+ char *buf;
+
+ switch (prealloc) {
+#ifdef CONFIG_POSIX_FALLOCATE
+ case PREALLOC_MODE_FALLOC:
+ /*
+ * Truncating before posix_fallocate() makes it about twice slower on
+ * file systems that do not support fallocate(), trying to check if a
+ * block is allocated before allocating it, so don't do that here.
+ */
+ result = -posix_fallocate(fd, 0, offset);
+ if (result != 0) {
+ /* posix_fallocate() doesn't set errno. */
+ error_setg_errno(errp, -result,
+ "Could not preallocate data for the new file");
+ }
+ return result;
+#endif
+ case PREALLOC_MODE_FULL:
+ {
+ int64_t num = 0, left = offset;
+
+ /*
+ * Knowing the final size from the beginning could allow the file
+ * system driver to do less allocations and possibly avoid
+ * fragmentation of the file.
+ */
+ if (ftruncate(fd, offset) != 0) {
+ result = -errno;
+ error_setg_errno(errp, -result, "Could not resize file");
+ return result;
+ }
+
+ buf = g_malloc0(65536);
+
+ while (left > 0) {
+ num = MIN(left, 65536);
+ result = write(fd, buf, num);
+ if (result < 0) {
+ result = -errno;
+ error_setg_errno(errp, -result,
+ "Could not write to the new file");
+ break;
+ }
+ left -= result;
+ }
+ if (result >= 0) {
+ result = fsync(fd);
+ if (result < 0) {
+ result = -errno;
+ error_setg_errno(errp, -result,
+ "Could not flush new file to disk");
+ }
+ }
+ g_free(buf);
+ return result;
+ }
+ case PREALLOC_MODE_OFF:
+ if (ftruncate(fd, offset) != 0) {
+ result = -errno;
+ error_setg_errno(errp, -result, "Could not resize file");
+ }
+ return result;
+ default:
+ result = -ENOTSUP;
+ error_setg(errp, "Unsupported preallocation mode: %s",
+ PreallocMode_lookup[prealloc]);
+ return result;
+ }
+}
+
static int raw_truncate(BlockDriverState *bs, int64_t offset,
PreallocMode prealloc, Error **errp)
{
@@ -1652,72 +1727,9 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
#endif
}
- switch (prealloc) {
-#ifdef CONFIG_POSIX_FALLOCATE
- case PREALLOC_MODE_FALLOC:
- /*
- * Truncating before posix_fallocate() makes it about twice slower on
- * file systems that do not support fallocate(), trying to check if a
- * block is allocated before allocating it, so don't do that here.
- */
- result = -posix_fallocate(fd, 0, total_size);
- if (result != 0) {
- /* posix_fallocate() doesn't set errno. */
- error_setg_errno(errp, -result,
- "Could not preallocate data for the new file");
- }
- break;
-#endif
- case PREALLOC_MODE_FULL:
- {
- int64_t num = 0, left = total_size;
-
- /*
- * Knowing the final size from the beginning could allow the file
- * system driver to do less allocations and possibly avoid
- * fragmentation of the file.
- */
- if (ftruncate(fd, total_size) != 0) {
- result = -errno;
- error_setg_errno(errp, -result, "Could not resize file");
- goto out_close;
- }
-
- buf = g_malloc0(65536);
-
- while (left > 0) {
- num = MIN(left, 65536);
- result = write(fd, buf, num);
- if (result < 0) {
- result = -errno;
- error_setg_errno(errp, -result,
- "Could not write to the new file");
- break;
- }
- left -= result;
- }
- if (result >= 0) {
- result = fsync(fd);
- if (result < 0) {
- result = -errno;
- error_setg_errno(errp, -result,
- "Could not flush new file to disk");
- }
- }
- g_free(buf);
- break;
- }
- case PREALLOC_MODE_OFF:
- if (ftruncate(fd, total_size) != 0) {
- result = -errno;
- error_setg_errno(errp, -result, "Could not resize file");
- }
- break;
- default:
- result = -ENOTSUP;
- error_setg(errp, "Unsupported preallocation mode: %s",
- PreallocMode_lookup[prealloc]);
- break;
+ result = raw_regular_truncate(fd, total_size, prealloc, errp);
+ if (result < 0) {
+ goto out_close;
}
out_close:
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (5 preceding siblings ...)
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 06/16] block/file-posix: Extract raw_regular_truncate() Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 11:00 ` Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 08/16] block/file-posix: Preallocation for truncate Max Reitz
` (8 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
Currently, raw_regular_truncate() is intended for setting the size of a
newly created file. However, we also want to use it for truncating an
existing file in which case only the newly added space (when growing)
should be preallocated.
This also means that if resizing failed, we should try to restore the
original file size. This is important when using preallocation.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/file-posix.c | 73 ++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 13 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 35a9e43f3e..cd229324ba 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1384,11 +1384,39 @@ static void raw_close(BlockDriverState *bs)
}
}
-static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
+/**
+ * Truncates the given regular file @fd to @offset and, when growing, fills the
+ * new space according to @prealloc.
+ *
+ * @create must be true iff the file is new. In that case, @bs is ignored. If
+ * @create is false, @bs must be valid and correspond to the same file as @fd.
+ */
+static int raw_regular_truncate(int fd, BlockDriverState *bs, int64_t offset,
+ PreallocMode prealloc, bool create,
Error **errp)
{
int result = 0;
- char *buf;
+ int64_t current_length = 0;
+ char *buf = NULL;
+
+ assert(create || bs);
+ if (!create) {
+ BDRVRawState *s = bs->opaque;
+ assert(s->fd == fd);
+ }
+
+ if (!create) {
+ current_length = bdrv_getlength(bs);
+ if (current_length < 0) {
+ error_setg_errno(errp, -current_length,
+ "Could not inquire current length");
+ return -current_length;
+ }
+
+ if (current_length > offset && prealloc != PREALLOC_MODE_OFF) {
+ return -ENOTSUP;
+ }
+ }
switch (prealloc) {
#ifdef CONFIG_POSIX_FALLOCATE
@@ -1398,17 +1426,17 @@ static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
* file systems that do not support fallocate(), trying to check if a
* block is allocated before allocating it, so don't do that here.
*/
- result = -posix_fallocate(fd, 0, offset);
+ result = -posix_fallocate(fd, current_length, offset - current_length);
if (result != 0) {
/* posix_fallocate() doesn't set errno. */
error_setg_errno(errp, -result,
- "Could not preallocate data for the new file");
+ "Could not preallocate new data");
}
- return result;
+ goto out;
#endif
case PREALLOC_MODE_FULL:
{
- int64_t num = 0, left = offset;
+ int64_t num = 0, left = offset - current_length;
/*
* Knowing the final size from the beginning could allow the file
@@ -1418,19 +1446,27 @@ static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
if (ftruncate(fd, offset) != 0) {
result = -errno;
error_setg_errno(errp, -result, "Could not resize file");
- return result;
+ goto out;
}
buf = g_malloc0(65536);
+ result = lseek(fd, current_length, SEEK_SET);
+ if (result < 0) {
+ result = -errno;
+ error_setg_errno(errp, -result,
+ "Failed to seek to the old end of file");
+ goto out;
+ }
+
while (left > 0) {
num = MIN(left, 65536);
result = write(fd, buf, num);
if (result < 0) {
result = -errno;
error_setg_errno(errp, -result,
- "Could not write to the new file");
- break;
+ "Could not write zeros for preallocation");
+ goto out;
}
left -= result;
}
@@ -1439,11 +1475,11 @@ static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
if (result < 0) {
result = -errno;
error_setg_errno(errp, -result,
- "Could not flush new file to disk");
+ "Could not flush file to disk");
+ goto out;
}
}
- g_free(buf);
- return result;
+ goto out;
}
case PREALLOC_MODE_OFF:
if (ftruncate(fd, offset) != 0) {
@@ -1457,6 +1493,17 @@ static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
PreallocMode_lookup[prealloc]);
return result;
}
+
+out:
+ if (result < 0 && !create) {
+ if (ftruncate(fd, current_length) < 0) {
+ error_report("Failed to restore old file length: %s",
+ strerror(errno));
+ }
+ }
+
+ g_free(buf);
+ return result;
}
static int raw_truncate(BlockDriverState *bs, int64_t offset,
@@ -1727,7 +1774,7 @@ static int raw_create(const char *filename, QemuOpts *opts, Error **errp)
#endif
}
- result = raw_regular_truncate(fd, total_size, prealloc, errp);
+ result = raw_regular_truncate(fd, NULL, total_size, prealloc, true, errp);
if (result < 0) {
goto out_close;
}
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 08/16] block/file-posix: Preallocation for truncate
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (6 preceding siblings ...)
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 11:01 ` Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 09/16] block/qcow2: Generalize preallocate() Max Reitz
` (7 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
By using raw_regular_truncate() in raw_truncate(), we can now easily
support preallocation.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/file-posix.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index cd229324ba..c181f13e98 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -1513,12 +1513,6 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset,
struct stat st;
int ret;
- if (prealloc != PREALLOC_MODE_OFF) {
- error_setg(errp, "Unsupported preallocation mode '%s'",
- PreallocMode_lookup[prealloc]);
- return -ENOTSUP;
- }
-
if (fstat(s->fd, &st)) {
ret = -errno;
error_setg_errno(errp, -ret, "Failed to fstat() the file");
@@ -1526,12 +1520,16 @@ static int raw_truncate(BlockDriverState *bs, int64_t offset,
}
if (S_ISREG(st.st_mode)) {
- if (ftruncate(s->fd, offset) < 0) {
- ret = -errno;
- error_setg_errno(errp, -ret, "Failed to resize the file");
- return ret;
- }
- } else if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
+ return raw_regular_truncate(s->fd, bs, offset, prealloc, false, errp);
+ }
+
+ if (prealloc != PREALLOC_MODE_OFF) {
+ error_setg(errp, "Preallocation mode '%s' unsupported for this "
+ "non-regular file", PreallocMode_lookup[prealloc]);
+ return -ENOTSUP;
+ }
+
+ if (S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode)) {
if (offset > raw_getlength(bs)) {
error_setg(errp, "Cannot grow device files");
return -EINVAL;
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 09/16] block/qcow2: Generalize preallocate()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (7 preceding siblings ...)
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 08/16] block/file-posix: Preallocation for truncate Max Reitz
@ 2017-03-13 21:40 ` Max Reitz
2017-03-20 11:04 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 10/16] block/qcow2: Lock s->lock in preallocate() Max Reitz
` (6 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:40 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
This patch adds two new parameters to the preallocate() function so we
will be able to use it not just for preallocating a new image but also
for preallocated image growth.
The offset parameter allows the caller to specify a virtual offset from
which to start preallocating. For newly created images this is always 0,
but for preallocating growth this will be the old image length.
The new_length parameter specifies the supposed new length of the image
(basically the "end offset" for preallocation). During image truncation,
bdrv_getlength() will return the old image length so we cannot rely on
its return value then.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 210646b60f..9abdf9e0fb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2034,17 +2034,16 @@ static int qcow2_change_backing_file(BlockDriverState *bs,
return qcow2_update_header(bs);
}
-static int preallocate(BlockDriverState *bs)
+static int preallocate(BlockDriverState *bs, int64_t offset, int64_t new_length)
{
uint64_t bytes;
- uint64_t offset;
uint64_t host_offset = 0;
unsigned int cur_bytes;
int ret;
QCowL2Meta *meta;
- bytes = bdrv_getlength(bs);
- offset = 0;
+ assert(offset <= new_length);
+ bytes = new_length - offset;
while (bytes) {
cur_bytes = MIN(bytes, INT_MAX);
@@ -2313,7 +2312,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
if (prealloc != PREALLOC_MODE_OFF) {
BDRVQcow2State *s = blk_bs(blk)->opaque;
qemu_co_mutex_lock(&s->lock);
- ret = preallocate(blk_bs(blk));
+ ret = preallocate(blk_bs(blk), 0, total_size);
qemu_co_mutex_unlock(&s->lock);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not preallocate metadata");
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 10/16] block/qcow2: Lock s->lock in preallocate()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (8 preceding siblings ...)
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 09/16] block/qcow2: Generalize preallocate() Max Reitz
@ 2017-03-13 21:41 ` Max Reitz
2017-03-20 11:05 ` Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 11/16] block/qcow2: Metadata preallocation for truncate Max Reitz
` (5 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:41 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
preallocate() is and will be called only from places that do not lock
s->lock: Currently that is qcow2_create2(), as of a future patch it will
be called from qcow2_truncate(), too.
It therefore makes sense to move locking that mutex into preallocate()
itself.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 9abdf9e0fb..5479fed938 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2036,12 +2036,15 @@ static int qcow2_change_backing_file(BlockDriverState *bs,
static int preallocate(BlockDriverState *bs, int64_t offset, int64_t new_length)
{
+ BDRVQcow2State *s = bs->opaque;
uint64_t bytes;
uint64_t host_offset = 0;
unsigned int cur_bytes;
int ret;
QCowL2Meta *meta;
+ qemu_co_mutex_lock(&s->lock);
+
assert(offset <= new_length);
bytes = new_length - offset;
@@ -2050,7 +2053,7 @@ static int preallocate(BlockDriverState *bs, int64_t offset, int64_t new_length)
ret = qcow2_alloc_cluster_offset(bs, offset, &cur_bytes,
&host_offset, &meta);
if (ret < 0) {
- return ret;
+ goto done;
}
while (meta) {
@@ -2060,7 +2063,7 @@ static int preallocate(BlockDriverState *bs, int64_t offset, int64_t new_length)
if (ret < 0) {
qcow2_free_any_clusters(bs, meta->alloc_offset,
meta->nb_clusters, QCOW2_DISCARD_NEVER);
- return ret;
+ goto done;
}
/* There are no dependent requests, but we need to remove our
@@ -2087,11 +2090,15 @@ static int preallocate(BlockDriverState *bs, int64_t offset, int64_t new_length)
ret = bdrv_pwrite(bs->file, (host_offset + cur_bytes) - 1,
&data, 1);
if (ret < 0) {
- return ret;
+ goto done;
}
}
- return 0;
+ ret = 0;
+
+done:
+ qemu_co_mutex_unlock(&s->lock);
+ return ret;
}
static int qcow2_create2(const char *filename, int64_t total_size,
@@ -2310,10 +2317,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
/* And if we're supposed to preallocate metadata, do that now */
if (prealloc != PREALLOC_MODE_OFF) {
- BDRVQcow2State *s = blk_bs(blk)->opaque;
- qemu_co_mutex_lock(&s->lock);
ret = preallocate(blk_bs(blk), 0, total_size);
- qemu_co_mutex_unlock(&s->lock);
if (ret < 0) {
error_setg_errno(errp, -ret, "Could not preallocate metadata");
goto out;
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 11/16] block/qcow2: Metadata preallocation for truncate
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (9 preceding siblings ...)
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 10/16] block/qcow2: Lock s->lock in preallocate() Max Reitz
@ 2017-03-13 21:41 ` Max Reitz
2017-03-20 11:23 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 12/16] block/qcow2: Extract qcow2_calc_size_usage() Max Reitz
` (4 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:41 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
We can support PREALLOC_MODE_METADATA by invoking preallocate() in
qcow2_truncate().
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 5479fed938..5dc95ff0a5 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2531,10 +2531,11 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
PreallocMode prealloc, Error **errp)
{
BDRVQcow2State *s = bs->opaque;
+ uint64_t old_length;
int64_t new_l1_size;
int ret;
- if (prealloc != PREALLOC_MODE_OFF) {
+ if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA) {
error_setg(errp, "Unsupported preallocation mode '%s'",
PreallocMode_lookup[prealloc]);
return -ENOTSUP;
@@ -2551,8 +2552,10 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
return -ENOTSUP;
}
+ old_length = bs->total_sectors * 512;
+
/* shrinking is currently not supported */
- if (offset < bs->total_sectors * 512) {
+ if (offset < old_length) {
error_setg(errp, "qcow2 doesn't support shrinking images yet");
return -ENOTSUP;
}
@@ -2564,6 +2567,23 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
return ret;
}
+ switch (prealloc) {
+ case PREALLOC_MODE_OFF:
+ break;
+
+ case PREALLOC_MODE_METADATA:
+ ret = preallocate(bs, old_length, offset);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Preallocation failed, image may now "
+ "occupy more space than necessary");
+ return ret;
+ }
+ break;
+
+ default:
+ g_assert_not_reached();
+ }
+
/* write updated header.size */
offset = cpu_to_be64(offset);
ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size),
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 12/16] block/qcow2: Extract qcow2_calc_size_usage()
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (10 preceding siblings ...)
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 11/16] block/qcow2: Metadata preallocation for truncate Max Reitz
@ 2017-03-13 21:41 ` Max Reitz
2017-03-20 9:43 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate Max Reitz
` (3 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:41 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
We will need very similar functionality for full/falloc preallocation in
qcow2_truncate(). Although we will not be able to reuse much of the
actual code, it still makes sense to keep all of this in one place.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 126 +++++++++++++++++++++++++++++++---------------------------
1 file changed, 68 insertions(+), 58 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 5dc95ff0a5..21b2b3cd53 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2101,6 +2101,70 @@ done:
return ret;
}
+static uint64_t qcow2_calc_size_usage(uint64_t new_size,
+ int cluster_bits, int refcount_order)
+{
+ size_t cluster_size = 1u << cluster_bits;
+
+ /* Note: The following calculation does not need to be exact; if it is a
+ * bit off, either some bytes will be "leaked" (which is fine) or we
+ * will need to increase the file size by some bytes (which is fine,
+ * too, as long as the bulk is allocated here). Therefore, using
+ * floating point arithmetic is fine. */
+ int64_t meta_size = 0;
+ uint64_t nreftablee, nrefblocke, nl1e, nl2e;
+ uint64_t aligned_total_size = align_offset(new_size, cluster_size);
+ int refblock_bits, refblock_size;
+ /* refcount entry size in bytes */
+ double rces = (1 << refcount_order) / 8.;
+
+ /* see qcow2_open() */
+ refblock_bits = cluster_bits - (refcount_order - 3);
+ refblock_size = 1 << refblock_bits;
+
+ /* header: 1 cluster */
+ meta_size += cluster_size;
+
+ /* total size of L2 tables */
+ nl2e = aligned_total_size / cluster_size;
+ nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
+ meta_size += nl2e * sizeof(uint64_t);
+
+ /* total size of L1 tables */
+ nl1e = nl2e * sizeof(uint64_t) / cluster_size;
+ nl1e = align_offset(nl1e, cluster_size / sizeof(uint64_t));
+ meta_size += nl1e * sizeof(uint64_t);
+
+ /* total size of refcount blocks
+ *
+ * note: every host cluster is reference-counted, including metadata
+ * (even refcount blocks are recursively included).
+ * Let:
+ * a = total_size (this is the guest disk size)
+ * m = meta size not including refcount blocks and refcount tables
+ * c = cluster size
+ * y1 = number of refcount blocks entries
+ * y2 = meta size including everything
+ * rces = refcount entry size in bytes
+ * then,
+ * y1 = (y2 + a)/c
+ * y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
+ * we can get y1:
+ * y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
+ */
+ nrefblocke = (aligned_total_size + meta_size + cluster_size)
+ / (cluster_size - rces - rces * sizeof(uint64_t)
+ / cluster_size);
+ meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
+
+ /* total size of refcount tables */
+ nreftablee = nrefblocke / refblock_size;
+ nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
+ meta_size += nreftablee * sizeof(uint64_t);
+
+ return aligned_total_size + meta_size;
+}
+
static int qcow2_create2(const char *filename, int64_t total_size,
const char *backing_file, const char *backing_format,
int flags, size_t cluster_size, PreallocMode prealloc,
@@ -2139,64 +2203,10 @@ static int qcow2_create2(const char *filename, int64_t total_size,
int ret;
if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
- /* Note: The following calculation does not need to be exact; if it is a
- * bit off, either some bytes will be "leaked" (which is fine) or we
- * will need to increase the file size by some bytes (which is fine,
- * too, as long as the bulk is allocated here). Therefore, using
- * floating point arithmetic is fine. */
- int64_t meta_size = 0;
- uint64_t nreftablee, nrefblocke, nl1e, nl2e;
- int64_t aligned_total_size = align_offset(total_size, cluster_size);
- int refblock_bits, refblock_size;
- /* refcount entry size in bytes */
- double rces = (1 << refcount_order) / 8.;
-
- /* see qcow2_open() */
- refblock_bits = cluster_bits - (refcount_order - 3);
- refblock_size = 1 << refblock_bits;
-
- /* header: 1 cluster */
- meta_size += cluster_size;
-
- /* total size of L2 tables */
- nl2e = aligned_total_size / cluster_size;
- nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
- meta_size += nl2e * sizeof(uint64_t);
-
- /* total size of L1 tables */
- nl1e = nl2e * sizeof(uint64_t) / cluster_size;
- nl1e = align_offset(nl1e, cluster_size / sizeof(uint64_t));
- meta_size += nl1e * sizeof(uint64_t);
-
- /* total size of refcount blocks
- *
- * note: every host cluster is reference-counted, including metadata
- * (even refcount blocks are recursively included).
- * Let:
- * a = total_size (this is the guest disk size)
- * m = meta size not including refcount blocks and refcount tables
- * c = cluster size
- * y1 = number of refcount blocks entries
- * y2 = meta size including everything
- * rces = refcount entry size in bytes
- * then,
- * y1 = (y2 + a)/c
- * y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
- * we can get y1:
- * y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
- */
- nrefblocke = (aligned_total_size + meta_size + cluster_size)
- / (cluster_size - rces - rces * sizeof(uint64_t)
- / cluster_size);
- meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
-
- /* total size of refcount tables */
- nreftablee = nrefblocke / refblock_size;
- nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
- meta_size += nreftablee * sizeof(uint64_t);
-
- qemu_opt_set_number(opts, BLOCK_OPT_SIZE,
- aligned_total_size + meta_size, &error_abort);
+ uint64_t file_size = qcow2_calc_size_usage(total_size, cluster_bits,
+ refcount_order);
+
+ qemu_opt_set_number(opts, BLOCK_OPT_SIZE, file_size, &error_abort);
qemu_opt_set(opts, BLOCK_OPT_PREALLOC, PreallocMode_lookup[prealloc],
&error_abort);
}
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (11 preceding siblings ...)
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 12/16] block/qcow2: Extract qcow2_calc_size_usage() Max Reitz
@ 2017-03-13 21:41 ` Max Reitz
2017-03-20 11:26 ` Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth Max Reitz
` (2 subsequent siblings)
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:41 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
This patch extends qcow2_calc_size_usage() so it can calculate the
additional space needed for preallocating image growth.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 137 +++++++++++++++++++++++++++++++++++++++++-----------------
1 file changed, 98 insertions(+), 39 deletions(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 21b2b3cd53..80fb815b15 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2101,7 +2101,15 @@ done:
return ret;
}
-static uint64_t qcow2_calc_size_usage(uint64_t new_size,
+/**
+ * Returns the number of bytes that must be allocated in the underlying file
+ * to accomodate an image growth from @current_size to @new_size.
+ *
+ * @current_size must be 0 when creating a new image. In that case, @bs is
+ * ignored; otherwise it must be valid.
+ */
+static uint64_t qcow2_calc_size_usage(BlockDriverState *bs,
+ uint64_t current_size, uint64_t new_size,
int cluster_bits, int refcount_order)
{
size_t cluster_size = 1u << cluster_bits;
@@ -2122,47 +2130,97 @@ static uint64_t qcow2_calc_size_usage(uint64_t new_size,
refblock_bits = cluster_bits - (refcount_order - 3);
refblock_size = 1 << refblock_bits;
- /* header: 1 cluster */
- meta_size += cluster_size;
-
- /* total size of L2 tables */
- nl2e = aligned_total_size / cluster_size;
- nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
- meta_size += nl2e * sizeof(uint64_t);
+ if (!current_size) {
+ /* header: 1 cluster */
+ meta_size += cluster_size;
+
+ /* total size of L2 tables */
+ nl2e = aligned_total_size / cluster_size;
+ nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
+ meta_size += nl2e * sizeof(uint64_t);
+
+ /* total size of L1 tables */
+ nl1e = nl2e * sizeof(uint64_t) / cluster_size;
+ nl1e = align_offset(nl1e, cluster_size / sizeof(uint64_t));
+ meta_size += nl1e * sizeof(uint64_t);
+
+ /* total size of refcount blocks
+ *
+ * note: every host cluster is reference-counted, including metadata
+ * (even refcount blocks are recursively included).
+ * Let:
+ * a = total_size (this is the guest disk size)
+ * m = meta size not including refcount blocks and refcount tables
+ * c = cluster size
+ * y1 = number of refcount blocks entries
+ * y2 = meta size including everything
+ * rces = refcount entry size in bytes
+ * then,
+ * y1 = (y2 + a)/c
+ * y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
+ * we can get y1:
+ * y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
+ */
+ nrefblocke = (aligned_total_size + meta_size + cluster_size)
+ / (cluster_size - rces - rces * sizeof(uint64_t)
+ / cluster_size);
+ meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
- /* total size of L1 tables */
- nl1e = nl2e * sizeof(uint64_t) / cluster_size;
- nl1e = align_offset(nl1e, cluster_size / sizeof(uint64_t));
- meta_size += nl1e * sizeof(uint64_t);
+ /* total size of refcount tables */
+ nreftablee = nrefblocke / refblock_size;
+ nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
+ meta_size += nreftablee * sizeof(uint64_t);
- /* total size of refcount blocks
- *
- * note: every host cluster is reference-counted, including metadata
- * (even refcount blocks are recursively included).
- * Let:
- * a = total_size (this is the guest disk size)
- * m = meta size not including refcount blocks and refcount tables
- * c = cluster size
- * y1 = number of refcount blocks entries
- * y2 = meta size including everything
- * rces = refcount entry size in bytes
- * then,
- * y1 = (y2 + a)/c
- * y2 = y1 * rces + y1 * rces * sizeof(u64) / c + m
- * we can get y1:
- * y1 = (a + m) / (c - rces - rces * sizeof(u64) / c)
- */
- nrefblocke = (aligned_total_size + meta_size + cluster_size)
- / (cluster_size - rces - rces * sizeof(uint64_t)
- / cluster_size);
- meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
+ return aligned_total_size + meta_size;
+ } else {
+ BDRVQcow2State *s = bs->opaque;
+ uint64_t aligned_cur_size = align_offset(current_size, cluster_size);
+ uint64_t creftable_length;
+ uint64_t i;
+
+ /* new total size of L2 tables */
+ nl2e = aligned_total_size / cluster_size;
+ nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
+ meta_size += nl2e * sizeof(uint64_t);
+
+ /* Subtract L2 tables which are already present */
+ for (i = 0; i < s->l1_size; i++) {
+ if (s->l1_table[i] & L1E_OFFSET_MASK) {
+ meta_size -= cluster_size;
+ }
+ }
- /* total size of refcount tables */
- nreftablee = nrefblocke / refblock_size;
- nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
- meta_size += nreftablee * sizeof(uint64_t);
+ /* Do not add L1 table size because the only caller of this path
+ * (qcow2_truncate) has increased its size already. */
- return aligned_total_size + meta_size;
+ /* Calculate size of the additional refblocks (this assumes that all of
+ * the existing image is covered by refblocks, which is extremely
+ * likely); this may result in overallocation because parts of the newly
+ * added space may be covered by existing refblocks, but that is fine.
+ *
+ * This only considers the newly added space. Since we cannot update the
+ * reftable in-place, we will have to able to store both the old and the
+ * new one at the same time, though. Therefore, we need to add the size
+ * of the old reftable here.
+ */
+ creftable_length = ROUND_UP(s->refcount_table_size * sizeof(uint64_t),
+ cluster_size);
+ nrefblocke = ((aligned_total_size - aligned_cur_size) + meta_size +
+ creftable_length + cluster_size)
+ / (cluster_size - rces -
+ rces * sizeof(uint64_t) / cluster_size);
+ meta_size += DIV_ROUND_UP(nrefblocke, refblock_size) * cluster_size;
+
+ /* total size of the new refcount table (again, may be too much because
+ * it assumes that the new area is not covered by any refcount blocks
+ * yet) */
+ nreftablee = s->max_refcount_table_index + 1 +
+ nrefblocke / refblock_size;
+ nreftablee = align_offset(nreftablee, cluster_size / sizeof(uint64_t));
+ meta_size += nreftablee * sizeof(uint64_t);
+
+ return (aligned_total_size - aligned_cur_size) + meta_size;
+ }
}
static int qcow2_create2(const char *filename, int64_t total_size,
@@ -2203,7 +2261,8 @@ static int qcow2_create2(const char *filename, int64_t total_size,
int ret;
if (prealloc == PREALLOC_MODE_FULL || prealloc == PREALLOC_MODE_FALLOC) {
- uint64_t file_size = qcow2_calc_size_usage(total_size, cluster_bits,
+ uint64_t file_size = qcow2_calc_size_usage(NULL, 0, total_size,
+ cluster_bits,
refcount_order);
qemu_opt_set_number(opts, BLOCK_OPT_SIZE, file_size, &error_abort);
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (12 preceding siblings ...)
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate Max Reitz
@ 2017-03-13 21:41 ` Max Reitz
2017-03-20 11:29 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 15/16] iotests: Add preallocated resize test for raw Max Reitz
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 16/16] iotests: Add preallocated growth test for qcow2 Max Reitz
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:41 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
Implement the preallocation modes falloc and full for growing qcow2
images.
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
block/qcow2.c | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/block/qcow2.c b/block/qcow2.c
index 80fb815b15..b6b08d70da 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2604,7 +2604,9 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
int64_t new_l1_size;
int ret;
- if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA) {
+ if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
+ prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
+ {
error_setg(errp, "Unsupported preallocation mode '%s'",
PreallocMode_lookup[prealloc]);
return -ENOTSUP;
@@ -2649,6 +2651,38 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
}
break;
+ case PREALLOC_MODE_FALLOC:
+ case PREALLOC_MODE_FULL:
+ {
+ uint64_t additional_size = qcow2_calc_size_usage(bs, old_length, offset,
+ s->cluster_bits,
+ s->refcount_order);
+ int64_t old_file_length = bdrv_getlength(bs->file->bs);
+ if (old_file_length < 0) {
+ error_setg(errp, "Failed to inquire file length");
+ return old_file_length;
+ }
+
+ old_file_length = ROUND_UP(old_file_length, s->cluster_size);
+
+ ret = bdrv_truncate(bs->file, old_file_length + additional_size,
+ prealloc, errp);
+ if (ret < 0) {
+ error_prepend(errp, "Failed to resize underlying file");
+ return ret;
+ }
+
+ /* Ensure that preallocation is done in the preallocated area */
+ s->free_cluster_index = old_file_length / s->cluster_size;
+ ret = preallocate(bs, old_length, offset);
+ if (ret < 0) {
+ error_setg_errno(errp, -ret, "Preallocation failed, image may now "
+ "occupy more space than necessary");
+ return ret;
+ }
+ break;
+ }
+
default:
g_assert_not_reached();
}
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 15/16] iotests: Add preallocated resize test for raw
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (13 preceding siblings ...)
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth Max Reitz
@ 2017-03-13 21:41 ` Max Reitz
2017-03-20 11:30 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 16/16] iotests: Add preallocated growth test for qcow2 Max Reitz
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:41 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/106 | 92 ++++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/106.out | 50 +++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 143 insertions(+)
create mode 100755 tests/qemu-iotests/106
create mode 100644 tests/qemu-iotests/106.out
diff --git a/tests/qemu-iotests/106 b/tests/qemu-iotests/106
new file mode 100755
index 0000000000..32649578fb
--- /dev/null
+++ b/tests/qemu-iotests/106
@@ -0,0 +1,92 @@
+#!/bin/bash
+#
+# Test preallocated resize of raw images
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+here=$PWD
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment and filters
+. ./common.rc
+. ./common.filter
+
+_supported_fmt raw
+_supported_proto file
+_supported_os Linux
+
+# in kB
+CREATION_SIZE=128
+GROWTH_SIZE=256
+
+echo '=== Testing image growth ==='
+
+for create_mode in off falloc full; do
+ for growth_mode in off falloc full; do
+ echo
+ echo "--- create_mode=$create_mode growth_mode=$growth_mode ---"
+
+ IMGOPTS="preallocation=$create_mode" _make_test_img ${CREATION_SIZE}K
+ $QEMU_IMG resize -f "$IMGFMT" --preallocation=$growth_mode "$TEST_IMG" +${GROWTH_SIZE}K
+
+ expected_size=0
+ if [ $create_mode != off ]; then
+ expected_size=$CREATION_SIZE
+ fi
+ if [ $growth_mode != off ]; then
+ expected_size=$((expected_size + $GROWTH_SIZE))
+ fi
+
+ actual_size=$($QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | grep 'disk size')
+ actual_size=$(echo "$actual_size" | sed -e 's/^[^0-9]*\([0-9]\+\).*$/\1/')
+
+ # The actual size may exceed the expected size, depending on the file
+ # system. Therefore we just test that the actual size is at least what
+ # we expect.
+ if [ $actual_size -lt $expected_size ]; then
+ echo "ERROR: Image should have at least ${expected_size}K, but has ${actual_size}K"
+ fi
+ done
+done
+
+echo
+echo '=== Testing image shrinking ==='
+
+# None of this should work except for "off", because other modes cannot be used
+# for shrinking
+for growth_mode in falloc full off; do
+ echo
+ echo "--- growth_mode=$growth_mode ---"
+ $QEMU_IMG resize -f "$IMGFMT" --preallocation=$growth_mode "$TEST_IMG" -${GROWTH_SIZE}K
+done
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/106.out b/tests/qemu-iotests/106.out
new file mode 100644
index 0000000000..0a42312301
--- /dev/null
+++ b/tests/qemu-iotests/106.out
@@ -0,0 +1,50 @@
+QA output created by 106
+=== Testing image growth ===
+
+--- create_mode=off growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=off
+Image resized.
+
+--- create_mode=off growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=off
+Image resized.
+
+--- create_mode=off growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=off
+Image resized.
+
+--- create_mode=falloc growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=falloc
+Image resized.
+
+--- create_mode=falloc growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=falloc
+Image resized.
+
+--- create_mode=falloc growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=falloc
+Image resized.
+
+--- create_mode=full growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=full
+Image resized.
+
+--- create_mode=full growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=full
+Image resized.
+
+--- create_mode=full growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=131072 preallocation=full
+Image resized.
+
+=== Testing image shrinking ===
+
+--- growth_mode=falloc ---
+qemu-img: Preallocation can only be used for growing images
+
+--- growth_mode=full ---
+qemu-img: Preallocation can only be used for growing images
+
+--- growth_mode=off ---
+Image resized.
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1f4bf03185..1a89bebe14 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -112,6 +112,7 @@
103 rw auto quick
104 rw auto
105 rw auto quick
+106 rw auto quick
107 rw auto quick
108 rw auto quick
109 rw auto
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* [Qemu-devel] [PATCH for-2.10 16/16] iotests: Add preallocated growth test for qcow2
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
` (14 preceding siblings ...)
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 15/16] iotests: Add preallocated resize test for raw Max Reitz
@ 2017-03-13 21:41 ` Max Reitz
2017-03-20 11:31 ` Stefan Hajnoczi
15 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-13 21:41 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, Max Reitz, Kevin Wolf
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
tests/qemu-iotests/125 | 130 +++++++++++++++
tests/qemu-iotests/125.out | 386 +++++++++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/group | 1 +
3 files changed, 517 insertions(+)
create mode 100755 tests/qemu-iotests/125
create mode 100644 tests/qemu-iotests/125.out
diff --git a/tests/qemu-iotests/125 b/tests/qemu-iotests/125
new file mode 100755
index 0000000000..9424313e82
--- /dev/null
+++ b/tests/qemu-iotests/125
@@ -0,0 +1,130 @@
+#!/bin/bash
+#
+# Test preallocated growth of qcow2 images
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=mreitz@redhat.com
+
+seq=$(basename $0)
+echo "QA output created by $seq"
+
+here=$PWD
+status=1 # failure is the default!
+
+_cleanup()
+{
+ _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+get_image_size_on_host()
+{
+ $QEMU_IMG info -f "$IMGFMT" "$TEST_IMG" | grep "disk size" \
+ | sed -e 's/^[^0-9]*\([0-9]\+\).*$/\1/'
+}
+
+# get standard environment and filters
+. ./common.rc
+. ./common.filter
+
+_supported_fmt qcow2
+_supported_proto file
+_supported_os Linux
+
+if [ -z "$TEST_IMG_FILE" ]; then
+ TEST_IMG_FILE=$TEST_IMG
+fi
+
+# Generally, we create some image with or without existing preallocation and
+# then resize it. Then we write some data into the image and verify that its
+# size does not change if we have used preallocation.
+
+# With a cluster size of 512 B, one L2 table covers 64 * 512 B = 32 kB.
+# One cluster of the L1 table covers 64 * 32 kB = 2 MB.
+# There are multiple cases we want to test:
+# (1) Grow an image without having to allocate a new L2 table.
+# (2) Grow an image, having to allocate a new L2 table.
+# (3) Grow an image, having to grow the L1 table.
+# Therefore, we create an image that is 48 kB below 2 MB. Then:
+# (1) We resize it to 2 MB - 32 kB. (+ 16 kB)
+# (2) We resize it to 2 MB. (+ 48 kB)
+# (3) We resize it to 2 MB + 32 kB. (+ 80 kB)
+
+# in B
+CREATION_SIZE=$((2 * 1024 * 1024 - 48 * 1024))
+
+# in kB
+for GROWTH_SIZE in 16 48 80; do
+ for create_mode in off metadata falloc full; do
+ for growth_mode in off metadata falloc full; do
+ echo "--- growth_size=$GROWTH_SIZE create_mode=$create_mode growth_mode=$growth_mode ---"
+
+ IMGOPTS="preallocation=$create_mode,cluster_size=512" _make_test_img ${CREATION_SIZE}
+ $QEMU_IMG resize -f "$IMGFMT" --preallocation=$growth_mode "$TEST_IMG" +${GROWTH_SIZE}K
+
+ host_size_0=$(get_image_size_on_host)
+ file_length_0=$(stat -c '%s' "$TEST_IMG_FILE")
+
+ $QEMU_IO -c "write 0 $CREATION_SIZE" "$TEST_IMG" | _filter_qemu_io
+
+ host_size_1=$(get_image_size_on_host)
+ file_length_1=$(stat -c '%s' "$TEST_IMG_FILE")
+
+ $QEMU_IO -c "write $CREATION_SIZE ${GROWTH_SIZE}K" "$TEST_IMG" | _filter_qemu_io
+
+ host_size_2=$(get_image_size_on_host)
+ file_length_2=$(stat -c '%s' "$TEST_IMG_FILE")
+
+ # Test creation preallocation: Compare #0 against #1
+ if [ $create_mode != off ]; then
+ # The image length should not have grown
+ if [ $file_length_1 -gt $file_length_0 ]; then
+ echo "ERROR (create): Image length has grown from $file_length_0 to $file_length_1"
+ fi
+ if [ $create_mode != metadata ]; then
+ # The host size should not have grown either
+ if [ $host_size_1 -gt $host_size_0 ]; then
+ echo "ERROR (create): Host size has grown from $host_size_0 to $host_size_1"
+ fi
+ fi
+ fi
+
+ # Test resize preallocation: Compare #2 against #1
+ if [ $growth_mode != off ]; then
+ # The image length should not have grown
+ if [ $file_length_2 -gt $file_length_1 ]; then
+ echo "ERROR (grow): Image length has grown from $file_length_1 to $file_length_2"
+ fi
+ if [ $create_mode != metadata ]; then
+ # The host size should not have grown either
+ if [ $host_size_2 -gt $host_size_1 ]; then
+ echo "ERROR (grow): Host size has grown from $host_size_1 to $host_size_2"
+ fi
+ fi
+ fi
+
+ echo
+ done
+ done
+done
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/125.out b/tests/qemu-iotests/125.out
new file mode 100644
index 0000000000..3f4d6e31a6
--- /dev/null
+++ b/tests/qemu-iotests/125.out
@@ -0,0 +1,386 @@
+QA output created by 125
+--- growth_size=16 create_mode=off growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=off growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=off growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=off growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=metadata growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=metadata growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=metadata growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=metadata growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=falloc growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=falloc growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=falloc growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=falloc growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=full growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=full growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=full growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=16 create_mode=full growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 16384/16384 bytes at offset 2048000
+16 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=off growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=off growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=off growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=off growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=metadata growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=metadata growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=metadata growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=metadata growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=falloc growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=falloc growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=falloc growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=falloc growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=full growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=full growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=full growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=48 create_mode=full growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 49152/49152 bytes at offset 2048000
+48 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=off growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=off growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=off growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=off growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=off
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=metadata growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=metadata growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=metadata growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=metadata growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=metadata
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=falloc growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=falloc growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=falloc growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=falloc growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=falloc
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=full growth_mode=off ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=full growth_mode=metadata ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=full growth_mode=falloc ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+--- growth_size=80 create_mode=full growth_mode=full ---
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=2048000 preallocation=full
+Image resized.
+wrote 2048000/2048000 bytes at offset 0
+1.953 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 81920/81920 bytes at offset 2048000
+80 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1a89bebe14..a9a3b29db9 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -131,6 +131,7 @@
122 rw auto
123 rw auto quick
124 rw auto backing
+125 rw auto
128 rw auto quick
129 rw auto quick
130 rw auto quick
--
2.12.0
^ permalink raw reply related [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 12/16] block/qcow2: Extract qcow2_calc_size_usage()
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 12/16] block/qcow2: Extract qcow2_calc_size_usage() Max Reitz
@ 2017-03-20 9:43 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 9:43 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 528 bytes --]
On Mon, Mar 13, 2017 at 10:41:13PM +0100, Max Reitz wrote:
> We will need very similar functionality for full/falloc preallocation in
> qcow2_truncate(). Although we will not be able to reuse much of the
> actual code, it still makes sense to keep all of this in one place.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2.c | 126 +++++++++++++++++++++++++++++++---------------------------
> 1 file changed, 68 insertions(+), 58 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-13 21:39 ` [Qemu-devel] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate() Max Reitz
@ 2017-03-20 10:10 ` Stefan Hajnoczi
2017-03-20 10:18 ` Stefan Hajnoczi
1 sibling, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 10:10 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]
On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> Add a PreallocMode parameter to the bdrv_truncate() function implemented
> by each block driver. Currently, we always pass PREALLOC_MODE_OFF and no
> driver accepts anything else.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> include/block/block_int.h | 3 ++-
> block.c | 2 +-
> block/blkdebug.c | 9 ++++++++-
> block/crypto.c | 8 +++++++-
> block/file-posix.c | 9 ++++++++-
> block/file-win32.c | 9 ++++++++-
> block/gluster.c | 6 +++++-
> block/iscsi.c | 7 ++++++-
> block/nfs.c | 8 +++++++-
> block/qcow2.c | 9 ++++++++-
> block/qed.c | 9 ++++++++-
> block/raw-format.c | 9 ++++++++-
> block/rbd.c | 7 ++++++-
> block/sheepdog.c | 11 +++++++++--
> 14 files changed, 91 insertions(+), 15 deletions(-)
This patch reminds me that some block drivers aren't using Error **errp
yet. I have added a BiteSizedTasks entry so that this can be fixed
someday.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-13 21:39 ` [Qemu-devel] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate() Max Reitz
2017-03-20 10:10 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2017-03-20 10:18 ` Stefan Hajnoczi
2017-03-20 15:07 ` Max Reitz
1 sibling, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 10:18 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1031 bytes --]
On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> diff --git a/block/iscsi.c b/block/iscsi.c
> index ab559a6f71..5d6265c4a6 100644
> --- a/block/iscsi.c
> +++ b/block/iscsi.c
> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
> }
> }
>
> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
> + PreallocMode prealloc, Error **errp)
> {
> IscsiLun *iscsilun = bs->opaque;
> Error *local_err = NULL;
>
> + if (prealloc != PREALLOC_MODE_OFF) {
> + return -ENOTSUP;
> + }
> +
> if (iscsilun->type != TYPE_DISK) {
> return -ENOTSUP;
> }
Nevermind what I said about adding a BiteSizedTasks entry:
The missing errp usage is not in qemu.git/master yet. Please fix up
your bdrv_truncate() errp patch to use errp in all cases, e.g.
error_setg("Unable to truncate non-disk LUN").
stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 03/16] block: Add PreallocMode to blk_truncate()
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 03/16] block: Add PreallocMode to blk_truncate() Max Reitz
@ 2017-03-20 10:33 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 10:33 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1036 bytes --]
On Mon, Mar 13, 2017 at 10:40:32PM +0100, Max Reitz wrote:
> blk_truncate() itself will pass that value to bdrv_truncate(), and all
> callers of blk_truncate() just set the parameter to PREALLOC_MODE_OFF
> for now.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> include/sysemu/block-backend.h | 3 ++-
> block/block-backend.c | 5 +++--
> block/commit.c | 4 ++--
> block/mirror.c | 3 ++-
> block/parallels.c | 2 +-
> block/qcow.c | 2 +-
> block/qcow2.c | 4 ++--
> block/qed.c | 2 +-
> block/vdi.c | 3 ++-
> block/vhdx.c | 5 +++--
> block/vmdk.c | 7 ++++---
> block/vpc.c | 2 +-
> blockdev.c | 2 +-
> qemu-img.c | 2 +-
> qemu-io-cmds.c | 2 +-
> 15 files changed, 27 insertions(+), 21 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 04/16] qemu-img: Expose PreallocMode for resizing
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 04/16] qemu-img: Expose PreallocMode for resizing Max Reitz
@ 2017-03-20 10:47 ` Stefan Hajnoczi
2017-03-20 15:07 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 10:47 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 369 bytes --]
On Mon, Mar 13, 2017 at 10:40:33PM +0100, Max Reitz wrote:
> @@ -3429,8 +3442,16 @@ static int img_resize(int argc, char **argv)
> goto out;
> }
>
> + current_size = blk_getlength(blk);
> + if (current_size < 0) {
> + error_report("Failed to inquire current image length: %s\n",
Spurious newline, error_report() doesn't need it.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 05/16] block/file-posix: Small fixes in raw_create()
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 05/16] block/file-posix: Small fixes in raw_create() Max Reitz
@ 2017-03-20 10:48 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 10:48 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 427 bytes --]
On Mon, Mar 13, 2017 at 10:40:34PM +0100, Max Reitz wrote:
> Variables should be declared at the start of a block, and if a certain
> parameter value is not supported it may be better to return -ENOTSUP
> instead of -EINVAL.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/file-posix.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 06/16] block/file-posix: Extract raw_regular_truncate()
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 06/16] block/file-posix: Extract raw_regular_truncate() Max Reitz
@ 2017-03-20 10:49 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 10:49 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 416 bytes --]
On Mon, Mar 13, 2017 at 10:40:35PM +0100, Max Reitz wrote:
> This functionality is part of raw_create() which we will be able to
> reuse nicely in raw_truncate().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/file-posix.c | 144 +++++++++++++++++++++++++++++------------------------
> 1 file changed, 78 insertions(+), 66 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate Max Reitz
@ 2017-03-20 11:00 ` Stefan Hajnoczi
2017-03-20 15:11 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:00 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2484 bytes --]
On Mon, Mar 13, 2017 at 10:40:36PM +0100, Max Reitz wrote:
> Currently, raw_regular_truncate() is intended for setting the size of a
> newly created file. However, we also want to use it for truncating an
> existing file in which case only the newly added space (when growing)
> should be preallocated.
>
> This also means that if resizing failed, we should try to restore the
> original file size. This is important when using preallocation.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/file-posix.c | 73 ++++++++++++++++++++++++++++++++++++++++++++----------
> 1 file changed, 60 insertions(+), 13 deletions(-)
>
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 35a9e43f3e..cd229324ba 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1384,11 +1384,39 @@ static void raw_close(BlockDriverState *bs)
> }
> }
>
> -static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
> +/**
> + * Truncates the given regular file @fd to @offset and, when growing, fills the
> + * new space according to @prealloc.
> + *
> + * @create must be true iff the file is new. In that case, @bs is ignored. If
> + * @create is false, @bs must be valid and correspond to the same file as @fd.
Returns: 0 on success, -errno on failure.
> + */
> +static int raw_regular_truncate(int fd, BlockDriverState *bs, int64_t offset,
The presence of both a file descriptor and a BlockDriverState (actually
it must be a BDRVRawState) arguments is unusual. It seems bs is needed
for bdrv_getlength().
How about using fstat(fd, &st) and then dropping bs and create?
> + PreallocMode prealloc, bool create,
> Error **errp)
> {
> int result = 0;
> - char *buf;
> + int64_t current_length = 0;
> + char *buf = NULL;
> +
> + assert(create || bs);
> + if (!create) {
> + BDRVRawState *s = bs->opaque;
> + assert(s->fd == fd);
> + }
> +
> + if (!create) {
> + current_length = bdrv_getlength(bs);
> + if (current_length < 0) {
> + error_setg_errno(errp, -current_length,
> + "Could not inquire current length");
> + return -current_length;
> + }
> +
> + if (current_length > offset && prealloc != PREALLOC_MODE_OFF) {
> + return -ENOTSUP;
> + }
Missing error_setg().
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 08/16] block/file-posix: Preallocation for truncate
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 08/16] block/file-posix: Preallocation for truncate Max Reitz
@ 2017-03-20 11:01 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:01 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 376 bytes --]
On Mon, Mar 13, 2017 at 10:40:37PM +0100, Max Reitz wrote:
> By using raw_regular_truncate() in raw_truncate(), we can now easily
> support preallocation.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/file-posix.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 09/16] block/qcow2: Generalize preallocate()
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 09/16] block/qcow2: Generalize preallocate() Max Reitz
@ 2017-03-20 11:04 ` Stefan Hajnoczi
2017-03-20 15:13 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:04 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 816 bytes --]
On Mon, Mar 13, 2017 at 10:40:38PM +0100, Max Reitz wrote:
> This patch adds two new parameters to the preallocate() function so we
> will be able to use it not just for preallocating a new image but also
> for preallocated image growth.
>
> The offset parameter allows the caller to specify a virtual offset from
> which to start preallocating. For newly created images this is always 0,
> but for preallocating growth this will be the old image length.
>
> The new_length parameter specifies the supposed new length of the image
> (basically the "end offset" for preallocation). During image truncation,
> bdrv_getlength() will return the old image length so we cannot rely on
> its return value then.
You documented the arguments in the commit description. Please move
them into doc comments.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 10/16] block/qcow2: Lock s->lock in preallocate()
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 10/16] block/qcow2: Lock s->lock in preallocate() Max Reitz
@ 2017-03-20 11:05 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:05 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 547 bytes --]
On Mon, Mar 13, 2017 at 10:41:11PM +0100, Max Reitz wrote:
> preallocate() is and will be called only from places that do not lock
> s->lock: Currently that is qcow2_create2(), as of a future patch it will
> be called from qcow2_truncate(), too.
>
> It therefore makes sense to move locking that mutex into preallocate()
> itself.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 11/16] block/qcow2: Metadata preallocation for truncate
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 11/16] block/qcow2: Metadata preallocation for truncate Max Reitz
@ 2017-03-20 11:23 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:23 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 365 bytes --]
On Mon, Mar 13, 2017 at 10:41:12PM +0100, Max Reitz wrote:
> We can support PREALLOC_MODE_METADATA by invoking preallocate() in
> qcow2_truncate().
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2.c | 24 ++++++++++++++++++++++--
> 1 file changed, 22 insertions(+), 2 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate Max Reitz
@ 2017-03-20 11:26 ` Stefan Hajnoczi
2017-03-20 15:14 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:26 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1915 bytes --]
On Mon, Mar 13, 2017 at 10:41:14PM +0100, Max Reitz wrote:
> This patch extends qcow2_calc_size_usage() so it can calculate the
> additional space needed for preallocating image growth.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2.c | 137 +++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 98 insertions(+), 39 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 21b2b3cd53..80fb815b15 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2101,7 +2101,15 @@ done:
> return ret;
> }
>
> -static uint64_t qcow2_calc_size_usage(uint64_t new_size,
> +/**
> + * Returns the number of bytes that must be allocated in the underlying file
> + * to accomodate an image growth from @current_size to @new_size.
> + *
> + * @current_size must be 0 when creating a new image. In that case, @bs is
> + * ignored; otherwise it must be valid.
> + */
> +static uint64_t qcow2_calc_size_usage(BlockDriverState *bs,
> + uint64_t current_size, uint64_t new_size,
> int cluster_bits, int refcount_order)
> {
> size_t cluster_size = 1u << cluster_bits;
> @@ -2122,47 +2130,97 @@ static uint64_t qcow2_calc_size_usage(uint64_t new_size,
> refblock_bits = cluster_bits - (refcount_order - 3);
> refblock_size = 1 << refblock_bits;
>
> - /* header: 1 cluster */
> - meta_size += cluster_size;
> -
> - /* total size of L2 tables */
> - nl2e = aligned_total_size / cluster_size;
> - nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
> - meta_size += nl2e * sizeof(uint64_t);
> + if (!current_size) {
This massive if statement effectively makes two functions: the old
qcow2_calc_size_usage() and the new qcow2_calc_size_change(bs) function.
It might be nicer to split the two functions.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth Max Reitz
@ 2017-03-20 11:29 ` Stefan Hajnoczi
2017-03-20 15:15 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:29 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1055 bytes --]
On Mon, Mar 13, 2017 at 10:41:15PM +0100, Max Reitz wrote:
> Implement the preallocation modes falloc and full for growing qcow2
> images.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> block/qcow2.c | 36 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 80fb815b15..b6b08d70da 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -2604,7 +2604,9 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
> int64_t new_l1_size;
> int ret;
>
> - if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA) {
> + if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
> + prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
Now all cases are covered so this if statement can be dropped. If you
are worried about new preallocation modes being added in the future then
the error_setg() can be moved into the switch statement's default case.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 15/16] iotests: Add preallocated resize test for raw
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 15/16] iotests: Add preallocated resize test for raw Max Reitz
@ 2017-03-20 11:30 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:30 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 476 bytes --]
On Mon, Mar 13, 2017 at 10:41:16PM +0100, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/106 | 92 ++++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/106.out | 50 +++++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 143 insertions(+)
> create mode 100755 tests/qemu-iotests/106
> create mode 100644 tests/qemu-iotests/106.out
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 16/16] iotests: Add preallocated growth test for qcow2
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 16/16] iotests: Add preallocated growth test for qcow2 Max Reitz
@ 2017-03-20 11:31 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:31 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 468 bytes --]
On Mon, Mar 13, 2017 at 10:41:17PM +0100, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> tests/qemu-iotests/125 | 130 +++++++++++++++
> tests/qemu-iotests/125.out | 386 +++++++++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/group | 1 +
> 3 files changed, 517 insertions(+)
> create mode 100755 tests/qemu-iotests/125
> create mode 100644 tests/qemu-iotests/125.out
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 02/16] block: Add PreallocMode to bdrv_truncate()
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 02/16] block: Add PreallocMode to bdrv_truncate() Max Reitz
@ 2017-03-20 11:33 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-20 11:33 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 874 bytes --]
On Mon, Mar 13, 2017 at 10:40:18PM +0100, Max Reitz wrote:
> For block drivers that just pass a truncate request to the underlying
> protocol, we can now pass the preallocation mode instead of aborting if
> it is not PREALLOC_MODE_OFF.
>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> include/block/block.h | 3 ++-
> block.c | 12 +++++++++---
> block/blkdebug.c | 8 +-------
> block/block-backend.c | 2 +-
> block/crypto.c | 8 +-------
> block/parallels.c | 11 +++++++----
> block/qcow.c | 6 ++++--
> block/qcow2-refcount.c | 2 +-
> block/qcow2.c | 4 ++--
> block/raw-format.c | 8 +-------
> block/vhdx-log.c | 2 +-
> block/vhdx.c | 3 ++-
> 12 files changed, 32 insertions(+), 37 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-20 10:18 ` Stefan Hajnoczi
@ 2017-03-20 15:07 ` Max Reitz
2017-03-22 16:28 ` Stefan Hajnoczi
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-20 15:07 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1552 bytes --]
On 20.03.2017 11:18, Stefan Hajnoczi wrote:
> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index ab559a6f71..5d6265c4a6 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
>> }
>> }
>>
>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
>> + PreallocMode prealloc, Error **errp)
>> {
>> IscsiLun *iscsilun = bs->opaque;
>> Error *local_err = NULL;
>>
>> + if (prealloc != PREALLOC_MODE_OFF) {
>> + return -ENOTSUP;
>> + }
>> +
>> if (iscsilun->type != TYPE_DISK) {
>> return -ENOTSUP;
>> }
>
> Nevermind what I said about adding a BiteSizedTasks entry:
>
> The missing errp usage is not in qemu.git/master yet. Please fix up
> your bdrv_truncate() errp patch to use errp in all cases, e.g.
> error_setg("Unable to truncate non-disk LUN").
The thing is that I wasn't comfortable doing that for all block drivers.
I mean, I can take another look but I'd rather have vague error messages
("truncation failed: #{strerror}") than outright wrong ones because I
didn't know what error message to use.
Of course you could argue that this may probably come out during review
but that implies that every submaintainer for every block driver would
actually come out for review...
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 04/16] qemu-img: Expose PreallocMode for resizing
2017-03-20 10:47 ` Stefan Hajnoczi
@ 2017-03-20 15:07 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2017-03-20 15:07 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 455 bytes --]
On 20.03.2017 11:47, Stefan Hajnoczi wrote:
> On Mon, Mar 13, 2017 at 10:40:33PM +0100, Max Reitz wrote:
>> @@ -3429,8 +3442,16 @@ static int img_resize(int argc, char **argv)
>> goto out;
>> }
>>
>> + current_size = blk_getlength(blk);
>> + if (current_size < 0) {
>> + error_report("Failed to inquire current image length: %s\n",
>
> Spurious newline, error_report() doesn't need it.
Oops, will fix.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate
2017-03-20 11:00 ` Stefan Hajnoczi
@ 2017-03-20 15:11 ` Max Reitz
2017-03-22 16:44 ` Stefan Hajnoczi
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-20 15:11 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]
On 20.03.2017 12:00, Stefan Hajnoczi wrote:
> On Mon, Mar 13, 2017 at 10:40:36PM +0100, Max Reitz wrote:
>> Currently, raw_regular_truncate() is intended for setting the size of a
>> newly created file. However, we also want to use it for truncating an
>> existing file in which case only the newly added space (when growing)
>> should be preallocated.
>>
>> This also means that if resizing failed, we should try to restore the
>> original file size. This is important when using preallocation.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block/file-posix.c | 73 ++++++++++++++++++++++++++++++++++++++++++++----------
>> 1 file changed, 60 insertions(+), 13 deletions(-)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index 35a9e43f3e..cd229324ba 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -1384,11 +1384,39 @@ static void raw_close(BlockDriverState *bs)
>> }
>> }
>>
>> -static int raw_regular_truncate(int fd, int64_t offset, PreallocMode prealloc,
>> +/**
>> + * Truncates the given regular file @fd to @offset and, when growing, fills the
>> + * new space according to @prealloc.
>> + *
>> + * @create must be true iff the file is new. In that case, @bs is ignored. If
>> + * @create is false, @bs must be valid and correspond to the same file as @fd.
>
> Returns: 0 on success, -errno on failure.
Yep, will add.
>> + */
>> +static int raw_regular_truncate(int fd, BlockDriverState *bs, int64_t offset,
>
> The presence of both a file descriptor and a BlockDriverState (actually
> it must be a BDRVRawState) arguments is unusual. It seems bs is needed
> for bdrv_getlength().
>
> How about using fstat(fd, &st) and then dropping bs and create?
Well, I could do that, but bdrv_getlength() is a bit simpler and I don't
see much of an issue with specifying both an fd and a bs.
>> + PreallocMode prealloc, bool create,
>> Error **errp)
>> {
>> int result = 0;
>> - char *buf;
>> + int64_t current_length = 0;
>> + char *buf = NULL;
>> +
>> + assert(create || bs);
>> + if (!create) {
>> + BDRVRawState *s = bs->opaque;
>> + assert(s->fd == fd);
>> + }
>> +
>> + if (!create) {
>> + current_length = bdrv_getlength(bs);
>> + if (current_length < 0) {
>> + error_setg_errno(errp, -current_length,
>> + "Could not inquire current length");
>> + return -current_length;
>> + }
>> +
>> + if (current_length > offset && prealloc != PREALLOC_MODE_OFF) {
>> + return -ENOTSUP;
>> + }
>
> Missing error_setg().
Indeed! Will fix.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 09/16] block/qcow2: Generalize preallocate()
2017-03-20 11:04 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2017-03-20 15:13 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2017-03-20 15:13 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 982 bytes --]
On 20.03.2017 12:04, Stefan Hajnoczi wrote:
> On Mon, Mar 13, 2017 at 10:40:38PM +0100, Max Reitz wrote:
>> This patch adds two new parameters to the preallocate() function so we
>> will be able to use it not just for preallocating a new image but also
>> for preallocated image growth.
>>
>> The offset parameter allows the caller to specify a virtual offset from
>> which to start preallocating. For newly created images this is always 0,
>> but for preallocating growth this will be the old image length.
>>
>> The new_length parameter specifies the supposed new length of the image
>> (basically the "end offset" for preallocation). During image truncation,
>> bdrv_getlength() will return the old image length so we cannot rely on
>> its return value then.
>
> You documented the arguments in the commit description. Please move
> them into doc comments.
Yeah, right, new_length is not really a super self-explaining name...
I'll add a comment.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate
2017-03-20 11:26 ` Stefan Hajnoczi
@ 2017-03-20 15:14 ` Max Reitz
2017-03-29 22:12 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-20 15:14 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2228 bytes --]
On 20.03.2017 12:26, Stefan Hajnoczi wrote:
> On Mon, Mar 13, 2017 at 10:41:14PM +0100, Max Reitz wrote:
>> This patch extends qcow2_calc_size_usage() so it can calculate the
>> additional space needed for preallocating image growth.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block/qcow2.c | 137 +++++++++++++++++++++++++++++++++++++++++-----------------
>> 1 file changed, 98 insertions(+), 39 deletions(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 21b2b3cd53..80fb815b15 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -2101,7 +2101,15 @@ done:
>> return ret;
>> }
>>
>> -static uint64_t qcow2_calc_size_usage(uint64_t new_size,
>> +/**
>> + * Returns the number of bytes that must be allocated in the underlying file
>> + * to accomodate an image growth from @current_size to @new_size.
>> + *
>> + * @current_size must be 0 when creating a new image. In that case, @bs is
>> + * ignored; otherwise it must be valid.
>> + */
>> +static uint64_t qcow2_calc_size_usage(BlockDriverState *bs,
>> + uint64_t current_size, uint64_t new_size,
>> int cluster_bits, int refcount_order)
>> {
>> size_t cluster_size = 1u << cluster_bits;
>> @@ -2122,47 +2130,97 @@ static uint64_t qcow2_calc_size_usage(uint64_t new_size,
>> refblock_bits = cluster_bits - (refcount_order - 3);
>> refblock_size = 1 << refblock_bits;
>>
>> - /* header: 1 cluster */
>> - meta_size += cluster_size;
>> -
>> - /* total size of L2 tables */
>> - nl2e = aligned_total_size / cluster_size;
>> - nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
>> - meta_size += nl2e * sizeof(uint64_t);
>> + if (!current_size) {
>
> This massive if statement effectively makes two functions: the old
> qcow2_calc_size_usage() and the new qcow2_calc_size_change(bs) function.
>
> It might be nicer to split the two functions.
Hm, yes, but at that point I might as well just write two completely
separate functions. I'll see what I can do, maybe I'll just put the new
logic that's needed into a completely new function after all.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth
2017-03-20 11:29 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
@ 2017-03-20 15:15 ` Max Reitz
2017-03-22 17:02 ` Stefan Hajnoczi
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-20 15:15 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1237 bytes --]
On 20.03.2017 12:29, Stefan Hajnoczi wrote:
> On Mon, Mar 13, 2017 at 10:41:15PM +0100, Max Reitz wrote:
>> Implement the preallocation modes falloc and full for growing qcow2
>> images.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> block/qcow2.c | 36 +++++++++++++++++++++++++++++++++++-
>> 1 file changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 80fb815b15..b6b08d70da 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -2604,7 +2604,9 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
>> int64_t new_l1_size;
>> int ret;
>>
>> - if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA) {
>> + if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
>> + prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
>
> Now all cases are covered so this if statement can be dropped. If you
> are worried about new preallocation modes being added in the future then
> the error_setg() can be moved into the switch statement's default case.
No, because the switch comes after we have already grown the L1 table.
That wouldn't be so nice.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-20 15:07 ` Max Reitz
@ 2017-03-22 16:28 ` Stefan Hajnoczi
2017-03-22 16:50 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-22 16:28 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2141 bytes --]
On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
> > On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> >> diff --git a/block/iscsi.c b/block/iscsi.c
> >> index ab559a6f71..5d6265c4a6 100644
> >> --- a/block/iscsi.c
> >> +++ b/block/iscsi.c
> >> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
> >> }
> >> }
> >>
> >> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
> >> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
> >> + PreallocMode prealloc, Error **errp)
> >> {
> >> IscsiLun *iscsilun = bs->opaque;
> >> Error *local_err = NULL;
> >>
> >> + if (prealloc != PREALLOC_MODE_OFF) {
> >> + return -ENOTSUP;
> >> + }
> >> +
> >> if (iscsilun->type != TYPE_DISK) {
> >> return -ENOTSUP;
> >> }
> >
> > Nevermind what I said about adding a BiteSizedTasks entry:
> >
> > The missing errp usage is not in qemu.git/master yet. Please fix up
> > your bdrv_truncate() errp patch to use errp in all cases, e.g.
> > error_setg("Unable to truncate non-disk LUN").
>
> The thing is that I wasn't comfortable doing that for all block drivers.
> I mean, I can take another look but I'd rather have vague error messages
> ("truncation failed: #{strerror}") than outright wrong ones because I
> didn't know what error message to use.
>
> Of course you could argue that this may probably come out during review
> but that implies that every submaintainer for every block driver would
> actually come out for review...
I'm worried about errp being set in only a subset of error cases.
This is likely to cause bugs if callers use if (local_err). Grepping
through the codebase I can see instances of:
ret = foo(..., &local_err);
if (local_err) { /* no ret check! */
...
}
The code would work fine with qcow2 but not iscsi, for example.
IMO we should always set errp, even if the error message is vague
("truncation failed: #{strerror}").
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate
2017-03-20 15:11 ` Max Reitz
@ 2017-03-22 16:44 ` Stefan Hajnoczi
2017-03-22 16:53 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-22 16:44 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 945 bytes --]
On Mon, Mar 20, 2017 at 04:11:24PM +0100, Max Reitz wrote:
> On 20.03.2017 12:00, Stefan Hajnoczi wrote:
> > On Mon, Mar 13, 2017 at 10:40:36PM +0100, Max Reitz wrote:
> >> +static int raw_regular_truncate(int fd, BlockDriverState *bs, int64_t offset,
> >
> > The presence of both a file descriptor and a BlockDriverState (actually
> > it must be a BDRVRawState) arguments is unusual. It seems bs is needed
> > for bdrv_getlength().
> >
> > How about using fstat(fd, &st) and then dropping bs and create?
>
> Well, I could do that, but bdrv_getlength() is a bit simpler and I don't
> see much of an issue with specifying both an fd and a bs.
Arguments that provide overlapping information make the function harder
to understand and use correctly (there are combinations of these
arguments that are invalid or don't make sense). Saving a few lines of
code in the function implementation is a poor trade-off IMO.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-22 16:28 ` Stefan Hajnoczi
@ 2017-03-22 16:50 ` Max Reitz
2017-03-23 15:32 ` Stefan Hajnoczi
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-22 16:50 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]
On 22.03.2017 17:28, Stefan Hajnoczi wrote:
> On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
>> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
>>> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>> index ab559a6f71..5d6265c4a6 100644
>>>> --- a/block/iscsi.c
>>>> +++ b/block/iscsi.c
>>>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
>>>> }
>>>> }
>>>>
>>>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
>>>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
>>>> + PreallocMode prealloc, Error **errp)
>>>> {
>>>> IscsiLun *iscsilun = bs->opaque;
>>>> Error *local_err = NULL;
>>>>
>>>> + if (prealloc != PREALLOC_MODE_OFF) {
>>>> + return -ENOTSUP;
>>>> + }
>>>> +
>>>> if (iscsilun->type != TYPE_DISK) {
>>>> return -ENOTSUP;
>>>> }
>>>
>>> Nevermind what I said about adding a BiteSizedTasks entry:
>>>
>>> The missing errp usage is not in qemu.git/master yet. Please fix up
>>> your bdrv_truncate() errp patch to use errp in all cases, e.g.
>>> error_setg("Unable to truncate non-disk LUN").
>>
>> The thing is that I wasn't comfortable doing that for all block drivers.
>> I mean, I can take another look but I'd rather have vague error messages
>> ("truncation failed: #{strerror}") than outright wrong ones because I
>> didn't know what error message to use.
>>
>> Of course you could argue that this may probably come out during review
>> but that implies that every submaintainer for every block driver would
>> actually come out for review...
>
> I'm worried about errp being set in only a subset of error cases.
>
> This is likely to cause bugs if callers use if (local_err). Grepping
> through the codebase I can see instances of:
Yes, but the generic bdrv_truncate() will always set errp if the driver
hasn't done so.
Max
> ret = foo(..., &local_err);
> if (local_err) { /* no ret check! */
> ...
> }
>
> The code would work fine with qcow2 but not iscsi, for example.
>
> IMO we should always set errp, even if the error message is vague
> ("truncation failed: #{strerror}").
>
> Stefan
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate
2017-03-22 16:44 ` Stefan Hajnoczi
@ 2017-03-22 16:53 ` Max Reitz
2017-03-23 16:13 ` Stefan Hajnoczi
0 siblings, 1 reply; 49+ messages in thread
From: Max Reitz @ 2017-03-22 16:53 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]
On 22.03.2017 17:44, Stefan Hajnoczi wrote:
> On Mon, Mar 20, 2017 at 04:11:24PM +0100, Max Reitz wrote:
>> On 20.03.2017 12:00, Stefan Hajnoczi wrote:
>>> On Mon, Mar 13, 2017 at 10:40:36PM +0100, Max Reitz wrote:
>>>> +static int raw_regular_truncate(int fd, BlockDriverState *bs, int64_t offset,
>>>
>>> The presence of both a file descriptor and a BlockDriverState (actually
>>> it must be a BDRVRawState) arguments is unusual. It seems bs is needed
>>> for bdrv_getlength().
>>>
>>> How about using fstat(fd, &st) and then dropping bs and create?
>>
>> Well, I could do that, but bdrv_getlength() is a bit simpler and I don't
>> see much of an issue with specifying both an fd and a bs.
>
> Arguments that provide overlapping information make the function harder
> to understand and use correctly (there are combinations of these
> arguments that are invalid or don't make sense). Saving a few lines of
> code in the function implementation is a poor trade-off IMO.
My brain likes to agree but for some reason my heart really prefers
block layer functions (where I know what can go wrong) over POSIX
functions (where I always have the feeling of maybe not having though of
everything).
I guess I'll have to silence my heart for a bit, then.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth
2017-03-20 15:15 ` Max Reitz
@ 2017-03-22 17:02 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-22 17:02 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1365 bytes --]
On Mon, Mar 20, 2017 at 04:15:59PM +0100, Max Reitz wrote:
> On 20.03.2017 12:29, Stefan Hajnoczi wrote:
> > On Mon, Mar 13, 2017 at 10:41:15PM +0100, Max Reitz wrote:
> >> Implement the preallocation modes falloc and full for growing qcow2
> >> images.
> >>
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> ---
> >> block/qcow2.c | 36 +++++++++++++++++++++++++++++++++++-
> >> 1 file changed, 35 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/block/qcow2.c b/block/qcow2.c
> >> index 80fb815b15..b6b08d70da 100644
> >> --- a/block/qcow2.c
> >> +++ b/block/qcow2.c
> >> @@ -2604,7 +2604,9 @@ static int qcow2_truncate(BlockDriverState *bs, int64_t offset,
> >> int64_t new_l1_size;
> >> int ret;
> >>
> >> - if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA) {
> >> + if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
> >> + prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
> >
> > Now all cases are covered so this if statement can be dropped. If you
> > are worried about new preallocation modes being added in the future then
> > the error_setg() can be moved into the switch statement's default case.
>
> No, because the switch comes after we have already grown the L1 table.
> That wouldn't be so nice.
Oops :)
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-22 16:50 ` Max Reitz
@ 2017-03-23 15:32 ` Stefan Hajnoczi
2017-03-27 14:19 ` Max Reitz
0 siblings, 1 reply; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-23 15:32 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 2580 bytes --]
On Wed, Mar 22, 2017 at 05:50:59PM +0100, Max Reitz wrote:
> On 22.03.2017 17:28, Stefan Hajnoczi wrote:
> > On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
> >> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
> >>> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
> >>>> diff --git a/block/iscsi.c b/block/iscsi.c
> >>>> index ab559a6f71..5d6265c4a6 100644
> >>>> --- a/block/iscsi.c
> >>>> +++ b/block/iscsi.c
> >>>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
> >>>> }
> >>>> }
> >>>>
> >>>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
> >>>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
> >>>> + PreallocMode prealloc, Error **errp)
> >>>> {
> >>>> IscsiLun *iscsilun = bs->opaque;
> >>>> Error *local_err = NULL;
> >>>>
> >>>> + if (prealloc != PREALLOC_MODE_OFF) {
> >>>> + return -ENOTSUP;
> >>>> + }
> >>>> +
> >>>> if (iscsilun->type != TYPE_DISK) {
> >>>> return -ENOTSUP;
> >>>> }
> >>>
> >>> Nevermind what I said about adding a BiteSizedTasks entry:
> >>>
> >>> The missing errp usage is not in qemu.git/master yet. Please fix up
> >>> your bdrv_truncate() errp patch to use errp in all cases, e.g.
> >>> error_setg("Unable to truncate non-disk LUN").
> >>
> >> The thing is that I wasn't comfortable doing that for all block drivers.
> >> I mean, I can take another look but I'd rather have vague error messages
> >> ("truncation failed: #{strerror}") than outright wrong ones because I
> >> didn't know what error message to use.
> >>
> >> Of course you could argue that this may probably come out during review
> >> but that implies that every submaintainer for every block driver would
> >> actually come out for review...
> >
> > I'm worried about errp being set in only a subset of error cases.
> >
> > This is likely to cause bugs if callers use if (local_err). Grepping
> > through the codebase I can see instances of:
>
> Yes, but the generic bdrv_truncate() will always set errp if the driver
> hasn't done so.
I prefer consistently setting errp to make patch review easy (no
checking all callers to see how they behave), making it safe to
copy-paste the code elsewhere, etc.
It's safer to be consistent, can produce more detailed error messages,
and the cost of writing error_setg() calls is low. But it's a matter of
style, you've pointed out the code is technically correct right now.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate
2017-03-22 16:53 ` Max Reitz
@ 2017-03-23 16:13 ` Stefan Hajnoczi
0 siblings, 0 replies; 49+ messages in thread
From: Stefan Hajnoczi @ 2017-03-23 16:13 UTC (permalink / raw)
To: Max Reitz; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]
On Wed, Mar 22, 2017 at 05:53:00PM +0100, Max Reitz wrote:
> On 22.03.2017 17:44, Stefan Hajnoczi wrote:
> > On Mon, Mar 20, 2017 at 04:11:24PM +0100, Max Reitz wrote:
> >> On 20.03.2017 12:00, Stefan Hajnoczi wrote:
> >>> On Mon, Mar 13, 2017 at 10:40:36PM +0100, Max Reitz wrote:
> >>>> +static int raw_regular_truncate(int fd, BlockDriverState *bs, int64_t offset,
> >>>
> >>> The presence of both a file descriptor and a BlockDriverState (actually
> >>> it must be a BDRVRawState) arguments is unusual. It seems bs is needed
> >>> for bdrv_getlength().
> >>>
> >>> How about using fstat(fd, &st) and then dropping bs and create?
> >>
> >> Well, I could do that, but bdrv_getlength() is a bit simpler and I don't
> >> see much of an issue with specifying both an fd and a bs.
> >
> > Arguments that provide overlapping information make the function harder
> > to understand and use correctly (there are combinations of these
> > arguments that are invalid or don't make sense). Saving a few lines of
> > code in the function implementation is a poor trade-off IMO.
>
> My brain likes to agree but for some reason my heart really prefers
> block layer functions (where I know what can go wrong) over POSIX
> functions (where I always have the feeling of maybe not having though of
> everything).
>
> I guess I'll have to silence my heart for a bit, then.
In matters of the heart I cannot give advice on this mailing list,
sorry.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [Qemu-block] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate()
2017-03-23 15:32 ` Stefan Hajnoczi
@ 2017-03-27 14:19 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2017-03-27 14:19 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3131 bytes --]
On 23.03.2017 16:32, Stefan Hajnoczi wrote:
> On Wed, Mar 22, 2017 at 05:50:59PM +0100, Max Reitz wrote:
>> On 22.03.2017 17:28, Stefan Hajnoczi wrote:
>>> On Mon, Mar 20, 2017 at 04:07:16PM +0100, Max Reitz wrote:
>>>> On 20.03.2017 11:18, Stefan Hajnoczi wrote:
>>>>> On Mon, Mar 13, 2017 at 10:39:46PM +0100, Max Reitz wrote:
>>>>>> diff --git a/block/iscsi.c b/block/iscsi.c
>>>>>> index ab559a6f71..5d6265c4a6 100644
>>>>>> --- a/block/iscsi.c
>>>>>> +++ b/block/iscsi.c
>>>>>> @@ -2060,11 +2060,16 @@ static void iscsi_reopen_commit(BDRVReopenState *reopen_state)
>>>>>> }
>>>>>> }
>>>>>>
>>>>>> -static int iscsi_truncate(BlockDriverState *bs, int64_t offset, Error **errp)
>>>>>> +static int iscsi_truncate(BlockDriverState *bs, int64_t offset,
>>>>>> + PreallocMode prealloc, Error **errp)
>>>>>> {
>>>>>> IscsiLun *iscsilun = bs->opaque;
>>>>>> Error *local_err = NULL;
>>>>>>
>>>>>> + if (prealloc != PREALLOC_MODE_OFF) {
>>>>>> + return -ENOTSUP;
>>>>>> + }
>>>>>> +
>>>>>> if (iscsilun->type != TYPE_DISK) {
>>>>>> return -ENOTSUP;
>>>>>> }
>>>>>
>>>>> Nevermind what I said about adding a BiteSizedTasks entry:
>>>>>
>>>>> The missing errp usage is not in qemu.git/master yet. Please fix up
>>>>> your bdrv_truncate() errp patch to use errp in all cases, e.g.
>>>>> error_setg("Unable to truncate non-disk LUN").
>>>>
>>>> The thing is that I wasn't comfortable doing that for all block drivers.
>>>> I mean, I can take another look but I'd rather have vague error messages
>>>> ("truncation failed: #{strerror}") than outright wrong ones because I
>>>> didn't know what error message to use.
>>>>
>>>> Of course you could argue that this may probably come out during review
>>>> but that implies that every submaintainer for every block driver would
>>>> actually come out for review...
>>>
>>> I'm worried about errp being set in only a subset of error cases.
>>>
>>> This is likely to cause bugs if callers use if (local_err). Grepping
>>> through the codebase I can see instances of:
>>
>> Yes, but the generic bdrv_truncate() will always set errp if the driver
>> hasn't done so.
>
> I prefer consistently setting errp to make patch review easy (no
> checking all callers to see how they behave), making it safe to
> copy-paste the code elsewhere, etc.
The thing is, there is only a single caller for all of the
BlockDriver.bdrv_truncate() functions and that is the generic
bdrv_truncate() function.
I mean, of course I can copy-paste my generic error message from the
global bdrv_truncate() function into every driver where I'm not sure
what error to emit, but I find that a bit... Well, it looks bad when you
just look at the code without the history behind it.
But since you insist, I'll do so gladly. :-)
Max
> It's safer to be consistent, can produce more detailed error messages,
> and the cost of writing error_setg() calls is low. But it's a matter of
> style, you've pointed out the code is technically correct right now.
>
> Stefan
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate
2017-03-20 15:14 ` Max Reitz
@ 2017-03-29 22:12 ` Max Reitz
0 siblings, 0 replies; 49+ messages in thread
From: Max Reitz @ 2017-03-29 22:12 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: qemu-block, Kevin Wolf, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3262 bytes --]
On 20.03.2017 16:14, Max Reitz wrote:
> On 20.03.2017 12:26, Stefan Hajnoczi wrote:
>> On Mon, Mar 13, 2017 at 10:41:14PM +0100, Max Reitz wrote:
>>> This patch extends qcow2_calc_size_usage() so it can calculate the
>>> additional space needed for preallocating image growth.
>>>
>>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>>> ---
>>> block/qcow2.c | 137 +++++++++++++++++++++++++++++++++++++++++-----------------
>>> 1 file changed, 98 insertions(+), 39 deletions(-)
>>>
>>> diff --git a/block/qcow2.c b/block/qcow2.c
>>> index 21b2b3cd53..80fb815b15 100644
>>> --- a/block/qcow2.c
>>> +++ b/block/qcow2.c
>>> @@ -2101,7 +2101,15 @@ done:
>>> return ret;
>>> }
>>>
>>> -static uint64_t qcow2_calc_size_usage(uint64_t new_size,
>>> +/**
>>> + * Returns the number of bytes that must be allocated in the underlying file
>>> + * to accomodate an image growth from @current_size to @new_size.
>>> + *
>>> + * @current_size must be 0 when creating a new image. In that case, @bs is
>>> + * ignored; otherwise it must be valid.
>>> + */
>>> +static uint64_t qcow2_calc_size_usage(BlockDriverState *bs,
>>> + uint64_t current_size, uint64_t new_size,
>>> int cluster_bits, int refcount_order)
>>> {
>>> size_t cluster_size = 1u << cluster_bits;
>>> @@ -2122,47 +2130,97 @@ static uint64_t qcow2_calc_size_usage(uint64_t new_size,
>>> refblock_bits = cluster_bits - (refcount_order - 3);
>>> refblock_size = 1 << refblock_bits;
>>>
>>> - /* header: 1 cluster */
>>> - meta_size += cluster_size;
>>> -
>>> - /* total size of L2 tables */
>>> - nl2e = aligned_total_size / cluster_size;
>>> - nl2e = align_offset(nl2e, cluster_size / sizeof(uint64_t));
>>> - meta_size += nl2e * sizeof(uint64_t);
>>> + if (!current_size) {
>>
>> This massive if statement effectively makes two functions: the old
>> qcow2_calc_size_usage() and the new qcow2_calc_size_change(bs) function.
>>
>> It might be nicer to split the two functions.
>
> Hm, yes, but at that point I might as well just write two completely
> separate functions. I'll see what I can do, maybe I'll just put the new
> logic that's needed into a completely new function after all.
I'm having a bit of trouble with the split, due to a couple of reasons:
First, I can't think of a good name for the new function.
qcow2_calc_growth_size_usage() is a bit long and still not really
meaningful...
Second, having all the functionality in a single function means we can
"share" the explanation of how nrefblocke is calculated. I don't really
want to just put a reference comment there ("look it up in
qcow2_create2()"), but I definitely don't want to duplicate it either.
Finally, having it in a single function may not make much sense from the
inside but it does very much so from the outside. Even though we have to
follow much different code paths, from the outside it's pretty much the
same thing.
Therefore, I'm probably going to keep this patch as-is in v2 (for now),
expecting more criticism and stronger wording than "might be nicer",
forcing me to finally do the split in an eventual v3. O:-)
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 512 bytes --]
^ permalink raw reply [flat|nested] 49+ messages in thread
end of thread, other threads:[~2017-03-29 22:12 UTC | newest]
Thread overview: 49+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13 21:39 [Qemu-devel] [PATCH for-2.10 00/16] block: Preallocated truncate Max Reitz
2017-03-13 21:39 ` [Qemu-devel] [PATCH for-2.10 01/16] block: Add PreallocMode to BD.bdrv_truncate() Max Reitz
2017-03-20 10:10 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-20 10:18 ` Stefan Hajnoczi
2017-03-20 15:07 ` Max Reitz
2017-03-22 16:28 ` Stefan Hajnoczi
2017-03-22 16:50 ` Max Reitz
2017-03-23 15:32 ` Stefan Hajnoczi
2017-03-27 14:19 ` Max Reitz
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 02/16] block: Add PreallocMode to bdrv_truncate() Max Reitz
2017-03-20 11:33 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 03/16] block: Add PreallocMode to blk_truncate() Max Reitz
2017-03-20 10:33 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 04/16] qemu-img: Expose PreallocMode for resizing Max Reitz
2017-03-20 10:47 ` Stefan Hajnoczi
2017-03-20 15:07 ` Max Reitz
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 05/16] block/file-posix: Small fixes in raw_create() Max Reitz
2017-03-20 10:48 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 06/16] block/file-posix: Extract raw_regular_truncate() Max Reitz
2017-03-20 10:49 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 07/16] block/file-posix: Generalize raw_regular_truncate Max Reitz
2017-03-20 11:00 ` Stefan Hajnoczi
2017-03-20 15:11 ` Max Reitz
2017-03-22 16:44 ` Stefan Hajnoczi
2017-03-22 16:53 ` Max Reitz
2017-03-23 16:13 ` Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 08/16] block/file-posix: Preallocation for truncate Max Reitz
2017-03-20 11:01 ` Stefan Hajnoczi
2017-03-13 21:40 ` [Qemu-devel] [PATCH for-2.10 09/16] block/qcow2: Generalize preallocate() Max Reitz
2017-03-20 11:04 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-20 15:13 ` Max Reitz
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 10/16] block/qcow2: Lock s->lock in preallocate() Max Reitz
2017-03-20 11:05 ` Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 11/16] block/qcow2: Metadata preallocation for truncate Max Reitz
2017-03-20 11:23 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 12/16] block/qcow2: Extract qcow2_calc_size_usage() Max Reitz
2017-03-20 9:43 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 13/16] block/qcow2: qcow2_calc_size_usage() for truncate Max Reitz
2017-03-20 11:26 ` Stefan Hajnoczi
2017-03-20 15:14 ` Max Reitz
2017-03-29 22:12 ` Max Reitz
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 14/16] block/qcow2: falloc/full preallocating growth Max Reitz
2017-03-20 11:29 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-20 15:15 ` Max Reitz
2017-03-22 17:02 ` Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 15/16] iotests: Add preallocated resize test for raw Max Reitz
2017-03-20 11:30 ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-03-13 21:41 ` [Qemu-devel] [PATCH for-2.10 16/16] iotests: Add preallocated growth test for qcow2 Max Reitz
2017-03-20 11:31 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.