[v11] Take the image size into account when allocating the L2 cache

[Qemu-devel] [PATCH v11 7/9] qcow2: Resize the cache upon image resizing

Posted by Leonid Bloch 7 years, 1 month ago

The caches are now recalculated upon image resizing. This is done
because the new default behavior of assigning L2 cache relatively to
the image size, implies that the cache will be adapted accordingly
after an image resize.

Signed-off-by: Leonid Bloch <lbloch@janustech.com>
Reviewed-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 589f6c1b1c..c68f896c66 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3418,6 +3418,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
     uint64_t old_length;
     int64_t new_l1_size;
     int ret;
+    QDict *options;
 
     if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
         prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
@@ -3642,6 +3643,8 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
         }
     }
 
+    bs->total_sectors = offset / BDRV_SECTOR_SIZE;
+
     /* write updated header.size */
     offset = cpu_to_be64(offset);
     ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size),
@@ -3652,6 +3655,13 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
     }
 
     s->l1_vm_state_index = new_l1_size;
+
+    /* Update cache sizes */
+    options = qdict_clone_shallow(bs->options);
+    ret = qcow2_update_options(bs, options, s->flags, errp);
+    if (ret < 0) {
+        goto fail;
+    }
     ret = 0;
 fail:
     qemu_co_mutex_unlock(&s->lock);
-- 
2.17.1

Re: [Qemu-devel] [PATCH v11 7/9] qcow2: Resize the cache upon image resizing

Posted by Kevin Wolf 7 years, 1 month ago

Am 25.09.2018 um 00:53 hat Leonid Bloch geschrieben:
> The caches are now recalculated upon image resizing. This is done
> because the new default behavior of assigning L2 cache relatively to
> the image size, implies that the cache will be adapted accordingly
> after an image resize.
> 
> Signed-off-by: Leonid Bloch <lbloch@janustech.com>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 589f6c1b1c..c68f896c66 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3418,6 +3418,7 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>      uint64_t old_length;
>      int64_t new_l1_size;
>      int ret;
> +    QDict *options;
>  
>      if (prealloc != PREALLOC_MODE_OFF && prealloc != PREALLOC_MODE_METADATA &&
>          prealloc != PREALLOC_MODE_FALLOC && prealloc != PREALLOC_MODE_FULL)
> @@ -3642,6 +3643,8 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>          }
>      }
>  
> +    bs->total_sectors = offset / BDRV_SECTOR_SIZE;
> +
>      /* write updated header.size */
>      offset = cpu_to_be64(offset);
>      ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, size),
> @@ -3652,6 +3655,13 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>      }
>  
>      s->l1_vm_state_index = new_l1_size;
> +
> +    /* Update cache sizes */
> +    options = qdict_clone_shallow(bs->options);
> +    ret = qcow2_update_options(bs, options, s->flags, errp);
> +    if (ret < 0) {
> +        goto fail;
> +    }

Isn't options leaked, both in success and error cases?

Kevin

Re: [Qemu-devel] [PATCH v11 7/9] qcow2: Resize the cache upon image resizing

Posted by Alberto Garcia 7 years, 1 month ago

On Wed 26 Sep 2018 12:43:03 PM CEST, Kevin Wolf wrote:
>> +    /* Update cache sizes */
>> +    options = qdict_clone_shallow(bs->options);
>> +    ret = qcow2_update_options(bs, options, s->flags, errp);
>> +    if (ret < 0) {
>> +        goto fail;
>> +    }
>
> Isn't options leaked, both in success and error cases?

I'm embarrassed not to have seen that :-!

Berto

Re: [Qemu-devel] [PATCH v11 7/9] qcow2: Resize the cache upon image resizing

Posted by Leonid Bloch 7 years, 1 month ago

Oops, sorry. Would something like that be OK:

On 9/26/18 1:55 PM, Alberto Garcia wrote:
> On Wed 26 Sep 2018 12:43:03 PM CEST, Kevin Wolf wrote:
>>> +    /* Update cache sizes */
>>> +    options = qdict_clone_shallow(bs->options);
>>> +    ret = qcow2_update_options(bs, options, s->flags, errp);
+    qobject_unref(options);
>>> +    if (ret < 0) {
>>> +        goto fail;
>>> +    }
>>

Leonid.

>> Isn't options leaked, both in success and error cases?
> 
> I'm embarrassed not to have seen that :-!
> 
> Berto
>

Re: [Qemu-devel] [PATCH v11 7/9] qcow2: Resize the cache upon image resizing

Posted by Alberto Garcia 7 years, 1 month ago

On Wed 26 Sep 2018 04:21:21 PM CEST, Leonid Bloch wrote:
> Oops, sorry. Would something like that be OK:
>
> On 9/26/18 1:55 PM, Alberto Garcia wrote:
>> On Wed 26 Sep 2018 12:43:03 PM CEST, Kevin Wolf wrote:
>>>> +    /* Update cache sizes */
>>>> +    options = qdict_clone_shallow(bs->options);
>>>> +    ret = qcow2_update_options(bs, options, s->flags, errp);
> +    qobject_unref(options);
>>>> +    if (ret < 0) {
>>>> +        goto fail;
>>>> +    }
>>>

Yes, I think that should be enough.

Berto