The caches are now recalculated upon image resizing. This is done
because the new default behavior of assigning a sufficient L2 cache to
cover the entire image implies that the cache will still be sufficient
after an image resizing.

Signed-off-by: Leonid Bloch <lbloch@janustech.com>
---
 block/qcow2.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 98cb96aaca..f60cb92169 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3639,6 +3639,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),
@@ -3649,6 +3651,12 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
     }
 
     s->l1_vm_state_index = new_l1_size;
+    /* Update cache sizes */
+    QDict *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

On Wed 08 Aug 2018 09:10:49 AM CEST, Leonid Bloch wrote:
> The caches are now recalculated upon image resizing. This is done
> because the new default behavior of assigning a sufficient L2 cache to
> cover the entire image implies that the cache will still be sufficient
> after an image resizing.

This is related to what I mentioned in the previous patch. The default
behavior doesn't make the cache try to cover the entire image (or at
least it doesn't *extend* the cache, which is what I understand from
this paragraph). What it does is *reduce* the cache if the smaller
version is enough for the entire image.

> Signed-off-by: Leonid Bloch <lbloch@janustech.com>
> ---
>  block/qcow2.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 98cb96aaca..f60cb92169 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3639,6 +3639,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),
> @@ -3649,6 +3651,12 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>      }
>  
>      s->l1_vm_state_index = new_l1_size;

You could add an empty line here for readability.

> +    /* Update cache sizes */
> +    QDict *options = qdict_clone_shallow(bs->options);

C99 allows variable declarations in the middle of a block, but we're
still doing it at the beginning (I don't know if there's a good reason
for this?).

Otherwise the patch looks good to me. Thanks!

Berto

On 08/08/2018 05:13 PM, Alberto Garcia wrote:
> On Wed 08 Aug 2018 09:10:49 AM CEST, Leonid Bloch wrote:
>> The caches are now recalculated upon image resizing. This is done
>> because the new default behavior of assigning a sufficient L2 cache to
>> cover the entire image implies that the cache will still be sufficient
>> after an image resizing.
> 
> This is related to what I mentioned in the previous patch. The default
> behavior doesn't make the cache try to cover the entire image (or at
> least it doesn't *extend* the cache, which is what I understand from
> this paragraph). What it does is *reduce* the cache if the smaller
> version is enough for the entire image.

But it doesn't say that it extends the cache. It says that it *adapts* 
the cache to the image size, and therefore it should be resized when the 
image is resized. At least I understand it this way. That said, I'd 
mention the limit there, instead of just "sufficient".

> 
>> Signed-off-by: Leonid Bloch <lbloch@janustech.com>
>> ---
>>   block/qcow2.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 98cb96aaca..f60cb92169 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -3639,6 +3639,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),
>> @@ -3649,6 +3651,12 @@ static int coroutine_fn qcow2_co_truncate(BlockDriverState *bs, int64_t offset,
>>       }
>>   
>>       s->l1_vm_state_index = new_l1_size;
> 
> You could add an empty line here for readability.

Yes, definitely, I will. Thanks.

> 
>> +    /* Update cache sizes */
>> +    QDict *options = qdict_clone_shallow(bs->options);
> 
> C99 allows variable declarations in the middle of a block, but we're
> still doing it at the beginning (I don't know if there's a good reason
> for this?).

I did it for readability, and I didn't see a style directive for this. 
But if the style requires it - no problem. :)

> 
> Otherwise the patch looks good to me. Thanks!

Thanks!

Leonid.

> 
> Berto
>