Skip to content

Commit

Permalink
migration: mark mixed functions that can suspend
Browse files Browse the repository at this point in the history
There should be no paths from a coroutine_fn to aio_poll, however in
practice coroutine_mixed_fn will call aio_poll in the !qemu_in_coroutine()
path.  By marking mixed functions, we can track accurately the call paths
that execute entirely in coroutine context, and find more missing
coroutine_fn markers.  This results in more accurate checks that
coroutine code does not end up blocking.

If the marking were extended transitively to all functions that call
these ones, static analysis could be done much more efficiently.
However, this is a start and makes it possible to use vrc's path-based
searches to find potential bugs where coroutine_fns call blocking functions.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
bonzini committed Apr 20, 2023
1 parent 1dd91b2 commit 394b940
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 12 deletions.
4 changes: 2 additions & 2 deletions include/migration/qemu-file-types.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ void qemu_put_byte(QEMUFile *f, int v);
void qemu_put_be16(QEMUFile *f, unsigned int v);
void qemu_put_be32(QEMUFile *f, unsigned int v);
void qemu_put_be64(QEMUFile *f, uint64_t v);
size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size);
size_t coroutine_mixed_fn qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size);

int qemu_get_byte(QEMUFile *f);

Expand Down Expand Up @@ -161,7 +161,7 @@ static inline void qemu_get_sbe64s(QEMUFile *f, int64_t *pv)
qemu_get_be64s(f, (uint64_t *)pv);
}

size_t qemu_get_counted_string(QEMUFile *f, char buf[256]);
size_t coroutine_mixed_fn qemu_get_counted_string(QEMUFile *f, char buf[256]);

void qemu_put_counted_string(QEMUFile *f, const char *name);

Expand Down
14 changes: 7 additions & 7 deletions migration/qemu-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset,
* case if the underlying file descriptor gives a short read, and that can
* happen even on a blocking fd.
*/
static ssize_t qemu_fill_buffer(QEMUFile *f)
static ssize_t coroutine_mixed_fn qemu_fill_buffer(QEMUFile *f)
{
int len;
int pending;
Expand Down Expand Up @@ -585,7 +585,7 @@ void qemu_file_skip(QEMUFile *f, int size)
* return as many as it managed to read (assuming blocking fd's which
* all current QEMUFile are)
*/
size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset)
size_t coroutine_mixed_fn qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset)
{
ssize_t pending;
size_t index;
Expand Down Expand Up @@ -633,7 +633,7 @@ size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset)
* return as many as it managed to read (assuming blocking fd's which
* all current QEMUFile are)
*/
size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
size_t coroutine_mixed_fn qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
{
size_t pending = size;
size_t done = 0;
Expand Down Expand Up @@ -674,7 +674,7 @@ size_t qemu_get_buffer(QEMUFile *f, uint8_t *buf, size_t size)
* Note: Since **buf may get changed, the caller should take care to
* keep a pointer to the original buffer if it needs to deallocate it.
*/
size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
size_t coroutine_mixed_fn qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
{
if (size < IO_BUF_SIZE) {
size_t res;
Expand All @@ -696,7 +696,7 @@ size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size)
* Peeks a single byte from the buffer; this isn't guaranteed to work if
* offset leaves a gap after the previous read/peeked data.
*/
int qemu_peek_byte(QEMUFile *f, int offset)
int coroutine_mixed_fn qemu_peek_byte(QEMUFile *f, int offset)
{
int index = f->buf_index + offset;

Expand All @@ -713,7 +713,7 @@ int qemu_peek_byte(QEMUFile *f, int offset)
return f->buf[index];
}

int qemu_get_byte(QEMUFile *f)
int coroutine_mixed_fn qemu_get_byte(QEMUFile *f)
{
int result;

Expand Down Expand Up @@ -894,7 +894,7 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src)
* else 0
* (Note a 0 length string will return 0 either way)
*/
size_t qemu_get_counted_string(QEMUFile *f, char buf[256])
size_t coroutine_fn qemu_get_counted_string(QEMUFile *f, char buf[256])
{
size_t len = qemu_get_byte(f);
size_t res = qemu_get_buffer(f, (uint8_t *)buf, len);
Expand Down
6 changes: 3 additions & 3 deletions migration/qemu-file.h
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,8 @@ bool qemu_file_is_writable(QEMUFile *f);

#include "migration/qemu-file-types.h"

size_t qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset);
size_t qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size);
size_t coroutine_mixed_fn qemu_peek_buffer(QEMUFile *f, uint8_t **buf, size_t size, size_t offset);
size_t coroutine_mixed_fn qemu_get_buffer_in_place(QEMUFile *f, uint8_t **buf, size_t size);
ssize_t qemu_put_compression_data(QEMUFile *f, z_stream *stream,
const uint8_t *p, size_t size);
int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
Expand All @@ -119,7 +119,7 @@ int qemu_put_qemu_file(QEMUFile *f_des, QEMUFile *f_src);
* is; you aren't guaranteed to be able to peak to +n bytes unless you've
* previously peeked +n-1.
*/
int qemu_peek_byte(QEMUFile *f, int offset);
int coroutine_mixed_fn qemu_peek_byte(QEMUFile *f, int offset);
void qemu_file_skip(QEMUFile *f, int size);
/*
* qemu_file_credit_transfer:
Expand Down

0 comments on commit 394b940

Please sign in to comment.