From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42540) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPuIG-0002Ar-Db for qemu-devel@nongnu.org; Wed, 10 Apr 2013 08:38:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UPuIC-00062p-M7 for qemu-devel@nongnu.org; Wed, 10 Apr 2013 08:38:28 -0400 Received: from e7.ny.us.ibm.com ([32.97.182.137]:49106) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UPuIC-00062h-Hu for qemu-devel@nongnu.org; Wed, 10 Apr 2013 08:38:24 -0400 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 10 Apr 2013 08:38:24 -0400 Received: from d01relay02.pok.ibm.com (d01relay02.pok.ibm.com [9.56.227.234]) by d01dlp01.pok.ibm.com (Postfix) with ESMTP id 592B038C8056 for ; Wed, 10 Apr 2013 08:38:20 -0400 (EDT) Received: from d01av02.pok.ibm.com (d01av02.pok.ibm.com [9.56.224.216]) by d01relay02.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r3ACcK0h313764 for ; Wed, 10 Apr 2013 08:38:20 -0400 Received: from d01av02.pok.ibm.com (loopback [127.0.0.1]) by d01av02.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r3ACcK8K012477 for ; Wed, 10 Apr 2013 09:38:20 -0300 Message-ID: <51655D3B.2040308@linux.vnet.ibm.com> Date: Wed, 10 Apr 2013 08:38:19 -0400 From: "Michael R. Hines" MIME-Version: 1.0 References: <1365568180-19593-1-git-send-email-mrhines@linux.vnet.ibm.com> <1365568180-19593-4-git-send-email-mrhines@linux.vnet.ibm.com> <51651A29.8030809@redhat.com> In-Reply-To: <51651A29.8030809@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH RDMA support v6: 3/7] Introduce QEMURamControlOps List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini 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 Acknowledged. On 04/10/2013 03:52 AM, Paolo Bonzini wrote: > Il 10/04/2013 06:29, mrhines@linux.vnet.ibm.com ha scritto: >> From: "Michael R. Hines" >> >> RDMA requires hooks before and after each iteration round >> in order to coordinate the new dynamic page registration support. >> This is done now by introducing a new set of function pointers >> which are only used by arch_init.c. >> >> Pointers include: >> 1. save_ram_page (which can be defined by anyone, not just RDMA) >> 2. hook after each iteration >> 3. hook before each iteration >> >> The pointers are then installed in savevm.c because they >> need visibility into QEMUFile. >> >> Now that we have a proper set of pointers, we no longer need >> specific checks anymore to determine whether or not RDMA >> is enabled. >> >> Signed-off-by: Michael R. Hines >> --- >> include/migration/migration.h | 52 +++++++++++++++++++++ >> savevm.c | 104 +++++++++++++++++++++++++++++++++++++---- >> 2 files changed, 146 insertions(+), 10 deletions(-) >> >> diff --git a/include/migration/migration.h b/include/migration/migration.h >> index e2acec6..0287321 100644 >> --- a/include/migration/migration.h >> +++ b/include/migration/migration.h >> @@ -21,6 +21,7 @@ >> #include "qapi/error.h" >> #include "migration/vmstate.h" >> #include "qapi-types.h" >> +#include "exec/cpu-common.h" >> >> struct MigrationParams { >> bool blk; >> @@ -75,6 +76,10 @@ void fd_start_incoming_migration(const char *path, Error **errp); >> >> void fd_start_outgoing_migration(MigrationState *s, const char *fdname, Error **errp); >> >> +void rdma_start_outgoing_migration(void *opaque, const char *host_port, Error **errp); >> + >> +void rdma_start_incoming_migration(const char * host_port, Error **errp); >> + >> void migrate_fd_error(MigrationState *s); >> >> void migrate_fd_connect(MigrationState *s); >> @@ -127,4 +132,51 @@ int migrate_use_xbzrle(void); >> int64_t migrate_xbzrle_cache_size(void); >> >> int64_t xbzrle_cache_resize(int64_t new_size); >> + >> +bool migrate_check_for_zero(void); >> +bool migrate_chunk_register_destination(void); >> + >> +/* >> + * Hooks before and after each iteration round to perform special functions. >> + * In the case of RDMA, this is to handle dynamic server registration. >> + */ >> +#define RAM_CONTROL_SETUP 0 >> +#define RAM_CONTROL_ROUND 1 >> +#define RAM_CONTROL_REGISTER 2 >> +#define RAM_CONTROL_FINISH 3 >> + >> +typedef void (RAMFunc)(QEMUFile *f, void *opaque, int section); >> + >> +struct QEMURamControlOps { >> + RAMFunc *before_ram_iterate; >> + RAMFunc *after_ram_iterate; >> + RAMFunc *register_ram_iterate; >> + size_t (*save_page)(QEMUFile *f, >> + void *opaque, ram_addr_t block_offset, >> + ram_addr_t offset, int cont, size_t size, >> + bool zero); >> +}; >> + >> +const QEMURamControlOps *qemu_savevm_get_control(QEMUFile *f); >> + >> +void ram_control_before_iterate(QEMUFile *f, int section); >> +void ram_control_after_iterate(QEMUFile *f, int section); >> +void ram_control_register_iterate(QEMUFile *f, int section); >> +size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, >> + ram_addr_t offset, int cont, >> + size_t size, bool zero); >> + >> +#ifdef CONFIG_RDMA >> +extern const QEMURamControlOps qemu_rdma_control; >> + >> +size_t qemu_rdma_save_page(QEMUFile *f, void *opaque, >> + ram_addr_t block_offset, >> + ram_addr_t offset, int cont, >> + size_t size, bool zero); >> + >> +void qemu_rdma_registration_stop(QEMUFile *f, void *opaque, int section); >> +void qemu_rdma_registration_handle(QEMUFile *f, void *opaque, int section); >> +void qemu_ram_registration_start(QEMUFile *f, void *opaque, int section); >> +#endif >> + >> #endif >> diff --git a/savevm.c b/savevm.c >> index b1d8988..26eabb3 100644 >> --- a/savevm.c >> +++ b/savevm.c >> @@ -409,16 +409,24 @@ static const QEMUFileOps socket_write_ops = { >> .close = socket_close >> }; >> >> -QEMUFile *qemu_fopen_socket(int fd, const char *mode) >> +bool qemu_file_mode_is_not_valid(const char * mode) >> { >> - QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket)); >> - >> if (mode == NULL || >> (mode[0] != 'r' && mode[0] != 'w') || >> mode[1] != 'b' || mode[2] != 0) { >> fprintf(stderr, "qemu_fopen: Argument validity check failed\n"); >> - return NULL; >> + return true; >> } >> + >> + return false; >> +} >> + >> +QEMUFile *qemu_fopen_socket(int fd, const char *mode) >> +{ >> + QEMUFileSocket *s = g_malloc0(sizeof(QEMUFileSocket)); >> + >> + if(qemu_file_mode_is_not_valid(mode)) >> + return NULL; >> >> s->fd = fd; >> if (mode[0] == 'w') { >> @@ -430,16 +438,44 @@ QEMUFile *qemu_fopen_socket(int fd, const char *mode) >> return s->file; >> } >> >> +#ifdef CONFIG_RDMA >> +const QEMURamControlOps qemu_rdma_write_control = { >> + .before_ram_iterate = qemu_ram_registration_start, >> + .after_ram_iterate = qemu_rdma_registration_stop, >> + .register_ram_iterate = qemu_rdma_registration_handle, >> + .save_page = qemu_rdma_save_page, >> +}; >> + >> +const QEMURamControlOps qemu_rdma_read_control = { >> + .register_ram_iterate = qemu_rdma_registration_handle, >> +}; >> + >> +const QEMUFileOps rdma_read_ops = { >> + .get_buffer = qemu_rdma_get_buffer, >> + .close = qemu_rdma_close, >> + .get_fd = qemu_rdma_get_fd, >> + .ram_control = &qemu_rdma_read_control, >> +}; >> + >> +const QEMUFileOps rdma_write_ops = { >> + .put_buffer = qemu_rdma_put_buffer, >> + .close = qemu_rdma_close, >> + .get_fd = qemu_rdma_get_fd, >> + .ram_control = &qemu_rdma_write_control, >> +}; >> +#endif >> + >> +const QEMURamControlOps *qemu_savevm_get_control(QEMUFile *f) >> +{ >> + return f->ops->ram_control; >> +} > This function is not needed (BTW, again you do not need the new struct). > >> QEMUFile *qemu_fopen(const char *filename, const char *mode) >> { >> QEMUFileStdio *s; >> >> - if (mode == NULL || >> - (mode[0] != 'r' && mode[0] != 'w') || >> - mode[1] != 'b' || mode[2] != 0) { >> - fprintf(stderr, "qemu_fopen: Argument validity check failed\n"); >> + if(qemu_file_mode_is_not_valid(mode)) >> return NULL; >> - } >> >> s = g_malloc0(sizeof(QEMUFileStdio)); >> >> @@ -509,7 +545,7 @@ int qemu_file_get_error(QEMUFile *f) >> return f->last_error; >> } >> >> -static void qemu_file_set_error(QEMUFile *f, int ret) >> +void qemu_file_set_error(QEMUFile *f, int ret) >> { >> if (f->last_error == 0) { >> f->last_error = ret; >> @@ -554,6 +590,54 @@ static void qemu_fflush(QEMUFile *f) >> } >> } >> >> +void ram_control_before_iterate(QEMUFile *f, int section) >> +{ >> + const QEMURamControlOps * control = qemu_savevm_get_control(f); >> + >> + if(control && control->before_ram_iterate) { >> + qemu_fflush(f); >> + control->before_ram_iterate(f, f->opaque, section); > Please make these return an int, and set the error here. You do not > need to make qemu_file_set_error public. > >> + } >> +} >> + >> +void ram_control_after_iterate(QEMUFile *f, int section) >> +{ >> + const QEMURamControlOps * control = qemu_savevm_get_control(f); >> + >> + if(control && control->after_ram_iterate) { >> + qemu_fflush(f); >> + control->after_ram_iterate(f, f->opaque, section); >> + } >> +} >> + >> +void ram_control_register_iterate(QEMUFile *f, int section) >> +{ >> + const QEMURamControlOps * control = qemu_savevm_get_control(f); >> + >> + if(control && control->register_ram_iterate) { >> + qemu_fflush(f); >> + control->register_ram_iterate(f, f->opaque, section); >> + } >> +} >> + >> +size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, >> + ram_addr_t offset, int cont, >> + size_t size, bool zero) >> +{ >> + const QEMURamControlOps * control = qemu_savevm_get_control(f); >> + >> + if(control && control->save_page) { >> + size_t bytes; >> + qemu_fflush(f); >> + bytes = control->save_page(f, f->opaque, block_offset, offset, cont, size, zero); >> + if(bytes > 0) >> + f->pos += bytes; >> + return bytes; >> + } >> + >> + return -ENOTSUP; >> +} >> + >> static void qemu_fill_buffer(QEMUFile *f) >> { >> int len; >>