qcow2: Take locks for accessing bs->file

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> Message-ID: <20231027155333.420094-22-kwolf@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
3 years ago · 8f8973416e
4 changed files with 59 additions and 41 deletions
--- a/block/qcow2-bitmap.c
+++ b/block/qcow2-bitmap.c
@ -105,7 +105,7 @@ static inline bool can_write(BlockDriverState *bs)
    return !bdrv_is_read_only(bs) && !(bdrv_get_flags(bs) & BDRV_O_INACTIVE);
 }

-static int update_header_sync(BlockDriverState *bs)
+static int GRAPH_RDLOCK update_header_sync(BlockDriverState *bs)
 {
    int ret;

@ -221,8 +221,9 @@ clear_bitmap_table(BlockDriverState *bs, uint64_t *bitmap_table,
    }
 }

-static int bitmap_table_load(BlockDriverState *bs, Qcow2BitmapTable *tb,
-                             uint64_t **bitmap_table)
+static int GRAPH_RDLOCK
+bitmap_table_load(BlockDriverState *bs, Qcow2BitmapTable *tb,
+                  uint64_t **bitmap_table)
 {
    int ret;
    BDRVQcow2State *s = bs->opaque;
@ -551,8 +552,9 @@ static uint32_t bitmap_list_count(Qcow2BitmapList *bm_list)
 * Get bitmap list from qcow2 image. Actually reads bitmap directory,
 * checks it and convert to bitmap list.
 */
-static Qcow2BitmapList *bitmap_list_load(BlockDriverState *bs, uint64_t offset,
-                                         uint64_t size, Error **errp)
+static Qcow2BitmapList * GRAPH_RDLOCK
+bitmap_list_load(BlockDriverState *bs, uint64_t offset, uint64_t size,
+                 Error **errp)
 {
    int ret;
    BDRVQcow2State *s = bs->opaque;
@ -961,7 +963,7 @@ static void set_readonly_helper(gpointer bitmap, gpointer value)
 * If header_updated is not NULL then it is set appropriately regardless of
 * the return value.
 */
-bool coroutine_fn GRAPH_RDLOCK
+bool coroutine_fn
 qcow2_load_dirty_bitmaps(BlockDriverState *bs,
                         bool *header_updated, Error **errp)
 {
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@ -391,11 +391,10 @@ fail:
 * If the L2 entry is invalid return -errno and set @type to
 * QCOW2_SUBCLUSTER_INVALID.
 */
-static int qcow2_get_subcluster_range_type(BlockDriverState *bs,
-                                           uint64_t l2_entry,
-                                           uint64_t l2_bitmap,
-                                           unsigned sc_from,
-                                           QCow2SubclusterType *type)
+static int GRAPH_RDLOCK
+qcow2_get_subcluster_range_type(BlockDriverState *bs, uint64_t l2_entry,
+                                uint64_t l2_bitmap, unsigned sc_from,
+                                QCow2SubclusterType *type)
 {
    BDRVQcow2State *s = bs->opaque;
    uint32_t val;
@ -442,9 +441,10 @@ static int qcow2_get_subcluster_range_type(BlockDriverState *bs,
 * On failure return -errno and update @l2_index to point to the
 * invalid entry.
 */
-static int count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
-                                        unsigned sc_index, uint64_t *l2_slice,
-                                        unsigned *l2_index)
+static int GRAPH_RDLOCK
+count_contiguous_subclusters(BlockDriverState *bs, int nb_clusters,
+                             unsigned sc_index, uint64_t *l2_slice,
+                             unsigned *l2_index)
 {
    BDRVQcow2State *s = bs->opaque;
    int i, count = 0;
@ -1329,7 +1329,8 @@ calculate_l2_meta(BlockDriverState *bs, uint64_t host_cluster_offset,
 * requires a new allocation (that is, if the cluster is unallocated
 * or has refcount > 1 and therefore cannot be written in-place).
 */
-static bool cluster_needs_new_alloc(BlockDriverState *bs, uint64_t l2_entry)
+static bool GRAPH_RDLOCK
+cluster_needs_new_alloc(BlockDriverState *bs, uint64_t l2_entry)
 {
    switch (qcow2_get_cluster_type(bs, l2_entry)) {
    case QCOW2_CLUSTER_NORMAL:
@ -1360,9 +1361,9 @@ static bool cluster_needs_new_alloc(BlockDriverState *bs, uint64_t l2_entry)
 * allocated and can be overwritten in-place (this includes clusters
 * of type QCOW2_CLUSTER_ZERO_ALLOC).
 */
-static int count_single_write_clusters(BlockDriverState *bs, int nb_clusters,
-                                       uint64_t *l2_slice, int l2_index,
-                                       bool new_alloc)
+static int GRAPH_RDLOCK
+count_single_write_clusters(BlockDriverState *bs, int nb_clusters,
+                            uint64_t *l2_slice, int l2_index, bool new_alloc)
 {
    BDRVQcow2State *s = bs->opaque;
    uint64_t l2_entry = get_l2_entry(s, l2_slice, l2_index);
--- a/block/qcow2.c
+++ b/block/qcow2.c
@ -95,9 +95,10 @@ static int qcow2_probe(const uint8_t *buf, int buf_size, const char *filename)
 }


-static int qcow2_crypto_hdr_read_func(QCryptoBlock *block, size_t offset,
-                                      uint8_t *buf, size_t buflen,
-                                      void *opaque, Error **errp)
+static int GRAPH_RDLOCK
+qcow2_crypto_hdr_read_func(QCryptoBlock *block, size_t offset,
+                           uint8_t *buf, size_t buflen,
+                           void *opaque, Error **errp)
 {
    BlockDriverState *bs = opaque;
    BDRVQcow2State *s = bs->opaque;
@ -156,7 +157,7 @@ qcow2_crypto_hdr_init_func(QCryptoBlock *block, size_t headerlen, void *opaque,


 /* The graph lock must be held when called in coroutine context */
-static int coroutine_mixed_fn
+static int coroutine_mixed_fn GRAPH_RDLOCK
 qcow2_crypto_hdr_write_func(QCryptoBlock *block, size_t offset,
                            const uint8_t *buf, size_t buflen,
                            void *opaque, Error **errp)
@ -2029,6 +2030,8 @@ static void qcow2_reopen_commit(BDRVReopenState *state)
 {
    BDRVQcow2State *s = state->bs->opaque;

+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
    qcow2_update_options_commit(state->bs, state->opaque);
    if (!s->data_file) {
        /*
@ -2064,6 +2067,8 @@ static void qcow2_reopen_abort(BDRVReopenState *state)
 {
    BDRVQcow2State *s = state->bs->opaque;

+    GRAPH_RDLOCK_GUARD_MAINLOOP();
+
    if (!s->data_file) {
        /*
         * If we don't have an external data file, s->data_file was cleared by
--- a/block/qcow2.h
+++ b/block/qcow2.h
@ -641,7 +641,7 @@ static inline void set_l2_bitmap(BDRVQcow2State *s, uint64_t *l2_slice,
    l2_slice[idx + 1] = cpu_to_be64(bitmap);
 }

-static inline bool has_data_file(BlockDriverState *bs)
+static inline bool GRAPH_RDLOCK has_data_file(BlockDriverState *bs)
 {
    BDRVQcow2State *s = bs->opaque;
    return (s->data_file != bs->file);
@ -709,8 +709,8 @@ static inline int64_t qcow2_vm_state_offset(BDRVQcow2State *s)
    return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
 }

-static inline QCow2ClusterType qcow2_get_cluster_type(BlockDriverState *bs,
-                                                      uint64_t l2_entry)
+static inline QCow2ClusterType GRAPH_RDLOCK
+qcow2_get_cluster_type(BlockDriverState *bs, uint64_t l2_entry)
 {
    BDRVQcow2State *s = bs->opaque;

@ -743,7 +743,7 @@ static inline QCow2ClusterType qcow2_get_cluster_type(BlockDriverState *bs,
 * (this checks the whole entry and bitmap, not only the bits related
 * to subcluster @sc_index).
 */
-static inline
+static inline GRAPH_RDLOCK
 QCow2SubclusterType qcow2_get_subcluster_type(BlockDriverState *bs,
                                              uint64_t l2_entry,
                                              uint64_t l2_bitmap,
@ -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);

 void GRAPH_RDLOCK
 qcow2_signal_corruption(BlockDriverState *bs, bool fatal, int64_t offset,
@ -890,10 +890,11 @@ int GRAPH_RDLOCK qcow2_write_caches(BlockDriverState *bs);
 int coroutine_fn qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                                       BdrvCheckMode fix);

-void qcow2_process_discards(BlockDriverState *bs, int ret);
+void GRAPH_RDLOCK qcow2_process_discards(BlockDriverState *bs, int ret);

-int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
-                                 int64_t size);
+int GRAPH_RDLOCK
+qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
+                             int64_t size);
 int GRAPH_RDLOCK
 qcow2_pre_write_overlap_check(BlockDriverState *bs, int ign, int64_t offset,
                              int64_t size, bool data_file);
@ -939,8 +940,9 @@ qcow2_alloc_host_offset(BlockDriverState *bs, uint64_t offset,
 int coroutine_fn GRAPH_RDLOCK
 qcow2_alloc_compressed_cluster_offset(BlockDriverState *bs, uint64_t offset,
                                      int compressed_size, uint64_t *host_offset);
-void qcow2_parse_compressed_l2_entry(BlockDriverState *bs, uint64_t l2_entry,
-                                     uint64_t *coffset, int *csize);
+void GRAPH_RDLOCK
+qcow2_parse_compressed_l2_entry(BlockDriverState *bs, uint64_t l2_entry,
+                                uint64_t *coffset, int *csize);

 int coroutine_fn GRAPH_RDLOCK
 qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m);
@ -993,8 +995,9 @@ qcow2_check_fix_snapshot_table(BlockDriverState *bs, BdrvCheckResult *result,
                               BdrvCheckMode fix);

 /* qcow2-cache.c functions */
-Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables,
-                               unsigned table_size);
+Qcow2Cache * GRAPH_RDLOCK
+qcow2_cache_create(BlockDriverState *bs, int num_tables, unsigned table_size);
+
 int qcow2_cache_destroy(Qcow2Cache *c);

 void qcow2_cache_entry_mark_dirty(Qcow2Cache *c, void *table);
@ -1020,17 +1023,24 @@ void *qcow2_cache_is_table_offset(Qcow2Cache *c, uint64_t offset);
 void qcow2_cache_discard(Qcow2Cache *c, void *table);

 /* qcow2-bitmap.c functions */
-int coroutine_fn
+int coroutine_fn GRAPH_RDLOCK
 qcow2_check_bitmaps_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                              void **refcount_table,
                              int64_t *refcount_table_size);
+
 bool coroutine_fn GRAPH_RDLOCK
-qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated, Error **errp);
-bool qcow2_get_bitmap_info_list(BlockDriverState *bs,
-                                Qcow2BitmapInfoList **info_list, Error **errp);
+qcow2_load_dirty_bitmaps(BlockDriverState *bs, bool *header_updated,
+                         Error **errp);
+
+bool GRAPH_RDLOCK
+qcow2_get_bitmap_info_list(BlockDriverState *bs,
+                           Qcow2BitmapInfoList **info_list, Error **errp);
+
 int GRAPH_RDLOCK qcow2_reopen_bitmaps_rw(BlockDriverState *bs, Error **errp);
 int GRAPH_RDLOCK qcow2_reopen_bitmaps_ro(BlockDriverState *bs, Error **errp);
-int coroutine_fn qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp);
+
+int coroutine_fn GRAPH_RDLOCK
+qcow2_truncate_bitmaps_check(BlockDriverState *bs, Error **errp);

 bool GRAPH_RDLOCK
 qcow2_store_persistent_dirty_bitmaps(BlockDriverState *bs, bool release_stored,