The current text corresponds to an earlier, simpler version of this
function and it does not explain how it works now.

Signed-off-by: Alberto Garcia <berto@igalia.com>
---
 block/qcow2-cluster.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 25e38daa78..f1ce6afcf5 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1713,18 +1713,22 @@ out:
 }
 
 /*
- * alloc_cluster_offset
+ * For a given area on the virtual disk defined by @offset and @bytes,
+ * find the corresponding area on the qcow2 image, allocating new
+ * clusters (or subclusters) if necessary. The result can span a
+ * combination of allocated and previously unallocated clusters.
  *
- * For a given offset on the virtual disk, find the cluster offset in qcow2
- * file. If the offset is not found, allocate a new cluster.
+ * On return, @host_offset is set to the beginning of the requested
+ * area. This area is guaranteed to be contiguous on the qcow2 file
+ * but it can be smaller than initially requested. In this case @bytes
+ * is updated with the actual size.
  *
- * If the cluster was already allocated, m->nb_clusters is set to 0 and
- * other fields in m are meaningless.
- *
- * If the cluster is newly allocated, m->nb_clusters is set to the number of
- * contiguous clusters that have been allocated. In this case, the other
- * fields of m are valid and contain information about the first allocated
- * cluster.
+ * If any clusters or subclusters were allocated then @m contains a
+ * list with the information of all the affected regions. Note that
+ * this can happen regardless of whether this function succeeds or
+ * not. The caller is responsible for updating the L2 metadata of the
+ * allocated clusters (on success) or freeing them (on failure), and
+ * for clearing the contents of @m afterwards in both cases.
  *
  * If the request conflicts with another write request in flight, the coroutine
  * is queued and will be reentered when the dependency has completed.
-- 
2.20.1

On 03.09.20 18:37, Alberto Garcia wrote:
> The current text corresponds to an earlier, simpler version of this
> function and it does not explain how it works now.
> 
> Signed-off-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/qcow2-cluster.c | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 25e38daa78..f1ce6afcf5 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -1713,18 +1713,22 @@ out:
>  }
>  
>  /*
> - * alloc_cluster_offset
> + * For a given area on the virtual disk defined by @offset and @bytes,
> + * find the corresponding area on the qcow2 image, allocating new
> + * clusters (or subclusters) if necessary. The result can span a
> + * combination of allocated and previously unallocated clusters.
>   *
> - * For a given offset on the virtual disk, find the cluster offset in qcow2
> - * file. If the offset is not found, allocate a new cluster.
> + * On return, @host_offset is set to the beginning of the requested
> + * area. This area is guaranteed to be contiguous on the qcow2 file
> + * but it can be smaller than initially requested. In this case @bytes
> + * is updated with the actual size.
>   *
> - * If the cluster was already allocated, m->nb_clusters is set to 0 and
> - * other fields in m are meaningless.
> - *
> - * If the cluster is newly allocated, m->nb_clusters is set to the number of
> - * contiguous clusters that have been allocated. In this case, the other
> - * fields of m are valid and contain information about the first allocated
> - * cluster.
> + * If any clusters or subclusters were allocated then @m contains a
> + * list with the information of all the affected regions. Note that
> + * this can happen regardless of whether this function succeeds or
> + * not. The caller is responsible for updating the L2 metadata of the
> + * allocated clusters (on success) or freeing them (on failure), and
> + * for clearing the contents of @m afterwards in both cases.

It’s too bad that preallocate_co() doesn’t do that then, isn’t it...

Max

On Fri 04 Sep 2020 11:28:18 AM CEST, Max Reitz wrote:
>> + * If any clusters or subclusters were allocated then @m contains a
>> + * list with the information of all the affected regions. Note that
>> + * this can happen regardless of whether this function succeeds or
>> + * not. The caller is responsible for updating the L2 metadata of the
>> + * allocated clusters (on success) or freeing them (on failure), and
>> + * for clearing the contents of @m afterwards in both cases.
>
> It’s too bad that preallocate_co() doesn’t do that then, isn’t it...

I was planning to have a closer look at that function next week.

Berto

On 04.09.20 11:36, Alberto Garcia wrote:
> On Fri 04 Sep 2020 11:28:18 AM CEST, Max Reitz wrote:
>>> + * If any clusters or subclusters were allocated then @m contains a
>>> + * list with the information of all the affected regions. Note that
>>> + * this can happen regardless of whether this function succeeds or
>>> + * not. The caller is responsible for updating the L2 metadata of the
>>> + * allocated clusters (on success) or freeing them (on failure), and
>>> + * for clearing the contents of @m afterwards in both cases.
>>
>> It’s too bad that preallocate_co() doesn’t do that then, isn’t it...
> 
> I was planning to have a closer look at that function next week.

Great! :)

Max