Re: [Qemu-devel] [PATCH v3 13/39] qcow2: Add l2_slice_size field to BDRVQcow2State

[Max Reitz]

On 2018-02-01 10:51, Alberto Garcia wrote:
> On Wed 31 Jan 2018 08:48:08 PM CET, Max Reitz wrote:
>> On 2018-01-26 15:59, Alberto Garcia wrote:
>>> The BDRVQcow2State structure contains an l2_size field, which stores
>>> the number of 64-bit entries in an L2 table.
>>>
>>> For efficiency reasons we want to be able to load slices instead of
>>> full L2 tables, so we need to know how many entries an L2 slice can
>>> hold.
>>>
>>> An L2 slice is the portion of an L2 table that is loaded by the qcow2
>>> cache. At the moment that cache can only load complete tables,
>>> therefore an L2 slice has the same size as an L2 table (one cluster)
>>> and l2_size == l2_slice_size.
>>>
>>> Later we'll allow smaller slices, but until then we have to use this
>>> new l2_slice_size field to make the rest of the code ready for that.
>>>
>>> Signed-off-by: Alberto Garcia 
>>> Reviewed-by: Eric Blake 
>>> ---
>>>  block/qcow2.c | 3 +++
>>>  block/qcow2.h | 1 +
>>>  2 files changed, 4 insertions(+)
>>
>> Am I missing something or does this patch miss setting l2_slice_size
>> in qcow2_do_open()?
> 
> qcow2_do_open() calls qcow2_update_options() which is what reads
> l2-cache-entry-size and sets s->l2_slice_size.

So I was missing something, good.

Reviewed-by: Max Reitz 



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v3 13/39] qcow2: Add l2_slice_size field to BDRVQcow2State

[Alberto Garcia]

On Wed 31 Jan 2018 08:48:08 PM CET, Max Reitz wrote:
> On 2018-01-26 15:59, Alberto Garcia wrote:
>> The BDRVQcow2State structure contains an l2_size field, which stores
>> the number of 64-bit entries in an L2 table.
>> 
>> For efficiency reasons we want to be able to load slices instead of
>> full L2 tables, so we need to know how many entries an L2 slice can
>> hold.
>> 
>> An L2 slice is the portion of an L2 table that is loaded by the qcow2
>> cache. At the moment that cache can only load complete tables,
>> therefore an L2 slice has the same size as an L2 table (one cluster)
>> and l2_size == l2_slice_size.
>> 
>> Later we'll allow smaller slices, but until then we have to use this
>> new l2_slice_size field to make the rest of the code ready for that.
>> 
>> Signed-off-by: Alberto Garcia 
>> Reviewed-by: Eric Blake 
>> ---
>>  block/qcow2.c | 3 +++
>>  block/qcow2.h | 1 +
>>  2 files changed, 4 insertions(+)
>
> Am I missing something or does this patch miss setting l2_slice_size
> in qcow2_do_open()?

qcow2_do_open() calls qcow2_update_options() which is what reads
l2-cache-entry-size and sets s->l2_slice_size.

Berto

Re: [Qemu-devel] [PATCH v3 13/39] qcow2: Add l2_slice_size field to BDRVQcow2State

[Max Reitz]

On 2018-01-26 15:59, Alberto Garcia wrote:
> The BDRVQcow2State structure contains an l2_size field, which stores
> the number of 64-bit entries in an L2 table.
> 
> For efficiency reasons we want to be able to load slices instead of
> full L2 tables, so we need to know how many entries an L2 slice can
> hold.
> 
> An L2 slice is the portion of an L2 table that is loaded by the qcow2
> cache. At the moment that cache can only load complete tables,
> therefore an L2 slice has the same size as an L2 table (one cluster)
> and l2_size == l2_slice_size.
> 
> Later we'll allow smaller slices, but until then we have to use this
> new l2_slice_size field to make the rest of the code ready for that.
> 
> Signed-off-by: Alberto Garcia 
> Reviewed-by: Eric Blake 
> ---
>  block/qcow2.c | 3 +++
>  block/qcow2.h | 1 +
>  2 files changed, 4 insertions(+)

Am I missing something or does this patch miss setting l2_slice_size in
qcow2_do_open()?

Max

> diff --git a/block/qcow2.c b/block/qcow2.c
> index e2d4bf7ad5..78f067cae7 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -805,6 +805,7 @@ static void read_cache_sizes(BlockDriverState *bs, 
> QemuOpts *opts,
>  typedef struct Qcow2ReopenState {
>  Qcow2Cache *l2_table_cache;
>  Qcow2Cache *refcount_block_cache;
> +int l2_slice_size; /* Number of entries in a slice of the L2 table */
>  bool use_lazy_refcounts;
>  int overlap_check;
>  bool discard_passthrough[QCOW2_DISCARD_MAX];
> @@ -886,6 +887,7 @@ static int qcow2_update_options_prepare(BlockDriverState 
> *bs,
>  }
>  }
>  
> +r->l2_slice_size = s->cluster_size / sizeof(uint64_t);
>  r->l2_table_cache = qcow2_cache_create(bs, l2_cache_size);
>  r->refcount_block_cache = qcow2_cache_create(bs, refcount_cache_size);
>  if (r->l2_table_cache == NULL || r->refcount_block_cache == NULL) {
> @@ -1049,6 +1051,7 @@ static void 
> qcow2_update_options_commit(BlockDriverState *bs,
>  }
>  s->l2_table_cache = r->l2_table_cache;
>  s->refcount_block_cache = r->refcount_block_cache;
> +s->l2_slice_size = r->l2_slice_size;
>  
>  s->overlap_check = r->overlap_check;
>  s->use_lazy_refcounts = r->use_lazy_refcounts;
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 0559afbc63..e0aee88811 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -251,6 +251,7 @@ typedef struct BDRVQcow2State {
>  int cluster_bits;
>  int cluster_size;
>  int cluster_sectors;
> +int l2_slice_size;
>  int l2_bits;
>  int l2_size;
>  int l1_size;
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v3 13/39] qcow2: Add l2_slice_size field to BDRVQcow2State

[Alberto Garcia]

The BDRVQcow2State structure contains an l2_size field, which stores
the number of 64-bit entries in an L2 table.

For efficiency reasons we want to be able to load slices instead of
full L2 tables, so we need to know how many entries an L2 slice can
hold.

An L2 slice is the portion of an L2 table that is loaded by the qcow2
cache. At the moment that cache can only load complete tables,
therefore an L2 slice has the same size as an L2 table (one cluster)
and l2_size == l2_slice_size.

Later we'll allow smaller slices, but until then we have to use this
new l2_slice_size field to make the rest of the code ready for that.

Signed-off-by: Alberto Garcia 
Reviewed-by: Eric Blake 
---
 block/qcow2.c | 3 +++
 block/qcow2.h | 1 +
 2 files changed, 4 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index e2d4bf7ad5..78f067cae7 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -805,6 +805,7 @@ static void read_cache_sizes(BlockDriverState *bs, QemuOpts 
*opts,
 typedef struct Qcow2ReopenState {
 Qcow2Cache *l2_table_cache;
 Qcow2Cache *refcount_block_cache;
+int l2_slice_size; /* Number of entries in a slice of the L2 table */
 bool use_lazy_refcounts;
 int overlap_check;
 bool discard_passthrough[QCOW2_DISCARD_MAX];
@@ -886,6 +887,7 @@ static int qcow2_update_options_prepare(BlockDriverState 
*bs,
 }
 }
 
+r->l2_slice_size = s->cluster_size / sizeof(uint64_t);
 r->l2_table_cache = qcow2_cache_create(bs, l2_cache_size);
 r->refcount_block_cache = qcow2_cache_create(bs, refcount_cache_size);
 if (r->l2_table_cache == NULL || r->refcount_block_cache == NULL) {
@@ -1049,6 +1051,7 @@ static void qcow2_update_options_commit(BlockDriverState 
*bs,
 }
 s->l2_table_cache = r->l2_table_cache;
 s->refcount_block_cache = r->refcount_block_cache;
+s->l2_slice_size = r->l2_slice_size;
 
 s->overlap_check = r->overlap_check;
 s->use_lazy_refcounts = r->use_lazy_refcounts;
diff --git a/block/qcow2.h b/block/qcow2.h
index 0559afbc63..e0aee88811 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -251,6 +251,7 @@ typedef struct BDRVQcow2State {
 int cluster_bits;
 int cluster_size;
 int cluster_sectors;
+int l2_slice_size;
 int l2_bits;
 int l2_size;
 int l1_size;
-- 
2.11.0