[v3,09/16] block/qcow2: Generalize preallocate()
diff mbox

Message ID 20170526165518.7580-10-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz May 26, 2017, 4:55 p.m. UTC
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 | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

Comments

Eric Blake May 30, 2017, 9:26 p.m. UTC | #1
On 05/26/2017 11:55 AM, 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.

bdrv_getlength() is (currently) always sector-aligned (rounding up as
needed).

new_length is passed from qcow2_create2()'s total_size, which in turn
comes from qcow2_create()'s size, which can be user-supplied - but also
appears that we round up to ensure it is always sector-aligned.

Testing: 'qemu-img create -f qcow2 a.img 1' reports "size=1", but
'qemu-img info a.img' reports "virtual size: 512" - so we have a
secondary bug worth fixing later in that we are rounding AFTER what we
report to the user, but I'm not seeing a behavior change in this patch.

> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>
Stefan Hajnoczi May 31, 2017, 10:22 a.m. UTC | #2
On Fri, May 26, 2017 at 06:55:11PM +0200, 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.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/qcow2.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Patch
diff mbox

diff --git a/block/qcow2.c b/block/qcow2.c
index 3d4f36d..df953aa 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2035,17 +2035,24 @@  static int qcow2_change_backing_file(BlockDriverState *bs,
     return qcow2_update_header(bs);
 }
 
-static int preallocate(BlockDriverState *bs)
+/**
+ * Preallocates metadata structures for data clusters between @offset (in the
+ * guest disk) and @new_length (which is thus generally the new guest disk
+ * size).
+ *
+ * Returns: 0 on success, -errno on failure.
+ */
+static int preallocate(BlockDriverState *bs,
+                       uint64_t offset, uint64_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);
@@ -2384,7 +2391,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");