Skip to content

Commit

Permalink
qcow2: Take locks for accessing bs->file
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@kevmw
kevmw committed Nov 8, 2023
1 parent 79a5586 commit 8f89734
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 41 deletions.
14 changes: 8 additions & 6 deletions block/qcow2-bitmap.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down
25 changes: 13 additions & 12 deletions block/qcow2-cluster.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,11 +391,10 @@ static int GRAPH_RDLOCK l2_allocate(BlockDriverState *bs, int l1_index)
* 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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 9 additions & 4 deletions block/qcow2.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
/*
Expand Down Expand Up @@ -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
Expand Down
48 changes: 29 additions & 19 deletions block/qcow2.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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,
Expand Down

0 comments on commit 8f89734

Please sign in to comment.