On 2018-01-26 15:59, Alberto Garcia wrote: > After the previous patch we're now always using l2_load() in > get_cluster_table() regardless of whether a new L2 table has to be > allocated or not. > > This patch refactors that part of the code to use one single l2_load() > call. > > Signed-off-by: Alberto Garcia <be...@igalia.com> > --- > block/qcow2-cluster.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index 2a53c1dc5f..0c0cab85e8 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -695,15 +695,7 @@ static int get_cluster_table(BlockDriverState *bs, > uint64_t offset, > return -EIO; > } > > - /* seek the l2 table of the given l2 offset */ > - > - if (s->l1_table[l1_index] & QCOW_OFLAG_COPIED) { > - /* load the l2 table in memory */ > - ret = l2_load(bs, offset, l2_offset, &l2_table); > - if (ret < 0) { > - return ret; > - } > - } else { > + if (!(s->l1_table[l1_index] & QCOW_OFLAG_COPIED)) { > /* First allocate a new L2 table (and do COW if needed) */ > ret = l2_allocate(bs, l1_index); > if (ret < 0) { > @@ -719,11 +711,12 @@ static int get_cluster_table(BlockDriverState *bs, > uint64_t offset, > /* Get the offset of the newly-allocated l2 table */ > l2_offset = s->l1_table[l1_index] & L1E_OFFSET_MASK; > assert(offset_into_cluster(s, l2_offset) == 0); > - /* Load the l2 table in memory */ > - ret = l2_load(bs, offset, l2_offset, &l2_table); > - if (ret < 0) { > - return ret; > - } > + } > + > + /* load the l2 table in memory */ > + ret = l2_load(bs, offset, l2_offset, &l2_table); > + if (ret < 0) { > + return ret; > }
I'd pull the l2_offset assignment (and alignment check) down below the QCOW_OFLAG_COPIED check, saving us another two lines (!!1!) which we could then spend on an assert(s->l1_table[l1_index] & QCOW_OFLAG_COPIED); Max > > /* find the cluster offset for the given disk offset */ >
signature.asc
Description: OpenPGP digital signature