Re: [Qemu-devel] [PATCH v7 15/42] Return path: Source handling of return path

[Amit Shah]

On (Tue) 16 Jun 2015 [11:26:28], Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> Open a return path, and handle messages that are received upon it.
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>


> -/* migration thread support */
> +/*
> + * Something bad happened to the RP stream, mark an error
> + * The caller shall print something to indicate why
> + */
> +static void source_return_path_bad(MigrationState *s)

Can you rename this to something like

mark_source_rp_bad()

?

Intent is clearer that way.

Also, the comment says caller will print something, but the
invocations below are a mix of printfs and traces.  Not saying the
caller has to print always, but maybe only comment needs update.

> +{
> +    s->rp_state.error = true;
> +    migrate_fd_cleanup_src_rp(s);
> +}
> +
> +/*
> + * 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) &&
> +        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);
> +            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;
> +        }

                Amit