Re: [PATCH 21/24] qcow2: Take locks for accessing bs->file

[Eric Blake]

On Fri, Oct 27, 2023 at 05:53:30PM +0200, Kevin Wolf wrote:
> This updates the qcow2 code to add GRAPH_RDLOCK annotations for all
> places that read bs->file.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.h         | 48 ++++++++++++++++++++++++++-----------------
>  block/qcow2-bitmap.c  | 14 +++++++------
>  block/qcow2-cluster.c | 25 +++++++++++-----------
>  block/qcow2.c         | 13 ++++++++----
>  4 files changed, 59 insertions(+), 41 deletions(-)
> 

> @@ -834,9 +834,9 @@ int64_t qcow2_refcount_metadata_size(int64_t clusters, 
> size_t cluster_size,
>                                       int refcount_order, bool 
> generous_increase,
>                                       uint64_t *refblock_count);
>  
> -int qcow2_mark_dirty(BlockDriverState *bs);
> -int qcow2_mark_corrupt(BlockDriverState *bs);
> -int qcow2_update_header(BlockDriverState *bs);
> +int GRAPH_RDLOCK qcow2_mark_dirty(BlockDriverState *bs);
> +int GRAPH_RDLOCK qcow2_mark_corrupt(BlockDriverState *bs);
> +int GRAPH_RDLOCK qcow2_update_header(BlockDriverState *bs);
>

My first thought was "why is this a read lock when these functions are
modifying the qcow2 headers?  Isn't that a write operation?"  But in
thinking further - they are reading the graph structure, not modifing
it (bs->file is not being changed; we may be modifying the contents to
be stored in bs->file, but whether or not that change is done now or
queued for the next flush, we still keep the same bs->file), so it
looks right after all - these really are operations where we don't
want the graph changed out from underneath us while we are modifying
the qcow2 header.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org