Skip to content

Commit

Permalink
migration (outgoing): add error propagation for all protocols
Browse files Browse the repository at this point in the history
Error propagation is already there for socket backends.  Add it to other
protocols, simplifying code that tests for errors that will never happen.
With all protocols understanding Error, the code can be simplified
further by removing the return value.

Unfortunately, the quality of error messages varies depending
on where the error is detected, because no Error is passed to the
NonBlockingConnectHandler.  Thus, the exact error message still cannot
be sent to the user if the OS reports it asynchronously via SO_ERROR.
If NonBlockingConnectHandler received an Error**, we could for
example report the error class and/or message via a new field of the
query-migration command even if it is reported asynchronously.

Before:

    (qemu) migrate fd:ffff
    migrate: An undefined error has occurred
    (qemu) info migrate
    (qemu)

After:

    (qemu) migrate fd:ffff
    migrate: File descriptor named 'ffff' has not been found
    (qemu) info migrate
    capabilities: xbzrle: off
    Migration status: failed
    total time: 0 milliseconds

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
  • Loading branch information
bonzini committed Oct 23, 2012
1 parent e08c95c commit f37afb5
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 65 deletions.
18 changes: 4 additions & 14 deletions migration-exec.c
Expand Up @@ -60,22 +60,18 @@ static int exec_close(MigrationState *s)
return ret;
}

int exec_start_outgoing_migration(MigrationState *s, const char *command)
void exec_start_outgoing_migration(MigrationState *s, const char *command, Error **errp)
{
FILE *f;

f = popen(command, "w");
if (f == NULL) {
DPRINTF("Unable to popen exec target\n");
goto err_after_popen;
error_setg_errno(errp, errno, "failed to popen the migration target");
return;
}

s->fd = fileno(f);
if (s->fd == -1) {
DPRINTF("Unable to retrieve file descriptor for popen'd handle\n");
goto err_after_open;
}

assert(s->fd != -1);
socket_set_nonblock(s->fd);

s->opaque = qemu_popen(f, "w");
Expand All @@ -85,12 +81,6 @@ int exec_start_outgoing_migration(MigrationState *s, const char *command)
s->write = file_write;

migrate_fd_connect(s);
return 0;

err_after_open:
pclose(f);
err_after_popen:
return -1;
}

static void exec_accept_incoming_migration(void *opaque)
Expand Down
19 changes: 4 additions & 15 deletions migration-fd.c
Expand Up @@ -73,30 +73,19 @@ static int fd_close(MigrationState *s)
return 0;
}

int fd_start_outgoing_migration(MigrationState *s, const char *fdname)
void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp)
{
s->fd = monitor_get_fd(cur_mon, fdname, NULL);
s->fd = monitor_get_fd(cur_mon, fdname, errp);
if (s->fd == -1) {
DPRINTF("fd_migration: invalid file descriptor identifier\n");
goto err_after_get_fd;
}

if (fcntl(s->fd, F_SETFL, O_NONBLOCK) == -1) {
DPRINTF("Unable to set nonblocking mode on file descriptor\n");
goto err_after_open;
return;
}

fcntl(s->fd, F_SETFL, O_NONBLOCK);
s->get_error = fd_errno;
s->write = fd_write;
s->close = fd_close;

migrate_fd_connect(s);
return 0;

err_after_open:
close(s->fd);
err_after_get_fd:
return -1;
}

static void fd_accept_incoming_migration(void *opaque)
Expand Down
13 changes: 2 additions & 11 deletions migration-tcp.c
Expand Up @@ -68,22 +68,13 @@ static void tcp_wait_for_connect(int fd, void *opaque)
}
}

int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
Error **errp)
void tcp_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp)
{
Error *local_err = NULL;

s->get_error = socket_errno;
s->write = socket_write;
s->close = tcp_close;

s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
return -1;
}

return 0;
s->fd = inet_nonblocking_connect(host_port, tcp_wait_for_connect, s, errp);
}

static void tcp_accept_incoming_migration(void *opaque)
Expand Down
11 changes: 2 additions & 9 deletions migration-unix.c
Expand Up @@ -68,20 +68,13 @@ static void unix_wait_for_connect(int fd, void *opaque)
}
}

int unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp)
void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp)
{
Error *local_err = NULL;

s->get_error = unix_errno;
s->write = unix_write;
s->close = unix_close;

s->fd = unix_nonblocking_connect(path, unix_wait_for_connect, s, &local_err);
if (local_err != NULL) {
error_propagate(errp, local_err);
return -1;
}
return 0;
s->fd = unix_nonblocking_connect(path, unix_wait_for_connect, s, errp);
}

static void unix_accept_incoming_migration(void *opaque)
Expand Down
17 changes: 6 additions & 11 deletions migration.c
Expand Up @@ -487,7 +487,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
MigrationState *s = migrate_get_current();
MigrationParams params;
const char *p;
int ret;

params.blk = blk;
params.shared = inc;
Expand All @@ -509,27 +508,23 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
s = migrate_init(&params);

if (strstart(uri, "tcp:", &p)) {
ret = tcp_start_outgoing_migration(s, p, &local_err);
tcp_start_outgoing_migration(s, p, &local_err);
#if !defined(WIN32)
} else if (strstart(uri, "exec:", &p)) {
ret = exec_start_outgoing_migration(s, p);
exec_start_outgoing_migration(s, p, &local_err);
} else if (strstart(uri, "unix:", &p)) {
ret = unix_start_outgoing_migration(s, p, &local_err);
unix_start_outgoing_migration(s, p, &local_err);
} else if (strstart(uri, "fd:", &p)) {
ret = fd_start_outgoing_migration(s, p);
fd_start_outgoing_migration(s, p, &local_err);
#endif
} else {
error_set(errp, QERR_INVALID_PARAMETER_VALUE, "uri", "a valid migration protocol");
return;
}

if (ret < 0 || local_err) {
if (local_err) {
migrate_fd_error(s);
if (!local_err) {
error_set_errno(errp, -ret, QERR_UNDEFINED_ERROR);
} else {
error_propagate(errp, local_err);
}
error_propagate(errp, local_err);
return;
}

Expand Down
9 changes: 4 additions & 5 deletions migration.h
Expand Up @@ -59,20 +59,19 @@ void do_info_migrate(Monitor *mon, QObject **ret_data);

int exec_start_incoming_migration(const char *host_port);

int exec_start_outgoing_migration(MigrationState *s, const char *host_port);
void exec_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);

int tcp_start_incoming_migration(const char *host_port, Error **errp);

int tcp_start_outgoing_migration(MigrationState *s, const char *host_port,
Error **errp);
void tcp_start_outgoing_migration(MigrationState *s, const char *host_port, Error **errp);

int unix_start_incoming_migration(const char *path, Error **errp);

int unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp);
void unix_start_outgoing_migration(MigrationState *s, const char *path, Error **errp);

int fd_start_incoming_migration(const char *path);

int fd_start_outgoing_migration(MigrationState *s, const char *fdname);
void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp);

void migrate_fd_error(MigrationState *s);

Expand Down

0 comments on commit f37afb5

Please sign in to comment.