From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:51292) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHVjL-0003KK-D8 for qemu-devel@nongnu.org; Mon, 18 Mar 2013 04:47:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UHVjJ-0003B1-Fm for qemu-devel@nongnu.org; Mon, 18 Mar 2013 04:47:43 -0400 Received: from mail-bk0-x233.google.com ([2a00:1450:4008:c01::233]:50934) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UHVjJ-0003Ai-4S for qemu-devel@nongnu.org; Mon, 18 Mar 2013 04:47:41 -0400 Received: by mail-bk0-f51.google.com with SMTP id ik5so2401919bkc.10 for ; Mon, 18 Mar 2013 01:47:40 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <5146D4A8.7040500@redhat.com> Date: Mon, 18 Mar 2013 09:47:36 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1363576743-6146-1-git-send-email-mrhines@linux.vnet.ibm.com> <1363576743-6146-10-git-send-email-mrhines@linux.vnet.ibm.com> In-Reply-To: <1363576743-6146-10-git-send-email-mrhines@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH RDMA support v4: 09/10] check for QMP string and bypass nonblock() calls List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: mrhines@linux.vnet.ibm.com Cc: aliguori@us.ibm.com, mst@redhat.com, qemu-devel@nongnu.org, owasserm@redhat.com, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com Il 18/03/2013 04:19, mrhines@linux.vnet.ibm.com ha scritto: > From: "Michael R. Hines" > > Since we're not using TCP anymore, we skip these calls. > > Also print a little extra text while debugging, like "gbps" > which is helpful to know how the link is being utilized. > > Signed-off-by: Michael R. Hines > --- > include/migration/migration.h | 3 +++ > migration.c | 19 +++++++++++++------ > 2 files changed, 16 insertions(+), 6 deletions(-) > > diff --git a/include/migration/migration.h b/include/migration/migration.h > index bb617fd..88ab5f6 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -20,6 +20,7 @@ > #include "qemu/notify.h" > #include "qapi/error.h" > #include "migration/vmstate.h" > +#include "migration/rdma.h" > #include "qapi-types.h" > > struct MigrationParams { > @@ -102,6 +103,7 @@ uint64_t xbzrle_mig_bytes_transferred(void); > uint64_t xbzrle_mig_pages_transferred(void); > uint64_t xbzrle_mig_pages_overflow(void); > uint64_t xbzrle_mig_pages_cache_miss(void); > +uint64_t delta_norm_mig_bytes_transferred(void); Please add the protocol under the > > /** > * @migrate_add_blocker - prevent migration from proceeding > @@ -122,6 +124,7 @@ int xbzrle_encode_buffer(uint8_t *old_buf, uint8_t *new_buf, int slen, > int xbzrle_decode_buffer(uint8_t *src, int slen, uint8_t *dst, int dlen); > > int migrate_use_xbzrle(void); > +void *migrate_use_rdma(QEMUFile *f); Perhaps you can add a new function to QEMUFile send_page? And if it returns -ENOTSUP, proceed with the normal is_dup_page + put_buffer. I wonder if that lets use remove migrate_use_rdma() completely. Also, if QEMUFileRDMA is moved to rdma.c, the number of public and stubbed functions should decrease noticeably. There is a patch on the list to move QEMUFile to its own source file. You could incorporate it in your series. > int64_t migrate_xbzrle_cache_size(void); > > int64_t xbzrle_cache_resize(int64_t new_size); > diff --git a/migration.c b/migration.c > index 185d112..634437a 100644 > --- a/migration.c > +++ b/migration.c > @@ -15,6 +15,7 @@ > > #include "qemu-common.h" > #include "migration/migration.h" > +#include "migration/rdma.h" > #include "monitor/monitor.h" > #include "migration/qemu-file.h" > #include "sysemu/sysemu.h" > @@ -77,6 +78,8 @@ void qemu_start_incoming_migration(const char *uri, Error **errp) > > if (strstart(uri, "tcp:", &p)) > tcp_start_incoming_migration(p, errp); > + else if (strstart(uri, "rdma:", &p)) > + rdma_start_incoming_migration(p, errp); > #if !defined(WIN32) > else if (strstart(uri, "exec:", &p)) > exec_start_incoming_migration(p, errp); > @@ -118,10 +121,11 @@ static void process_incoming_migration_co(void *opaque) > 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); > + if(!migrate_use_rdma(f)) { > + int fd = qemu_get_fd(f); > + assert(fd != -1); > + socket_set_nonblock(fd); Is this because qemu_get_fd(f) returns -1 for RDMA? Then, you can instead put socket_set_nonblock under an if(fd != -1). > + } > qemu_coroutine_enter(co, f); > } > > @@ -404,6 +408,8 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > > if (strstart(uri, "tcp:", &p)) { > tcp_start_outgoing_migration(s, p, &local_err); > + } else if (strstart(uri, "rdma:", &p)) { > + rdma_start_outgoing_migration(s, p, &local_err); > #if !defined(WIN32) > } else if (strstart(uri, "exec:", &p)) { > exec_start_outgoing_migration(s, p, &local_err); > @@ -545,8 +551,9 @@ static void *migration_thread(void *opaque) > max_size = bandwidth * migrate_max_downtime() / 1000000; > > DPRINTF("transferred %" PRIu64 " time_spent %" PRIu64 > - " bandwidth %g max_size %" PRId64 "\n", > - transferred_bytes, time_spent, bandwidth, max_size); > + " bandwidth %g (%0.2f mbps) max_size %" PRId64 "\n", > + transferred_bytes, time_spent, > + bandwidth, Gbps(transferred_bytes, time_spent), max_size); > /* if we haven't sent anything, we don't want to recalculate > 10000 is a small enough number for our purposes */ > if (s->dirty_bytes_rate && transferred_bytes > 10000) { > Otherwise looks good.