On 02/12/2013 12:49 AM, Michael R. Hines wrote:
> From: "Michael R. Hines" <mrhi...@us.ibm.com>
>
>
> Signed-off-by: Michael R. Hines <mrhi...@us.ibm.com>
> ---
> migration-tcp.c | 19 +++++++++++++++++++
> migration.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 71 insertions(+), 1 deletion(-)
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index e78a296..113576e 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -14,6 +14,7 @@
> */
>
> #include "qemu-common.h"
> +#include "qemu/rdma.h"
> #include "qemu/sockets.h"
> #include "migration/migration.h"
> #include "migration/qemu-file.h"
> @@ -55,6 +56,9 @@ static void tcp_wait_for_connect(int fd, void *opaque)
>
> if (fd < 0) {
> DPRINTF("migrate connect error\n");
> + if (migration_use_rdma()) {
> + rdma_cleanup(&rdma_mdata);
> + }
Those should be moved to migration-rdma.c
Are you still using the tcp for transferring device state?
If so you can call the tcp functions from the migration rdma code as a first
step
but I would prefer it to use RDMA too.
> s->fd = -1;
> migrate_fd_error(s);
> } else {
> @@ -62,6 +66,10 @@ static void tcp_wait_for_connect(int fd, void *opaque)
> s->fd = fd;
> socket_set_block(s->fd);
> migrate_fd_connect(s);
> +
> + if (migration_use_rdma() && rdma_wait_for_connect(fd, opaque)) {
> + migrate_fd_error(s);
> + }
See above
> }
> }
>
> @@ -101,6 +109,12 @@ static void tcp_accept_incoming_migration(void *opaque)
> goto out;
> }
>
> + if (migration_use_rdma() &&
> + rdma_accept_incoming_migration(&rdma_mdata)) {
> + close(s);
> + return;
> + }
> +
See above
> process_incoming_migration(f);
> return;
>
> @@ -117,6 +131,11 @@ void tcp_start_incoming_migration(const char *host_port,
> Error **errp)
> return;
> }
>
> + if (migration_use_rdma() && rdma_start_incoming_migration(s)) {
> + close(s);
> + return;
> + }
> +
See above
> qemu_set_fd_handler2(s, NULL, tcp_accept_incoming_migration, NULL,
> (void *)(intptr_t)s);
> }
> diff --git a/migration.c b/migration.c
> index 77c1971..5ef923d 100644
> --- a/migration.c
> +++ b/migration.c
> @@ -22,6 +22,7 @@
> #include "qemu/sockets.h"
> #include "migration/block.h"
> #include "qemu/thread.h"
> +#include "qemu/rdma.h"
> #include "qmp-commands.h"
>
> //#define DEBUG_MIGRATION
> @@ -262,6 +263,16 @@ void
> qmp_migrate_set_capabilities(MigrationCapabilityStatusList *params,
> }
>
> for (cap = params; cap; cap = cap->next) {
> + if (cap->value->capability == MIGRATION_CAPABILITY_RDMA) {
> +#ifndef CONFIG_RDMA
> + error_set(errp, QERR_MIGRATION_RDMA_NOT_ENABLED);
> + continue;
> +#endif
> + if (!migration_use_rdma()) {
> + error_set(errp, QERR_MIGRATION_RDMA_NOT_CONFIGURED,
> rdma_mdata.host, rdma_mdata.port);
> + continue;
> + }
> + }
I would let the user to enable the RDMA capability first than set the RDMA
host/port.
> s->enabled_capabilities[cap->value->capability] = cap->value->state;
> }
> }
> @@ -279,6 +290,11 @@ static int migrate_fd_cleanup(MigrationState *s)
> }
>
> assert(s->fd == -1);
> +
> + if (migrate_rdma_enabled()) {
> + rdma_cleanup(&rdma_mdata);
> + }
> +
> return ret;
> }
>
> @@ -386,6 +402,9 @@ static MigrationState *migrate_init(const MigrationParams
> *params)
> s->params = *params;
> memcpy(s->enabled_capabilities, enabled_capabilities,
> sizeof(enabled_capabilities));
> +
> + rdma_update_capability(s);
Why? the user/management needs to enable the capability manually.
The fact that you can set the capability means this QEMU version supports it.
> +
> s->xbzrle_cache_size = xbzrle_cache_size;
>
> s->bandwidth_limit = bandwidth_limit;
> @@ -481,6 +500,29 @@ int64_t qmp_query_migrate_cache_size(Error **errp)
> return migrate_xbzrle_cache_size();
> }
>
> +void qmp_migrate_set_rdma_port(int64_t port, Error **errp)
> +{
> + MigrationState *s = migrate_get_current();
> + if (s && (s->state == MIG_STATE_ACTIVE)) {
> + return;
> + }
> + DPRINTF("rdma migration port: %" PRId64 "\n", port);
> + rdma_mdata.port = port;
> + rdma_update_capability(s);
I would allow setting the port only if the capability is set.
> +}
> +
> +void qmp_migrate_set_rdma_host(const char *host, Error **errp)
> +{
> + MigrationState *s = migrate_get_current();
> + if (s && (s->state == MIG_STATE_ACTIVE)) {
> + return;
> + }
> + DPRINTF("rdma migration host name: %s\n", host);
> + strncpy(rdma_mdata.host, host, 64);
> + rdma_mdata.host[63] = '\0';
> + rdma_update_capability(s);
see above.
> +}
> +
> void qmp_migrate_set_speed(int64_t value, Error **errp)
> {
> MigrationState *s;
> @@ -505,6 +547,15 @@ int migrate_use_xbzrle(void)
> {
> MigrationState *s;
>
> + /*
> + * RFC RDMA: time(run-length encoding) +
> + * time(communication) is too big. RDMA throughput tanks
> + * when this feature is enabled. But there's no need
> + * to change the code since the feature is optional.
> + */
> + if (migrate_rdma_enabled())
> + return 0;
> +
If you won't allow enabling one capability when the other is active,
you won't need this check.
Regards,
Orit
> s = migrate_get_current();
>
> return s->enabled_capabilities[MIGRATION_CAPABILITY_XBZRLE];
> @@ -571,7 +622,7 @@ static int buffered_put_buffer(void *opaque, const
> uint8_t *buf,
> }
>
> if (size > (s->buffer_capacity - s->buffer_size)) {
> - DPRINTF("increasing buffer capacity from %zu by %zu\n",
> + DPRINTF("increasing buffer capacity from %zu by %d\n",
> s->buffer_capacity, size + 1024);
>
> s->buffer_capacity += size + 1024;
>
