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 <kw...@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 <ebl...@redhat.com> -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org