| Message ID | bb5bd06f07c5a05b0818611de0d06ec5b66c8df3.1599150873.git.berto@igalia.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2: Fix removal of list members from BDRVQcow2State.cluster_allocs | expand |
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
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.
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(-)