| Message ID | 1434450415-11339-14-git-send-email-dgilbert@redhat.com |
|---|---|
| State | New |
| Headers | show |
"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > 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 <dgilbert@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > --- > 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. > 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 */ 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? > 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) ""
* Juan Quintela (quintela@redhat.com) wrote: > "Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> wrote: > > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > > > 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 <dgilbert@redhat.com> > > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> > > --- > > 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 / dgilbert@redhat.com / Manchester, UK
On (Tue) 16 Jun 2015 [11:26:26], Dr. David Alan Gilbert (git) wrote: > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com> > > 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 <dgilbert@redhat.com> > Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Reviewed-by: Amit Shah <amit.shah@redhat.com> Amit
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 */ }; bool qemu_savevm_state_blocked(Error **errp); @@ -97,6 +99,8 @@ void qemu_savevm_state_cancel(void); uint64_t qemu_savevm_state_pending(QEMUFile *f, uint64_t max_size); void qemu_savevm_command_send(QEMUFile *f, enum qemu_vm_cmd command, uint16_t len, uint8_t *data); +void qemu_savevm_send_ping(QEMUFile *f, uint32_t value); +void qemu_savevm_send_open_return_path(QEMUFile *f); int qemu_loadvm_state(QEMUFile *f); typedef enum DisplayType diff --git a/migration/savevm.c b/migration/savevm.c index 7ce9d21..a995014 100644 --- a/migration/savevm.c +++ b/migration/savevm.c @@ -703,6 +703,20 @@ void qemu_savevm_command_send(QEMUFile *f, qemu_fflush(f); } +void qemu_savevm_send_ping(QEMUFile *f, uint32_t value) +{ + uint32_t buf; + + trace_savevm_send_ping(value); + buf = cpu_to_be32(value); + qemu_savevm_command_send(f, MIG_CMD_PING, sizeof(value), (uint8_t *)&buf); +} + +void qemu_savevm_send_open_return_path(QEMUFile *f) +{ + qemu_savevm_command_send(f, MIG_CMD_OPEN_RETURN_PATH, 0, NULL); +} + bool qemu_savevm_state_blocked(Error **errp) { SaveStateEntry *se; @@ -999,20 +1013,66 @@ static SaveStateEntry *find_se(const char *idstr, int instance_id) return NULL; } +static int loadvm_process_command_simple_lencheck(const char *name, + unsigned int actual, + unsigned int expected) +{ + if (actual != expected) { + error_report("%s received with bad length - expecting %d, got %d", + name, expected, actual); + return -1; + } + + return 0; +} + /* * Process an incoming 'QEMU_VM_COMMAND' * negative return on error (will issue error message) */ 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); 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) ""