[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v7 13/42] Return path: Control commands
From: |
Dr. David Alan Gilbert |
Subject: |
Re: [Qemu-devel] [PATCH v7 13/42] Return path: Control commands |
Date: |
Tue, 23 Jun 2015 19:57:29 +0100 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
* Juan Quintela (address@hidden) wrote:
> "Dr. David Alan Gilbert (git)" <address@hidden> wrote:
> > From: "Dr. David Alan Gilbert" <address@hidden>
> >
> > Add two src->dest commands:
> > * OPEN_RETURN_PATH - To request that the destination open the return path
> > * PING - Request an acknowledge from the destination
> >
> > Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> > Reviewed-by: David Gibson <address@hidden>
> > ---
> > include/migration/migration.h | 2 ++
> > include/sysemu/sysemu.h | 6 ++++-
> > migration/savevm.c | 60
> > +++++++++++++++++++++++++++++++++++++++++++
> > trace-events | 2 ++
> > 4 files changed, 69 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/migration/migration.h b/include/migration/migration.h
> > index 8adaa45..65fe5db 100644
> > --- a/include/migration/migration.h
> > +++ b/include/migration/migration.h
> > @@ -47,6 +47,8 @@ typedef QLIST_HEAD(, LoadStateEntry) LoadStateEntry_Head;
> > struct MigrationIncomingState {
> > QEMUFile *file;
> >
> > + QEMUFile *return_path;
> > +
> > /* See savevm.c */
> > LoadStateEntry_Head loadvm_handlers;
> > };
> > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> > index 5869607..d8875ca 100644
> > --- a/include/sysemu/sysemu.h
> > +++ b/include/sysemu/sysemu.h
> > @@ -84,7 +84,9 @@ void qemu_announce_self(void);
> >
> > /* Subcommands for QEMU_VM_COMMAND */
> > enum qemu_vm_cmd {
> > - MIG_CMD_INVALID = 0, /* Must be 0 */
> > + MIG_CMD_INVALID = 0, /* Must be 0 */
> > + MIG_CMD_OPEN_RETURN_PATH, /* Tell the dest to open the Return path */
> > + MIG_CMD_PING, /* Request a PONG on the RP */
>
> Add MIG_CMD_MAX
>
>
> struct cmd_args {
> int len;
> char *name;
> } cmd_args[] ={
> .[MIG_CMD_INVALID] = {
> .len = 0,
> .name = "CMD_INVALID"},
> .[MIG_CMD_OPEN_RETURN_PATH] = {
> .len = 0,
> .name = "CMD_OPEN_RETURN_PATH"},
> .....
> }
> }
>
> I have done the initialization by hand, not sure if syntax is ok.
Pretty close, no '.] before the [] - we end up with (at the end of the series):
static struct mig_cmd_args {
ssize_t len; /* -1 = variable */
const char *name;
} mig_cmd_args[] = {
[MIG_CMD_INVALID] = { .len = -1, .name = "INVALID" },
[MIG_CMD_OPEN_RETURN_PATH] = { .len = 0, .name = "OPEN_RETURN_PATH" },
[MIG_CMD_PING] = { .len = sizeof(uint32_t), .name = "PING" },
[MIG_CMD_POSTCOPY_ADVISE] = { .len = 16, .name = "POSTCOPY_ADVISE" },
[MIG_CMD_POSTCOPY_LISTEN] = { .len = 0, .name = "POSTCOPY_LISTEN" },
[MIG_CMD_POSTCOPY_RUN] = { .len = 0, .name = "POSTCOPY_RUN" },
[MIG_CMD_POSTCOPY_RAM_DISCARD] =
{ .len = -1, .name = "POSTCOPY_RAM_DISCARD" },
[MIG_CMD_MAX] = { .len = -1, .name = "MAX" },
};
> > static int loadvm_process_command(QEMUFile *f)
> > {
> > + MigrationIncomingState *mis = migration_incoming_get_current();
> > uint16_t cmd;
> > uint16_t len;
> > + uint32_t tmp32;
> >
> > cmd = qemu_get_be16(f);
> > len = qemu_get_be16(f);
> >
> > trace_loadvm_process_command(cmd, len);
>
> /* yes, this should go in previous patch */
Yes, done.
> if (cmd > MIG_CMD_MAX ) {
> error_report("VM_COMMAND 0x%x unknown (len 0x%x)", cmd, len);
> .....
> }
>
> if (loadvm_process_command_simple_lencheck(cmd_args[cmd].name, len,
> cmd_args[cmd].len) {
> return -1;
> }
>
> switch (cmd) {
> case MIG_CMD_OPEN_RETURN_PATH:
> if (mis->return_path) {
> error_report("CMD_OPEN_RETURN_PATH called when RP already open");
> /* Not really a problem, so don't give up */
> return 0;
> }
> mis->return_path = qemu_file_get_return_path(f);
> if (!mis->return_path) {
> error_report("CMD_OPEN_RETURN_PATH failed");
> return -1;
> }
> break;
>
> .....
> }
>
> You get the idea. I normally even put the code from the command in a
> function pointer, but I know that other people don't like it.
> This way you factor the common code at the beginning of the function.
>
> What do you think?
Yep, done.
Dave
>
>
> > switch (cmd) {
> > + case MIG_CMD_OPEN_RETURN_PATH:
> > + if (loadvm_process_command_simple_lencheck("CMD_OPEN_RETURN_PATH",
> > + len, 0)) {
> > + return -1;
> > + }
> > + if (mis->return_path) {
> > + error_report("CMD_OPEN_RETURN_PATH called when RP already
> > open");
> > + /* Not really a problem, so don't give up */
> > + return 0;
> > + }
> > + mis->return_path = qemu_file_get_return_path(f);
> > + if (!mis->return_path) {
> > + error_report("CMD_OPEN_RETURN_PATH failed");
> > + return -1;
> > + }
> > + break;
> > +
> > + case MIG_CMD_PING:
> > + if (loadvm_process_command_simple_lencheck("CMD_PING", len,
> > + sizeof(tmp32))) {
> > + return -1;
> > + }
> > + tmp32 = qemu_get_be32(f);
> > + trace_loadvm_process_command_ping(tmp32);
> > + if (!mis->return_path) {
> > + error_report("CMD_PING (0x%x) received with no return path",
> > + tmp32);
> > + return -1;
> > + }
> > + /* migrate_send_rp_pong(mis, tmp32); TODO: gets added later */
> > + break;
> >
> > default:
> > error_report("VM_COMMAND 0x%x unknown (len 0x%x)", cmd, len);
> > diff --git a/trace-events b/trace-events
> > index 73a65c3..5967fdf 100644
> > --- a/trace-events
> > +++ b/trace-events
> > @@ -1184,8 +1184,10 @@ qemu_loadvm_state_section(unsigned int section_type)
> > "%d"
> > qemu_loadvm_state_section_partend(uint32_t section_id) "%u"
> > qemu_loadvm_state_section_startfull(uint32_t section_id, const char
> > *idstr, uint32_t instance_id, uint32_t version_id) "%u(%s) %u %u"
> > loadvm_process_command(uint16_t com, uint16_t len) "com=0x%x len=%d"
> > +loadvm_process_command_ping(uint32_t val) "%x"
> > savevm_section_start(const char *id, unsigned int section_id) "%s,
> > section_id %u"
> > savevm_section_end(const char *id, unsigned int section_id, int ret) "%s,
> > section_id %u -> %d"
> > +savevm_send_ping(uint32_t val) "%x"
> > savevm_state_begin(void) ""
> > savevm_state_header(void) ""
> > savevm_state_iterate(void) ""
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK
- Re: [Qemu-devel] [PATCH v7 10/42] Return path: Open a return path on QEMUFile for sockets, (continued)
- [Qemu-devel] [PATCH v7 11/42] Return path: socket_writev_buffer: Block even on non-blocking fd's, Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 12/42] Migration commands, Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 13/42] Return path: Control commands, Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 14/42] Return path: Send responses from destination to source, Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 15/42] Return path: Source handling of return path, Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 17/42] Add migration-capability boolean for postcopy-ram., Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 16/42] Rework loadvm path for subloops, Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 19/42] MIG_CMD_PACKAGED: Send a packaged chunk of migration stream, Dr. David Alan Gilbert (git), 2015/06/16
- [Qemu-devel] [PATCH v7 20/42] Modify save_live_pending for postcopy, Dr. David Alan Gilbert (git), 2015/06/16