From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46639) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z376D-00012y-Me for qemu-devel@nongnu.org; Thu, 11 Jun 2015 14:21:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z376A-0005fV-Eh for qemu-devel@nongnu.org; Thu, 11 Jun 2015 14:21:09 -0400 Received: from e17.ny.us.ibm.com ([129.33.205.207]:36452) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z376A-0005eJ-9n for qemu-devel@nongnu.org; Thu, 11 Jun 2015 14:21:06 -0400 Received: from /spool/local by e17.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 11 Jun 2015 14:21:06 -0400 Received: from b01cxnp22036.gho.pok.ibm.com (b01cxnp22036.gho.pok.ibm.com [9.57.198.26]) by d01dlp02.pok.ibm.com (Postfix) with ESMTP id 5DDD26E8040 for ; Thu, 11 Jun 2015 14:12:50 -0400 (EDT) Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by b01cxnp22036.gho.pok.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t5BIL3va53674212 for ; Thu, 11 Jun 2015 18:21:03 GMT Received: from d01av01.pok.ibm.com (localhost [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t5BIL2fw009460 for ; Thu, 11 Jun 2015 14:21:02 -0400 Message-ID: <5579D168.1080004@linux.vnet.ibm.com> Date: Thu, 11 Jun 2015 13:20:24 -0500 From: "Michael R. Hines" MIME-Version: 1.0 References: <1434043048-4444-1-git-send-email-dgilbert@redhat.com> <1434043048-4444-8-git-send-email-dgilbert@redhat.com> In-Reply-To: <1434043048-4444-8-git-send-email-dgilbert@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Dr. David Alan Gilbert (git)" , qemu-devel@nongnu.org Cc: amit.shah@redhat.com, arei.gonglei@huawei.com, mrhines@us.ibm.com, quintela@redhat.com On 06/11/2015 12:17 PM, Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" > > We need the names of RAMBlocks as they're loaded for RDMA, > reuse a slightly modified ram_control_load_hook: > a) Pass a 'data' parameter to use for the name in the block-reg > case > b) Only some hook types now require the presence of a hook function. > > Signed-off-by: Dr. David Alan Gilbert > --- > arch_init.c | 4 +++- > include/migration/migration.h | 2 +- > include/migration/qemu-file.h | 14 +++++++++----- > migration/qemu-file.c | 16 +++++++++++----- > migration/rdma.c | 28 ++++++++++++++++++++++------ > trace-events | 2 +- > 6 files changed, 47 insertions(+), 19 deletions(-) > > diff --git a/arch_init.c b/arch_init.c > index d294474..dc9cc7e 100644 > --- a/arch_init.c > +++ b/arch_init.c > @@ -1569,6 +1569,8 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) > error_report_err(local_err); > } > } > + ram_control_load_hook(f, RAM_CONTROL_BLOCK_REG, > + block->idstr); > break; > } > } > @@ -1637,7 +1639,7 @@ static int ram_load(QEMUFile *f, void *opaque, int version_id) > break; > default: > if (flags & RAM_SAVE_FLAG_HOOK) { > - ram_control_load_hook(f, flags); > + ram_control_load_hook(f, RAM_CONTROL_HOOK, NULL); > } else { > error_report("Unknown combination of migration flags: %#x", > flags); > diff --git a/include/migration/migration.h b/include/migration/migration.h > index a6e025a..096e1ea 100644 > --- a/include/migration/migration.h > +++ b/include/migration/migration.h > @@ -164,7 +164,7 @@ int migrate_decompress_threads(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); > +void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data); > > /* Whenever this is found in the data stream, the flags > * will be passed to ram_control_load_hook in the incoming-migration > diff --git a/include/migration/qemu-file.h b/include/migration/qemu-file.h > index a01c5b8..7aafe19 100644 > --- a/include/migration/qemu-file.h > +++ b/include/migration/qemu-file.h > @@ -63,16 +63,20 @@ typedef ssize_t (QEMUFileWritevBufferFunc)(void *opaque, struct iovec *iov, > /* > * This function provides hooks around different > * stages of RAM migration. > + * 'opaque' is the backend specific data in QEMUFile > + * 'data' is call specific data associated with the 'flags' value > */ > -typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags); > +typedef int (QEMURamHookFunc)(QEMUFile *f, void *opaque, uint64_t flags, > + void *data); > > /* > * 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 > +#define RAM_CONTROL_SETUP 0 > +#define RAM_CONTROL_ROUND 1 > +#define RAM_CONTROL_HOOK 2 > +#define RAM_CONTROL_FINISH 3 > +#define RAM_CONTROL_BLOCK_REG 4 > > /* > * This function allows override of where the RAM page > diff --git a/migration/qemu-file.c b/migration/qemu-file.c > index 2750365..5493977 100644 > --- a/migration/qemu-file.c > +++ b/migration/qemu-file.c > @@ -128,7 +128,7 @@ 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); > + ret = f->ops->before_ram_iterate(f, f->opaque, flags, NULL); > if (ret < 0) { > qemu_file_set_error(f, ret); > } > @@ -140,24 +140,30 @@ 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); > + ret = f->ops->after_ram_iterate(f, f->opaque, flags, NULL); > if (ret < 0) { > qemu_file_set_error(f, ret); > } > } > } > > -void ram_control_load_hook(QEMUFile *f, uint64_t flags) > +void ram_control_load_hook(QEMUFile *f, uint64_t flags, void *data) > { > int ret = -EINVAL; > > if (f->ops->hook_ram_load) { > - ret = f->ops->hook_ram_load(f, f->opaque, flags); > + ret = f->ops->hook_ram_load(f, f->opaque, flags, data); > if (ret < 0) { > qemu_file_set_error(f, ret); > } > } else { > - qemu_file_set_error(f, ret); > + /* > + * Hook is a hook specifically requested by the source sending a flag > + * that expects there to be a hook on the destination. > + */ > + if (flags == RAM_CONTROL_HOOK) { > + qemu_file_set_error(f, ret); > + } > } > } > > diff --git a/migration/rdma.c b/migration/rdma.c > index cb66721..396329c 100644 > --- a/migration/rdma.c > +++ b/migration/rdma.c > @@ -2905,8 +2905,7 @@ err_rdma_dest_wait: > * > * Keep doing this until the source tells us to stop. > */ > -static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque, > - uint64_t flags) > +static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque) > { > RDMAControlHeader reg_resp = { .len = sizeof(RDMARegisterResult), > .type = RDMA_CONTROL_REGISTER_RESULT, > @@ -2936,7 +2935,7 @@ static int qemu_rdma_registration_handle(QEMUFile *f, void *opaque, > CHECK_ERROR_STATE(); > > do { > - trace_qemu_rdma_registration_handle_wait(flags); > + trace_qemu_rdma_registration_handle_wait(); > > ret = qemu_rdma_exchange_recv(rdma, &head, RDMA_CONTROL_NONE); > > @@ -3124,8 +3123,25 @@ out: > return ret; > } > > +static int rdma_load_hook(QEMUFile *f, void *opaque, uint64_t flags, void *data) > +{ > + switch (flags) { > + case RAM_CONTROL_BLOCK_REG: > + /* TODO A later patch */ > + return 0; > + break; > + > + case RAM_CONTROL_HOOK: > + return qemu_rdma_registration_handle(f, opaque); > + > + default: > + /* Shouldn't be called with any other values */ > + abort(); > + } > +} > + > static int qemu_rdma_registration_start(QEMUFile *f, void *opaque, > - uint64_t flags) > + uint64_t flags, void *data) > { > QEMUFileRDMA *rfile = opaque; > RDMAContext *rdma = rfile->rdma; > @@ -3144,7 +3160,7 @@ static int qemu_rdma_registration_start(QEMUFile *f, void *opaque, > * First, flush writes, if any. > */ > static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque, > - uint64_t flags) > + uint64_t flags, void *data) > { > Error *local_err = NULL, **errp = &local_err; > QEMUFileRDMA *rfile = opaque; > @@ -3266,7 +3282,7 @@ static const QEMUFileOps rdma_read_ops = { > .get_buffer = qemu_rdma_get_buffer, > .get_fd = qemu_rdma_get_fd, > .close = qemu_rdma_close, > - .hook_ram_load = qemu_rdma_registration_handle, > + .hook_ram_load = rdma_load_hook, > }; > > static const QEMUFileOps rdma_write_ops = { > diff --git a/trace-events b/trace-events > index 557770c..0f37a4b 100644 > --- a/trace-events > +++ b/trace-events > @@ -1432,7 +1432,7 @@ qemu_rdma_registration_handle_register_rkey(int rkey) "%x" > qemu_rdma_registration_handle_unregister(int requests) "%d requests" > qemu_rdma_registration_handle_unregister_loop(int count, int index, uint64_t chunk) "Unregistration request (%d): index %d, chunk %" PRIu64 > qemu_rdma_registration_handle_unregister_success(uint64_t chunk) "%" PRIu64 > -qemu_rdma_registration_handle_wait(uint64_t flags) "Waiting for next request %" PRIu64 > +qemu_rdma_registration_handle_wait(void) "" Are you using some kind of script to generate these trace prototypes? What happened to the message? =) > qemu_rdma_registration_start(uint64_t flags) "%" PRIu64 > qemu_rdma_registration_stop(uint64_t flags) "%" PRIu64 > qemu_rdma_registration_stop_ram(void) ""