Re: [Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load
* Juan Quintela (quint...@redhat.com) wrote:
> "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
>
> > @@ -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);
>
> Using a function in only two places, and passing two additional
> parameters for that
>
> > +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();
> > +}
>
> And you are doing two completely different things depending of the flag
The other way would be to add a new function pointer to qemu_file and
wire up the new pointer. It didn't seem any prettier.
Dave
> Later, Juan.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load
"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
> @@ -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);
Using a function in only two places, and passing two additional
parameters for that
> +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();
> +}
And you are doing two completely different things depending of the flag
Later, Juan.
Re: [Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load
"Dr. David Alan Gilbert" wrote: > * Michael R. Hines (mrhi...@linux.vnet.ibm.com) wrote: >> > 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? =) > > The prototypes I added manually. You normally have the trace name in your > tool so you don't need the text that gives you the same piece of information. > So I get output like: > > 25988@1434041422.299944:qemu_rdma_registration_handle_ram_blocks > > without having to write anything in the text. > (That's with the stderr trace backend, and the prefix being PID and time) Notice also that he removed the flags argument on the caller function, so nothing to print there, really. > > Dave > >> >> > qemu_rdma_registration_start(uint64_t flags) "%" PRIu64 >> > qemu_rdma_registration_stop(uint64_t flags) "%" PRIu64 >> > qemu_rdma_registration_stop_ram(void) "" >> > -- > Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
Re: [Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load
* Michael R. Hines (mrhi...@linux.vnet.ibm.com) wrote:
> 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_SETUP0
> >-#define RAM_CONTROL_ROUND1
> >-#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_FINISH3
> >+#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
Re: [Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load
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_SETUP0
-#define RAM_CONTROL_ROUND1
-#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_FINISH3
+#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);
+}
}
[Qemu-devel] [PATCH v2 07/12] Rework ram_control_load_hook to hook during block load
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_SETUP0
-#define RAM_CONTROL_ROUND1
-#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_FINISH3
+#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
+
