From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:55373) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPuvf-0007Bo-4E for qemu-devel@nongnu.org; Wed, 10 Apr 2013 09:19:12 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPuva-0003Uq-5F for qemu-devel@nongnu.org; Wed, 10 Apr 2013 09:19:10 -0400 Received: from mx1.redhat.com ([209.132.183.28]:32655) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPuvZ-0003So-Nh for qemu-devel@nongnu.org; Wed, 10 Apr 2013 09:19:06 -0400 Message-ID: <5165671E.6000606@redhat.com> Date: Wed, 10 Apr 2013 16:20:30 +0300 From: Orit Wasserman MIME-Version: 1.0 References: <1365568180-19593-1-git-send-email-mrhines@linux.vnet.ibm.com> <1365568180-19593-5-git-send-email-mrhines@linux.vnet.ibm.com> <51655F78.5040500@redhat.com> <51656597.1060601@linux.vnet.ibm.com> In-Reply-To: <51656597.1060601@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH RDMA support v6: 4/7] Introduce two new capabilities List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael R. Hines" Cc: aliguori@us.ibm.com, mst@redhat.com, qemu-devel@nongnu.org, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com, pbonzini@redhat.com On 04/10/2013 04:13 PM, Michael R. Hines wrote: > On 04/10/2013 08:47 AM, Orit Wasserman wrote: >> On 04/10/2013 07:29 AM, mrhines@linux.vnet.ibm.com wrote: >>> From: "Michael R. Hines" >>> >>> RDMA performs very slowly with zero-page checking. >>> Without the ability to disable it, RDMA throughput and >>> latency promises and high performance links cannot be >>> fully realized. >>> >>> On the other hand, dynamic page registration support is also >>> included in the RDMA protocol. This second capability also >>> cannot be fully realized without the ability to enable zero >>> page scanning. >>> >>> So, we have two new capabilities which work together: >>> >>> 1. migrate_set_capability check_for_zero on|off (default on) >> This is not a capability but a migration parameter, the command >> should be named migrate_check_for_zero (on|off) and you will need to add a flag >> to MigrationParams (and it should be in a separate patch as Paolo already requested) > > I'm happy to convert to using a command, but is this not consistent with > XBZRLE the capability? The XBZRLE capability does not affect the destination > either - but it is implemented as a capability. > > Am I misundertanding? > XBZRLE requires the destination QEMU to know how to decode it, if the destination is running on older QEMU than it won't be able to do so. Orit > - Michael > >>> 2. migrate_set_capability chunk_register_destination on|off (default off) >> This is indeed a capability as it effect both source and destination > > Acknowledged. > > void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size); > +void qemu_file_set_error(QEMUFile *f, int ret); > + > +void qemu_rdma_cleanup(void *opaque); > +int qemu_rdma_close(void *opaque); > +int qemu_rdma_get_fd(void *opaque); > +int qemu_rdma_get_buffer(void *opaque, uint8_t *buf, int64_t pos, int size); > +int qemu_rdma_put_buffer(void *opaque, const uint8_t *buf, > + int64_t pos, int size); > +bool qemu_file_mode_is_not_valid(const char * mode); > + > +extern const QEMUFileOps rdma_read_ops; > +extern const QEMUFileOps rdma_write_ops; > > static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v) > { > diff --git a/migration.c b/migration.c > index 3b4b467..875cee3 100644 > --- a/migration.c > +++ b/migration.c > @@ -66,6 +66,7 @@ MigrationState *migrate_get_current(void) > .state = MIG_STATE_SETUP, > .bandwidth_limit = MAX_THROTTLE, > .xbzrle_cache_size = DEFAULT_MIGRATE_CACHE_SIZE, > + .enabled_capabilities[MIGRATION_CAPABILITY_CHECK_FOR_ZERO] = true, > }; > > return ¤t_migration; > @@ -77,6 +78,10 @@ void qemu_start_incoming_migration(const char *uri, Error **errp) > > if (strstart(uri, "tcp:", &p)) > tcp_start_incoming_migration(p, errp); > +#ifdef CONFIG_RDMA > + else if (strstart(uri, "rdma:", &p)) > + rdma_start_incoming_migration(p, errp); > +#endif > #if !defined(WIN32) > else if (strstart(uri, "exec:", &p)) > exec_start_incoming_migration(p, errp); > @@ -120,8 +125,10 @@ void process_incoming_migration(QEMUFile *f) > Coroutine *co = qemu_coroutine_create(process_incoming_migration_co); > int fd = qemu_get_fd(f); > > - assert(fd != -1); > - qemu_set_nonblock(fd); > + if(fd != -2) { /* rdma returns -2 */ > + assert(fd != -1); > + qemu_set_nonblock(fd); > + } > qemu_coroutine_enter(co, f); > } > > @@ -405,6 +412,10 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk, > > if (strstart(uri, "tcp:", &p)) { > tcp_start_outgoing_migration(s, p, &local_err); > +#ifdef CONFIG_RDMA > + } else if (strstart(uri, "rdma:", &p)) { > + rdma_start_outgoing_migration(s, p, &local_err); > +#endif > #if !defined(WIN32) > } else if (strstart(uri, "exec:", &p)) { > exec_start_outgoing_migration(s, p, &local_err); > @@ -474,6 +485,24 @@ void qmp_migrate_set_downtime(double value, Error **errp) > max_downtime = (uint64_t)value; > } > > +bool migrate_chunk_register_destination(void) > +{ > + MigrationState *s; > + > + s = migrate_get_current(); > + > + return s->enabled_capabilities[MIGRATION_CAPABILITY_CHUNK_REGISTER_DESTINATION]; > +} > + > +bool migrate_check_for_zero(void) > +{ > + MigrationState *s; > + > + s = migrate_get_current(); > + > + return s->enabled_capabilities[MIGRATION_CAPABILITY_CHECK_FOR_ZERO]; > +} > + > int migrate_use_xbzrle(void) > { > MigrationState *s; > diff --git a/qapi-schema.json b/qapi-schema.json > index db542f6..7ebcf99 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -602,7 +602,7 @@ > # Since: 1.2 > ## > { 'enum': 'MigrationCapability', > - 'data': ['xbzrle'] } > + 'data': ['xbzrle', 'check_for_zero', 'chunk_register_destination'] } > > ## > # @MigrationCapabilityStatus > >