|
From: | Leonid Bloch |
Subject: | Re: [Qemu-block] [PATCH v3 3/5] qcow2: Resize the cache upon image resizing |
Date: | Wed, 8 Aug 2018 17:52:54 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
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 <address@hidden> --- 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
[Prev in Thread] | Current Thread | [Next in Thread] |