Skip to content

Commit

Permalink
migration: restrict scope of incoming fd read handler
Browse files Browse the repository at this point in the history
The incoming migration is processed in a coroutine and uses an fd read
handler to enter the yielded coroutine when data becomes available.

The read handler was set too broadly, so that spurious coroutine entries
were be triggered if other coroutine users yielded (like the block
layer's bdrv_write() function).

Install the fd read only only when yielding for more data to become
available.  This prevents spurious coroutine entries which break code
that assumes only a specific set of places can re-enter the coroutine.

This patch fixes crashes in block/raw-posix.c that are triggered with
"migrate -b" when qiov becomes a dangling pointer due to a spurious
coroutine entry that frees qiov early.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1360598505-5512-1-git-send-email-stefanha@redhat.com
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
  • Loading branch information
stefanhaRH authored and Anthony Liguori committed Feb 11, 2013
1 parent d0bce76 commit d7cd369
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 12 deletions.
8 changes: 0 additions & 8 deletions migration.c
Expand Up @@ -95,7 +95,6 @@ static void process_incoming_migration_co(void *opaque)
int ret;

ret = qemu_loadvm_state(f);
qemu_set_fd_handler(qemu_get_fd(f), NULL, NULL, NULL);
qemu_fclose(f);
if (ret < 0) {
fprintf(stderr, "load of migration failed\n");
Expand All @@ -115,20 +114,13 @@ static void process_incoming_migration_co(void *opaque)
}
}

static void enter_migration_coroutine(void *opaque)
{
Coroutine *co = opaque;
qemu_coroutine_enter(co, NULL);
}

void process_incoming_migration(QEMUFile *f)
{
Coroutine *co = qemu_coroutine_create(process_incoming_migration_co);
int fd = qemu_get_fd(f);

assert(fd != -1);
socket_set_nonblock(fd);
qemu_set_fd_handler(fd, enter_migration_coroutine, NULL, co);
qemu_coroutine_enter(co, f);
}

Expand Down
34 changes: 30 additions & 4 deletions savevm.c
Expand Up @@ -140,6 +140,34 @@ typedef struct QEMUFileSocket
QEMUFile *file;
} QEMUFileSocket;

typedef struct {
Coroutine *co;
int fd;
} FDYieldUntilData;

static void fd_coroutine_enter(void *opaque)
{
FDYieldUntilData *data = opaque;
qemu_set_fd_handler(data->fd, NULL, NULL, NULL);
qemu_coroutine_enter(data->co, NULL);
}

/**
* Yield until a file descriptor becomes readable
*
* Note that this function clobbers the handlers for the file descriptor.
*/
static void coroutine_fn yield_until_fd_readable(int fd)
{
FDYieldUntilData data;

assert(qemu_in_coroutine());
data.co = qemu_coroutine_self();
data.fd = fd;
qemu_set_fd_handler(fd, fd_coroutine_enter, NULL, &data);
qemu_coroutine_yield();
}

static int socket_get_fd(void *opaque)
{
QEMUFileSocket *s = opaque;
Expand All @@ -158,8 +186,7 @@ static int socket_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
break;
}
if (socket_error() == EAGAIN) {
assert(qemu_in_coroutine());
qemu_coroutine_yield();
yield_until_fd_readable(s->fd);
} else if (socket_error() != EINTR) {
break;
}
Expand Down Expand Up @@ -205,8 +232,7 @@ static int stdio_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size)
break;
}
if (errno == EAGAIN) {
assert(qemu_in_coroutine());
qemu_coroutine_yield();
yield_until_fd_readable(fileno(fp));
} else if (errno != EINTR) {
break;
}
Expand Down

0 comments on commit d7cd369

Please sign in to comment.