[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 17/39] qcow2: Update l2_allocate() to support
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH v3 17/39] qcow2: Update l2_allocate() to support L2 slices |
Date: |
Wed, 31 Jan 2018 21:07:48 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 |
On 2018-01-26 15:59, Alberto Garcia wrote:
> This patch updates l2_allocate() to support the qcow2 cache returning
> L2 slices instead of full L2 tables.
>
> The old code simply gets an L2 table from the cache and initializes it
> with zeroes or with the contents of an existing table. With a cache
> that returns slices instead of tables the idea remains the same, but
> the code must now iterate over all the slices that are contained in an
> L2 table.
>
> Since now we're operating with slices the function can no longer
> return the newly-allocated table, so it's up to the caller to retrieve
> the appropriate L2 slice after calling l2_allocate() (note that with
> this patch the caller is still loading full L2 tables, but we'll deal
> with that in a separate patch).
>
> Signed-off-by: Alberto Garcia <address@hidden>
> ---
> block/qcow2-cluster.c | 56
> +++++++++++++++++++++++++++++++--------------------
> 1 file changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 57349928a9..2a53c1dc5f 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -264,11 +264,12 @@ int qcow2_write_l1_entry(BlockDriverState *bs, int
> l1_index)
> *
> */
>
> -static int l2_allocate(BlockDriverState *bs, int l1_index, uint64_t **table)
> +static int l2_allocate(BlockDriverState *bs, int l1_index)
> {
> BDRVQcow2State *s = bs->opaque;
> uint64_t old_l2_offset;
> - uint64_t *l2_table = NULL;
> + uint64_t *l2_slice = NULL;
> + unsigned slice, slice_size2, n_slices;
I'd personally prefer size_t, but oh well.
(And also maybe slice_size_bytes or slice_bytes instead of the
not-so-intuitive slice_size2. I know we're using *_size2 in other
places, but that's bad enough as it is.)
Overall (with that fixed or not, and with the spelling fixed as pointed
out by Eric);
Reviewed-by: Max Reitz <address@hidden>
However, I'm wondering whether this is the best approach. The old L2
table is probably not going to be used after this function, so we're
basically polluting the cache here. That was bad enough so far, but now
that actually means wasting multiple cache entries on it.
Sure, the code is simpler this way. But maybe it would still be better
to manually copy the data over from the old offset... (As long as it's
not much more complicated.)
Max
> int64_t l2_offset;
> int ret;
>
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH v3 39/39] iotests: Add l2-cache-entry-size to iotest 137, (continued)
- [Qemu-devel] [PATCH v3 27/39] qcow2: Update qcow2_update_snapshot_refcount() to support L2 slices, Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 04/39] qcow2: Remove BDS parameter from qcow2_cache_get_table_idx(), Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 34/39] qcow2: Rename l2_table in count_contiguous_clusters_unallocated(), Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 01/39] qcow2: Fix documentation of get_cluster_table(), Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 17/39] qcow2: Update l2_allocate() to support L2 slices, Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 20/39] qcow2: Update qcow2_get_cluster_offset() to support L2 slices, Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 32/39] qcow2: Rename l2_table in qcow2_alloc_compressed_cluster_offset(), Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 06/39] qcow2: Remove BDS parameter from qcow2_cache_entry_mark_dirty(), Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 38/39] iotests: Test downgrading an image using a small L2 slice size, Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 35/39] qcow2: Rename l2_table in count_cow_clusters(), Alberto Garcia, 2018/01/26
- [Qemu-devel] [PATCH v3 28/39] qcow2: Read refcount before L2 table in expand_zero_clusters_in_l1(), Alberto Garcia, 2018/01/26