"Dr. David Alan Gilbert (git)" <dgilb...@redhat.com> wrote: > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > > Open a return path, and handle messages that are received upon it. > > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > +/* > + * Handles messages sent on the return path towards the source VM > + * > + */ > +static void *source_return_path_thread(void *opaque) > +{ > + MigrationState *ms = opaque; > + QEMUFile *rp = ms->rp_state.file; > + uint16_t expected_len, header_len, header_type; > + const int max_len = 512; > + uint8_t buf[max_len]; > + uint32_t tmp32; > + int res; > + > + trace_source_return_path_thread_entry(); > + while (rp && !qemu_file_get_error(rp) &&
What can make rp == NULL? THinking about that, could you mean *rp here? > + migration_already_active(ms)) { > + trace_source_return_path_thread_loop_top(); > + header_type = qemu_get_be16(rp); > + header_len = qemu_get_be16(rp); > + > + switch (header_type) { > + case MIG_RP_MSG_SHUT: > + case MIG_RP_MSG_PONG: > + expected_len = 4; > + break; > + > + default: > + error_report("RP: Received invalid message 0x%04x length 0x%04x", > + header_type, header_len); > + source_return_path_bad(ms); > + goto out; > + } > > + if (header_len > expected_len) { > + error_report("RP: Received message 0x%04x with" > + "incorrect length %d expecting %d", > + header_type, header_len, > + expected_len); I know this is a big request, but getting an array with messages length and message names to be able to print nice error messages looks ilke good? > + source_return_path_bad(ms); > + goto out; > + } > + > + /* We know we've got a valid header by this point */ > + res = qemu_get_buffer(rp, buf, header_len); > + if (res != header_len) { > + trace_source_return_path_thread_failed_read_cmd_data(); > + source_return_path_bad(ms); > + goto out; > + } > + > + /* OK, we have the message and the data */ > + switch (header_type) { > + case MIG_RP_MSG_SHUT: > + tmp32 = be32_to_cpup((uint32_t *)buf); make local variable and call it sibling_error or whatever you like? > + trace_source_return_path_thread_shut(tmp32); > + if (tmp32) { > + error_report("RP: Sibling indicated error %d", tmp32); > + source_return_path_bad(ms); > + } > + /* > + * We'll let the main thread deal with closing the RP > + * we could do a shutdown(2) on it, but we're the only user > + * anyway, so there's nothing gained. > + */ > + goto out; > + > + case MIG_RP_MSG_PONG: > + tmp32 = be32_to_cpup((uint32_t *)buf); unused? Althought I guess it is used somewhere to make sure that the value is the same that whatever we did the ping. credentials? I can't see with this and previous patch what value is sent here. > + trace_source_return_path_thread_pong(tmp32); > + break; > + > + default: > + break; > + } > + } > + if (rp && qemu_file_get_error(rp)) { > + trace_source_return_path_thread_bad_end(); > + source_return_path_bad(ms); > + } > + > + trace_source_return_path_thread_end(); > +out: > + return NULL; > +} > + > +__attribute__ (( unused )) /* Until later in patch series */ unused_by_know attribute required O:-)