From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:49469) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USPbI-00070m-VK for qemu-devel@nongnu.org; Wed, 17 Apr 2013 06:28:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1USPbH-0000DL-Hf for qemu-devel@nongnu.org; Wed, 17 Apr 2013 06:28:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58350) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1USPbH-0000DE-9s for qemu-devel@nongnu.org; Wed, 17 Apr 2013 06:28:27 -0400 Message-ID: <516E79A5.2050207@redhat.com> Date: Wed, 17 Apr 2013 13:29:57 +0300 From: Orit Wasserman MIME-Version: 1.0 References: <1366172418-8729-1-git-send-email-mrhines@linux.vnet.ibm.com> <1366172418-8729-3-git-send-email-mrhines@linux.vnet.ibm.com> <516E75C5.4070801@redhat.com> <516E76C4.60104@redhat.com> In-Reply-To: <516E76C4.60104@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PULL v3 2/7] rdma: new QEMUFileOps hooks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Paolo Bonzini Cc: aliguori@us.ibm.com, mst@redhat.com, quintela@redhat.com, qemu-devel@nongnu.org, mrhines@linux.vnet.ibm.com, abali@us.ibm.com, mrhines@us.ibm.com, gokul@us.ibm.com On 04/17/2013 01:17 PM, Paolo Bonzini wrote: > Il 17/04/2013 12:13, Orit Wasserman ha scritto: >> On 04/17/2013 07:20 AM, mrhines@linux.vnet.ibm.com wrote: >>> From: "Michael R. Hines" >>> >>> These are the prototypes and implementation of new hooks that >>> RDMA takes advantage of to perform dynamic page registration. >>> >>> An optional hook is also introduced for a custom function >>> to be able to override the default save_page function. >>> >>> Also included are the prototypes and accessor methods used by >>> arch_init.c which invoke funtions inside savevm.c to call out >>> to the hooks that may or may not have been overridden >>> inside of QEMUFileOps. >>> >>> Signed-off-by: Michael R. Hines >>> --- >>> include/migration/migration.h | 19 ++++++++++ >>> include/migration/qemu-file.h | 31 ++++++++++++++++ >>> savevm.c | 79 ++++++++++++++++++++++++++++++++++++----- >>> 3 files changed, 121 insertions(+), 8 deletions(-) >>> >>> diff --git a/include/migration/migration.h b/include/migration/migration.h >>> index e2acec6..8e02391 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" >> >> Is this include needed here? > > I think it's for ram_addr_t. > > Paolo > Yes I missed that. >>> >>> struct MigrationParams { >>> bool blk; >>> @@ -127,4 +128,22 @@ int migrate_use_xbzrle(void); >>> int64_t migrate_xbzrle_cache_size(void); >>> >>> int64_t xbzrle_cache_resize(int64_t new_size); >>> + >>> +void ram_handle_compressed(void *host, uint8_t ch, uint64_t size); >>> + >>> +bool migrate_chunk_register_destination(void); >>> +void ram_control_before_iterate(QEMUFile *f, uint64_t flags); >>> +void ram_control_after_iterate(QEMUFile *f, uint64_t flags); >>> +void ram_control_load_hook(QEMUFile *f, uint64_t flags); >>> + >>> +/* Whenever this is found in the data stream, the flags >>> + * will be passed to ram_control_load_hook in the incoming-migration >>> + * side. This lets before_ram_iterate/after_ram_iterate add >>> + * transport-specific sections to the RAM migration data. >>> + */ >>> +#define RAM_SAVE_FLAG_HOOK 0x80 >>> + >>> +size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, >>> + ram_addr_t offset, size_t size); >>> + >>> #endif >>> diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h >>> index 7519464..5166a42 100644 >>> --- a/include/migration/qemu-file.h >>> +++ b/include/migration/qemu-file.h >>> @@ -23,6 +23,7 @@ >>> */ >>> #ifndef QEMU_FILE_H >>> #define QEMU_FILE_H 1 >>> +#include "exec/cpu-common.h" >>> >>> /* This function writes a chunk of data to a file at the given position. >>> * The pos argument can be ignored if the file is only being used for >>> @@ -57,12 +58,39 @@ typedef int (QEMUFileGetFD)(void *opaque); >>> typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov, >>> int iovcnt, int64_t pos); >>> >>> +/* >>> + * This function provides hooks around different >>> + * stages of RAM migration. >>> + */ >>> +typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags); >>> + >>> +/* >>> + * Constants used by ram_control_* hooks >>> + */ >>> +#define RAM_CONTROL_SETUP 0 >>> +#define RAM_CONTROL_ROUND 1 >>> +#define RAM_CONTROL_HOOK 2 >>> +#define RAM_CONTROL_FINISH 3 >>> + >>> +/* >>> + * This function allows override of where the RAM page >>> + * is saved (such as RDMA, for example.) >>> + */ >>> +typedef size_t (QEMURamSaveFunc)(QEMUFile *f, void *opaque, >>> + ram_addr_t block_offset, >>> + ram_addr_t offset, >>> + size_t size); >>> + >>> typedef struct QEMUFileOps { >>> QEMUFilePutBufferFunc *put_buffer; >>> QEMUFileGetBufferFunc *get_buffer; >>> QEMUFileCloseFunc *close; >>> QEMUFileGetFD *get_fd; >>> QEMUFileWritevBufferFunc *writev_buffer; >>> + QEMURamHookFunc *before_ram_iterate; >>> + QEMURamHookFunc *after_ram_iterate; >>> + QEMURamHookFunc *hook_ram_load; >>> + QEMURamSaveFunc *save_page; >>> } QEMUFileOps; >>> >>> QEMUFile *qemu_fopen_ops(void *opaque, const QEMUFileOps *ops); >>> @@ -81,6 +109,8 @@ void qemu_put_byte(QEMUFile *f, int v); >>> */ >>> void qemu_put_buffer_async(QEMUFile *f, const uint8_t *buf, int size); >>> >>> +bool qemu_file_mode_is_not_valid(const char *mode); >>> + >> >> This should be in a separate patch as it not related to the new hooks >> you can add a new patch for adding qemu_file_mode_is_invalid func >> >>> static inline void qemu_put_ubyte(QEMUFile *f, unsigned int v) >>> { >>> qemu_put_byte(f, (int)v); >>> @@ -110,6 +140,7 @@ void qemu_file_reset_rate_limit(QEMUFile *f); >>> void qemu_file_set_rate_limit(QEMUFile *f, int64_t new_rate); >>> int64_t qemu_file_get_rate_limit(QEMUFile *f); >>> int qemu_file_get_error(QEMUFile *f); >>> +void qemu_fflush(QEMUFile *f); >> >> This also should not be in this patch but in a separate patch >> (probably didn't read the rest of the patches yet) >> >> Orit >> >>> >>> static inline void qemu_put_be64s(QEMUFile *f, const uint64_t *pv) >>> { >>> diff --git a/savevm.c b/savevm.c >>> index 53515cb..cdb1690 100644 >>> --- a/savevm.c >>> +++ b/savevm.c >>> @@ -409,14 +409,23 @@ 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 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; >>> } >>> >>> @@ -434,10 +443,7 @@ 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; >>> } >>> >>> @@ -542,7 +548,7 @@ static inline bool qemu_file_is_writable(QEMUFile *f) >>> * If there is writev_buffer QEMUFileOps it uses it otherwise uses >>> * put_buffer ops. >>> */ >>> -static void qemu_fflush(QEMUFile *f) >>> +void qemu_fflush(QEMUFile *f) >>> { >>> ssize_t ret = 0; >>> >>> @@ -569,6 +575,63 @@ static void qemu_fflush(QEMUFile *f) >>> } >>> } >>> >>> +void ram_control_before_iterate(QEMUFile *f, uint64_t flags) >>> +{ >>> + int ret = 0; >>> + >>> + if (f->ops->before_ram_iterate) { >>> + ret = f->ops->before_ram_iterate(f, f->opaque, flags); >>> + if (ret < 0) { >>> + qemu_file_set_error(f, ret); >>> + } >>> + } >>> +} >>> + >>> +void ram_control_after_iterate(QEMUFile *f, uint64_t flags) >>> +{ >>> + int ret = 0; >>> + >>> + if (f->ops->after_ram_iterate) { >>> + ret = f->ops->after_ram_iterate(f, f->opaque, flags); >>> + if (ret < 0) { >>> + qemu_file_set_error(f, ret); >>> + } >>> + } >>> +} >>> + >>> +void ram_control_load_hook(QEMUFile *f, uint64_t flags) >>> +{ >>> + int ret = 0; >>> + >>> + if (f->ops->hook_ram_load) { >>> + ret = f->ops->hook_ram_load(f, f->opaque, flags); >>> + if (ret < 0) { >>> + qemu_file_set_error(f, ret); >>> + } >>> + } else { >>> + qemu_file_set_error(f, ret); >>> + } >>> +} >>> + >>> +size_t ram_control_save_page(QEMUFile *f, ram_addr_t block_offset, >>> + ram_addr_t offset, size_t size) >>> +{ >>> + if (f->ops->save_page) { >>> + int64_t bytes; >>> + bytes = f->ops->save_page(f, f->opaque, block_offset, offset, size); >>> + >>> + if (bytes >= 0) { >>> + f->pos += bytes; >>> + } else { >>> + qemu_file_set_error(f, bytes); >>> + } >>> + >>> + return bytes; >>> + } >>> + >>> + return -1; >>> +} >>> + >>> static void qemu_fill_buffer(QEMUFile *f) >>> { >>> int len; >>> >> >