Message ID | e2cb85926f1f4910c7d18fcc902be60109592381.1516978645.git.berto@igalia.com |
---|---|
State | New |
Headers | show |
Series | Allow configuring the qcow2 L2 cache entry size | expand |
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 <berto@igalia.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > 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; >
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 <berto@igalia.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> 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
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 <berto@igalia.com> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> --- >>> 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 <mreitz@redhat.com>
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;