Re: [RFC PATCH v2 02/26] qcow2: Split cluster_needs_cow() out of count_cow_clusters()

2019-10-28 Thread Vladimir Sementsov-Ogievskiy
27.10.2019 0:25, Alberto Garcia wrote:
> We are going to need it in other places.
> 
> Signed-off-by: Alberto Garcia 
Reviewed-by: Vladimir Sementsov-Ogievskiy 

> ---
>   block/qcow2-cluster.c | 34 +++---
>   1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 6c1dcdc781..aa1010a515 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1068,6 +1068,24 @@ static void calculate_l2_meta(BlockDriverState *bs, 
> uint64_t host_offset,
>   QLIST_INSERT_HEAD(>cluster_allocs, *m, next_in_flight);
>   }
>   
> +/* Returns true if writing to a cluster requires COW */
> +static bool cluster_needs_cow(BlockDriverState *bs, uint64_t l2_entry)
> +{
> +switch (qcow2_get_cluster_type(bs, l2_entry)) {
> +case QCOW2_CLUSTER_NORMAL:
> +if (l2_entry & QCOW_OFLAG_COPIED) {
> +return false;
> +}
> +case QCOW2_CLUSTER_UNALLOCATED:
> +case QCOW2_CLUSTER_COMPRESSED:
> +case QCOW2_CLUSTER_ZERO_PLAIN:
> +case QCOW2_CLUSTER_ZERO_ALLOC:
> +return true;
> +default:
> +abort();
> +}
> +}

Not sure how much is it better than just
return !(qcow2_get_cluster_type(bs, l2_entry) == QCOW2_CLUSTER_NORMAL && 
(l2_entry & QCOW_OFLAG_COPIED));

> +
>   /*
>* Returns the number of contiguous clusters that can be used for an 
> allocating
>* write, but require COW to be performed (this includes yet unallocated 
> space,
> @@ -1080,25 +1098,11 @@ static int count_cow_clusters(BlockDriverState *bs, 
> int nb_clusters,
>   
>   for (i = 0; i < nb_clusters; i++) {
>   uint64_t l2_entry = be64_to_cpu(l2_slice[l2_index + i]);
> -QCow2ClusterType cluster_type = qcow2_get_cluster_type(bs, l2_entry);
> -
> -switch(cluster_type) {
> -case QCOW2_CLUSTER_NORMAL:
> -if (l2_entry & QCOW_OFLAG_COPIED) {
> -goto out;
> -}
> +if (!cluster_needs_cow(bs, l2_entry)) {
>   break;
> -case QCOW2_CLUSTER_UNALLOCATED:
> -case QCOW2_CLUSTER_COMPRESSED:
> -case QCOW2_CLUSTER_ZERO_PLAIN:
> -case QCOW2_CLUSTER_ZERO_ALLOC:
> -break;
> -default:
> -abort();
>   }
>   }
>   
> -out:
>   assert(i <= nb_clusters);
>   return i;
>   }
> 


-- 
Best regards,
Vladimir


[RFC PATCH v2 02/26] qcow2: Split cluster_needs_cow() out of count_cow_clusters()

2019-10-26 Thread Alberto Garcia
We are going to need it in other places.

Signed-off-by: Alberto Garcia 
---
 block/qcow2-cluster.c | 34 +++---
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 6c1dcdc781..aa1010a515 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1068,6 +1068,24 @@ static void calculate_l2_meta(BlockDriverState *bs, 
uint64_t host_offset,
 QLIST_INSERT_HEAD(>cluster_allocs, *m, next_in_flight);
 }
 
+/* Returns true if writing to a cluster requires COW */
+static bool cluster_needs_cow(BlockDriverState *bs, uint64_t l2_entry)
+{
+switch (qcow2_get_cluster_type(bs, l2_entry)) {
+case QCOW2_CLUSTER_NORMAL:
+if (l2_entry & QCOW_OFLAG_COPIED) {
+return false;
+}
+case QCOW2_CLUSTER_UNALLOCATED:
+case QCOW2_CLUSTER_COMPRESSED:
+case QCOW2_CLUSTER_ZERO_PLAIN:
+case QCOW2_CLUSTER_ZERO_ALLOC:
+return true;
+default:
+abort();
+}
+}
+
 /*
  * Returns the number of contiguous clusters that can be used for an allocating
  * write, but require COW to be performed (this includes yet unallocated space,
@@ -1080,25 +1098,11 @@ static int count_cow_clusters(BlockDriverState *bs, int 
nb_clusters,
 
 for (i = 0; i < nb_clusters; i++) {
 uint64_t l2_entry = be64_to_cpu(l2_slice[l2_index + i]);
-QCow2ClusterType cluster_type = qcow2_get_cluster_type(bs, l2_entry);
-
-switch(cluster_type) {
-case QCOW2_CLUSTER_NORMAL:
-if (l2_entry & QCOW_OFLAG_COPIED) {
-goto out;
-}
+if (!cluster_needs_cow(bs, l2_entry)) {
 break;
-case QCOW2_CLUSTER_UNALLOCATED:
-case QCOW2_CLUSTER_COMPRESSED:
-case QCOW2_CLUSTER_ZERO_PLAIN:
-case QCOW2_CLUSTER_ZERO_ALLOC:
-break;
-default:
-abort();
 }
 }
 
-out:
 assert(i <= nb_clusters);
 return i;
 }
-- 
2.20.1